[java-idp-plugin-webauthn] branch main updated: Code cleanup and Javadoc fixes

Phil Smart philip.smart at jisc.ac.uk
Tue Dec 19 16:49:10 UTC 2023


This is an automated email from the git hooks/post-receive script.

philsmart pushed a commit to branch main
in repository java-idp-plugin-webauthn.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-webauthn.git;a=commit;h=36608ecb2dc3e52599edfeb204b0cdd480948a89

The following commit(s) were added to refs/heads/main by this push:
     new 36608ec  Code cleanup and Javadoc fixes
36608ec is described below

commit 36608ecb2dc3e52599edfeb204b0cdd480948a89
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Dec 19 16:49:07 2023 +0000

    Code cleanup and Javadoc fixes
---
 .../webauthn/context/BaseWebAuthnContext.java      |  5 +-
 .../exception/AssertionFailureException.java       |  2 +-
 .../exception/RegistrationFailureException.java    |  2 +-
 webauthn-impl/pom.xml                              | 15 ++--
 .../CreatePublicKeyCredentialCreationOptions.java  | 41 +++++-----
 .../admin/impl/DeletePublicKeyCredential.java      |  2 +-
 ...actAuthenticatorAttestationFromFormRequest.java | 94 +++++++---------------
 ...xtractKeyRemovalInformationFromFormRequest.java | 34 +++-----
 .../webauthn/admin/impl/GenerateUserHandle.java    | 83 ++++++++++++-------
 .../ValidateAuthenticatorAttestationResponse.java  | 13 ++-
 .../impl/AbstractWebAuthnRegistrationAction.java   |  6 +-
 .../webauthn/impl/GenerateServerChallenge.java     | 74 ++++++++++++-----
 .../webauthn/impl/LookupRegisteredCredentials.java |  7 +-
 .../YubicoWebauthnAuthenticationClientTest.java    | 18 +++--
 14 files changed, 205 insertions(+), 191 deletions(-)

diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/BaseWebAuthnContext.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/BaseWebAuthnContext.java
index 4fdb824..ff00804 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/BaseWebAuthnContext.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/BaseWebAuthnContext.java
@@ -35,8 +35,9 @@ import net.shibboleth.shared.logic.Constraint;
 public class BaseWebAuthnContext extends BaseContext {
     
     /** 
-     * The username of the user that is the subject of this authentication. If {@code null} we can not determine 
-     * the userHandle (and hence public key) to use, and the flow will require a discoverable credential.
+     * The username of the user that is the subject of this authentication. In the authentication ceremony, if 
+     * {@code null} we can not determine the userHandle (and hence public key) to use, and the flow will require a 
+     * discoverable credential.
      */
     @Nullable private String username;    
     
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/AssertionFailureException.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/AssertionFailureException.java
index 25fa6b9..faf209d 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/AssertionFailureException.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/AssertionFailureException.java
@@ -15,7 +15,7 @@
 package net.shibboleth.idp.plugin.authn.webauthn.exception;
 
 /**
- * Exception that is throw if an assertion is not valid.
+ * Exception that is throw if an authentication assertion is not valid.
  */
 public class AssertionFailureException extends WebAuthnAuthenticationClientException {
     
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/RegistrationFailureException.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/RegistrationFailureException.java
index 7ed0b4e..2f9da25 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/RegistrationFailureException.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/RegistrationFailureException.java
@@ -15,7 +15,7 @@
 package net.shibboleth.idp.plugin.authn.webauthn.exception;
 
 /**
- * Exception that is throw if a public key credential registration is not valid.
+ * Exception that is throw if a public key credential registration (attestation) is not valid.
  */
 public class RegistrationFailureException extends WebAuthnAuthenticationClientException {
     
diff --git a/webauthn-impl/pom.xml b/webauthn-impl/pom.xml
index b916b3e..c7a2930 100644
--- a/webauthn-impl/pom.xml
+++ b/webauthn-impl/pom.xml
@@ -104,6 +104,13 @@
             <scope>provided</scope>
         </dependency>
         <!-- Provided dependencies -->
+        <dependency>
+            <groupId>net.shibboleth</groupId>
+            <artifactId>shib-security</artifactId>
+            <!-- TODO, why does this need a version? -->
+           
+            <scope>provided</scope>
+        </dependency>
         <dependency>
             <groupId>${idp.groupId}</groupId>
             <artifactId>idp-authn-api</artifactId>
@@ -202,11 +209,7 @@
             <artifactId>opensaml-core-impl</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>${shib-shared.groupId}</groupId>
-            <artifactId>shib-security</artifactId>
-            <scope>test</scope>
-        </dependency>
+
         <dependency>
             <groupId>${shib-shared.groupId}</groupId>
             <artifactId>shib-testing</artifactId>
@@ -216,7 +219,7 @@
             <groupId>${spring.groupId}</groupId>
             <artifactId>spring-test</artifactId>
             <scope>test</scope>
-        </dependency>
+        </dependency>        
     </dependencies>
 
     <build>
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java
index 3b1ca61..d35a105 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java
@@ -15,7 +15,6 @@
 
 package net.shibboleth.idp.plugin.authn.webauthn.admin.impl;
 
-import java.util.Collection;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -23,6 +22,7 @@ import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 
 import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 
@@ -31,19 +31,21 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.yubico.webauthn.data.PublicKeyCredentialCreationOptions;
 import com.yubico.webauthn.data.PublicKeyCredentialDescriptor;
 
-import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.plugin.authn.webauthn.client.WebAuthnAuthenticationClient;
 import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
 import net.shibboleth.idp.plugin.authn.webauthn.exception.WebAuthnAuthenticationClientException;
 import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnRegistrationAction;
-import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
- * Action to create a PublicKeyCredentialCreationOptions from the parameters in the WebAuthn context.
+ * Action to create a PublicKeyCredentialCreationOptions from the parameters in the WebAuthn context using the available
+ * {@link WebAuthnAuthenticationClient client}.
+ * 
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @event {@link EventIds#IO_ERROR}
  */
 public class CreatePublicKeyCredentialCreationOptions extends AbstractWebAuthnRegistrationAction {
     
@@ -64,7 +66,6 @@ public class CreatePublicKeyCredentialCreationOptions extends AbstractWebAuthnRe
         }
     }
     
-    
     /**
      * Set the JSON object mapper to use.
      * 
@@ -72,41 +73,41 @@ public class CreatePublicKeyCredentialCreationOptions extends AbstractWebAuthnRe
      */
     public void setObjectMapper(@Nonnull final ObjectMapper mapper) {
         checkSetterPreconditions();
-        objectMapper = Constraint.isNotNull(mapper, "JsonObjectMapper can not be null");
+        objectMapper = Constraint.isNotNull(mapper, "JSON Object Mapper can not be null");
     }       
-  
     
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final WebAuthnRegistrationContext context) {
-        
-        final WebAuthnAuthenticationClient client = getWebAuthnClient();
+
         final byte[] challenge = context.getServerChallenge();
         if (challenge == null) {
             log.error("{} WebAuthn challenge is null, has the context been created correctly?",getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
             return;
         }
         
         try {         
-            final Collection<CredentialRegistration> existingCredentials = context.getExistingCredentials();
-            final Set<PublicKeyCredentialDescriptor> existingCredentialDescriptors  = existingCredentials.stream()
-                .map(cred -> cred.toPublicKeyCredentialDescriptor())
-                .filter(Objects::nonNull)
-                .collect(Collectors.toSet());
+            final Set<PublicKeyCredentialDescriptor> existingCredentialDescriptors  = context.getExistingCredentials()
+                    .stream()
+                        .map(cred -> cred.toPublicKeyCredentialDescriptor())
+                        .filter(Objects::nonNull)
+                        .collect(Collectors.toSet());
+            
+            assert null != existingCredentialDescriptors;
             
             final PublicKeyCredentialCreationOptions pkCredCreationOptions = 
-                    client.createRegistrationRequest(existingCredentialDescriptors, context.getUsername(), 
+                    getWebAuthnClient().createRegistrationRequest(existingCredentialDescriptors, context.getUsername(), 
                             context.getUserHandle(), challenge);
+            
             context.setPublicKeyCredentialCreationOptions(pkCredCreationOptions);
-            //convert to JSON for the JS api to use
-            context.setPublicKeyCredentialCreationOptionsJSON(
-                    objectMapper.writeValueAsString(pkCredCreationOptions));
+            //convert to JSON for the JS API to use
+            context.setPublicKeyCredentialCreationOptionsJSON(objectMapper.writeValueAsString(pkCredCreationOptions));
             
             log.debug("{} Created PublicKeyCredentialCreationOptions '{}'",getLogPrefix(), pkCredCreationOptions);
         } catch (final WebAuthnAuthenticationClientException | JsonProcessingException e) {
             log.error("{} Unable to generate PublicKeyCredentialCreationOptions",getLogPrefix(), e);
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
+            ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
             return;
         }        
     }
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java
index eda1779..47de2a7 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java
@@ -34,7 +34,7 @@ import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
- * An action that stores the public key credential into the credential repository.
+ * An action that removes a public key credential from the credential repository.
  */
 public class DeletePublicKeyCredential extends AbstractWebAuthnRegistrationAction {
 
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java
index 914b452..e33a7fb 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java
@@ -20,13 +20,12 @@ package net.shibboleth.idp.plugin.authn.webauthn.admin.impl;
 import java.io.IOException;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 
 import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.yubico.webauthn.data.AuthenticatorAttestationResponse;
 import com.yubico.webauthn.data.ClientRegistrationExtensionOutputs;
 import com.yubico.webauthn.data.PublicKeyCredential;
@@ -35,84 +34,61 @@ import jakarta.servlet.http.HttpServletRequest;
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
 import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnRegistrationAction;
-import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
-import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.shared.primitive.StringSupport;
 
 
 /**
- * An action that extracts the AuthenticatorAttestationResponse from the incoming HTTP request.
+ * An action that extracts the Authenticator Attestation Response and credential Nickname from the incoming
+ * HTTP request.
  */
 public class ExtractAuthenticatorAttestationFromFormRequest extends AbstractWebAuthnRegistrationAction {
 
-    /** Default assertion field name. */
+    /** Default assertion parameter name. */
     @Nonnull @NotEmpty public static final String DEFAULT_ASSERTION_FIELD_NAME = "authenticatorAttestation";
     
-    /** Default nickname field name. */
+    /** Default nickname parameter name. */
     @Nonnull @NotEmpty public static final String DEFAULT_NICKNAME_FIELD_NAME = "credentialNickname";
     
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(ExtractAuthenticatorAttestationFromFormRequest.class);
     
-    /** Name of assertion field. */
-    @Nonnull @NotEmpty private String assertionFieldName;
+    /** Name of the attestation parameter. */
+    @Nonnull @NotEmpty private String attestationParameterName;
     
-    /** Name of nickname field. */
-    @Nonnull @NotEmpty private  String credentialNicknameFieldName;
-    
-    /** JSON object mapper. */
-    @NonnullAfterInit private ObjectMapper objectMapper;
+    /** Name of the nickname parameter. */
+    @Nonnull @NotEmpty private String credentialNicknameParameterName;
     
     /** Constructor. */
     public ExtractAuthenticatorAttestationFromFormRequest() {
-        assertionFieldName = DEFAULT_ASSERTION_FIELD_NAME;
-        credentialNicknameFieldName = DEFAULT_NICKNAME_FIELD_NAME;
-    }
-    
-    @Override protected void doInitialize() throws ComponentInitializationException {
-        super.doInitialize();
-
-        if (objectMapper ==  null) {
-            throw new ComponentInitializationException("The objectMapper cannot be null");
-        }
-    }    
-    
-    /**
-     * Set the JSON {@link ObjectMapper}.
-     * 
-     * @param mapper object mapper
-     */
-    public void setObjectMapper(@Nonnull final ObjectMapper mapper) {
-        checkSetterPreconditions();
-
-        objectMapper = Constraint.isNotNull(mapper, "Object mapper cannot be null");
+        attestationParameterName = DEFAULT_ASSERTION_FIELD_NAME;
+        credentialNicknameParameterName = DEFAULT_NICKNAME_FIELD_NAME;
     }
     
     /**
-     * Set the name of the assertion field to examine.
+     * Set the name of the parameter to extract the attestation response from.
      * 
      * @param field field name
      */
-    public void setAssertionFieldName(@Nonnull @NotEmpty final String field) {
+    public void setAttestationParameterName(@Nonnull @NotEmpty final String field) {
         checkSetterPreconditions();
         
-        assertionFieldName = Constraint.isNotNull(StringSupport.trimOrNull(field), 
-                "Assertion Field name cannot be null or empty");
+        attestationParameterName = Constraint.isNotNull(StringSupport.trimOrNull(field), 
+                "Attestation parameter name cannot be null or empty");
     }
     
     /**
-     * Set the name of the nickname field to examine.
+     * Set the name of the parameter to extract the credential nickname from.
      * 
      * @param field field name
      */
-    public void setCredentialNicknameFieldName(@Nonnull @NotEmpty final String field) {
+    public void setCredentialNicknameParameterName(@Nonnull @NotEmpty final String field) {
         checkSetterPreconditions();
         
-        credentialNicknameFieldName = Constraint.isNotNull(StringSupport.trimOrNull(field),
-                "Nickname FieldName can not be null");
+        credentialNicknameParameterName = Constraint.isNotNull(StringSupport.trimOrNull(field),
+                "Nickname parameter can not be null or empty");
     }
     
     @Override
@@ -122,22 +98,23 @@ public class ExtractAuthenticatorAttestationFromFormRequest extends AbstractWebA
         final HttpServletRequest request = getHttpServletRequest();
         if (request == null) {
             log.debug("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
             return;
         }
         
-        final String pkCredAttestationJson = extractParameter(request, assertionFieldName);   
-        log.trace("Public key credential authenticator attestation response in JSON is '{}'",pkCredAttestationJson);
+        final String pkCredAttestationJson = request.getParameter(attestationParameterName);        
         if (StringSupport.trimOrNull(pkCredAttestationJson) == null) {
-            log.warn("{} Could not extract authenticator attestation response from form", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
+            log.warn("{} No authenticator attestation response in request", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
             return;
         }
+        log.trace("Public key credential authenticator attestation response in JSON is '{}'",pkCredAttestationJson);
         
-        final String credNickname = extractParameter(request, credentialNicknameFieldName);   
-        log.trace("Public key credential nickname is '{}'",pkCredAttestationJson);
+        final String credNickname = request.getParameter(credentialNicknameParameterName);   
+        log.trace("Public key credential nickname is '{}'",credNickname);
         if (StringSupport.trimOrNull(credNickname) == null) {
-            log.warn("{} Could not extract nickname from form", getLogPrefix());
+            log.warn("{} No nickname in request", getLogPrefix());
+            //TODO which event?
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
@@ -152,21 +129,6 @@ public class ExtractAuthenticatorAttestationFromFormRequest extends AbstractWebA
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
-
-       
-
-    }
-
-    /**
-     * Extract the given parameter from the servlet request.
-     * 
-     * @param httpRequest the http request
-     * 
-     * @return the value of the parameter.
-     */
-    @Nullable private String extractParameter(@Nonnull final HttpServletRequest httpRequest, 
-            @Nonnull @NotEmpty final String field) {
-        return httpRequest.getParameter(field);
     }
     
 }
\ No newline at end of file
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java
index 3bc15fe..fc597a2 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java
@@ -18,7 +18,6 @@
 package net.shibboleth.idp.plugin.authn.webauthn.admin.impl;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 
 import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventIds;
@@ -28,7 +27,6 @@ import org.slf4j.Logger;
 import jakarta.servlet.http.HttpServletRequest;
 import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
 import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnRegistrationAction;
-import net.shibboleth.idp.profile.IdPEventIds;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.codec.Base64Support;
@@ -43,29 +41,30 @@ import net.shibboleth.shared.primitive.StringSupport;
  */
 public class ExtractKeyRemovalInformationFromFormRequest extends AbstractWebAuthnRegistrationAction {
 
-    /** Default token code field name. */
-    @Nonnull @NotEmpty public static final String DEFAULT_FIELD_NAME = "credentialId";
+    /** Default credential Id parameter name. */
+    @Nonnull @NotEmpty public static final String DEFAULT_PARAMETER_NAME = "credentialId";
     
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(ExtractKeyRemovalInformationFromFormRequest.class);
     
-    /** Name of header. */
-    @NonnullAfterInit @NotEmpty private String fieldName;
+    /** Name of credential Id parameter. */
+    @NonnullAfterInit @NotEmpty private String credentialIdParameterName;
 
     /** Constructor. */
     public ExtractKeyRemovalInformationFromFormRequest() {       
-        fieldName = DEFAULT_FIELD_NAME;
+        credentialIdParameterName = DEFAULT_PARAMETER_NAME;
     }
     
     /**
-     * Set the name of the field to examine.
+     * Set the name of the credential Id parameter.
      * 
      * @param field field name
      */
-    public void setFieldName(@Nonnull @NotEmpty final String field) {
+    public void setCedentialIdParameterName(@Nonnull @NotEmpty final String field) {
         checkSetterPreconditions();
         
-        fieldName = Constraint.isNotNull(StringSupport.trimOrNull(field), "Field name cannot be null or empty");
+        credentialIdParameterName = Constraint.isNotNull(StringSupport.trimOrNull(field), "Credential ID "
+                + "parameter cannot be null or empty");
     }
     
     @Override
@@ -75,11 +74,11 @@ public class ExtractKeyRemovalInformationFromFormRequest extends AbstractWebAuth
         final HttpServletRequest request = getHttpServletRequest();
         if (request == null) {
             log.debug("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_PROFILE_CONFIG);
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
             return;
         }
 
-        final String credentialId = extractCredentialId(request);         
+        final String credentialId = request.getParameter(credentialIdParameterName);         
         if (credentialId == null) {
             //TODO look at these eventIds
             log.debug("{} CredentialID not found in HTTP request",getLogPrefix());
@@ -99,16 +98,5 @@ public class ExtractKeyRemovalInformationFromFormRequest extends AbstractWebAuth
         }
 
     }
-
-    /**
-     * Extract the credential identifier from the HTTP request parameters.
-     * 
-     * @param httpRequest the http request
-     * 
-     * @return the credential identifier
-     */
-    @Nullable private String extractCredentialId(@Nonnull final HttpServletRequest httpRequest) {
-        return httpRequest.getParameter(fieldName);
-    }
     
 }
\ No newline at end of file
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/GenerateUserHandle.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/GenerateUserHandle.java
index cd16203..3f13598 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/GenerateUserHandle.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/GenerateUserHandle.java
@@ -19,64 +19,93 @@ package net.shibboleth.idp.plugin.authn.webauthn.admin.impl;
 
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
+import java.util.function.Function;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 
-import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
 import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnRegistrationAction;
+import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
- * An action to generate an appropriate server challenge for use.
+ * An action to generate a UserHandle as the user.id. This is used by the IdP to map a public key credential to a 
+ * users session map of public keys, and by the Authenticator to map the IdP's ID (RelyingParty ID) and the 
+ * User Handle to a public key credential source (which contains the private key).
  * 
  * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
  * @event {@link org.opensaml.profile.action.EventIds#INVALID_PROFILE_CTX}
- * @event {@link net.shibboleth.idp.authn.AuthnEventIds#NO_CREDENTIALS}
- * @event {@link net.shibboleth.idp.authn.AuthnEventIds#AUTHN_EXCEPTION}
- * @post See above.
+ * @post a UserHandle is added to the registration context
  */
 public class GenerateUserHandle extends AbstractWebAuthnRegistrationAction {
     
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(GenerateUserHandle.class);
    
+    /** Strategy used to generate the UserHandle. */
+    @Nonnull private Function<ProfileRequestContext,byte[]> userHandleGeneratorStrategy;
+    
+    /** Constructor. */
+    public GenerateUserHandle() {
+        userHandleGeneratorStrategy = new DefaultUserHandleGenerator();
+    }
+    
+    /**
+     * Set the strategy used to generate the UserHandle.
+     * 
+     * @param strategy the strategy
+     */
+    public void setUserHandleGeneratorStrategy(
+            @Nonnull final Function<ProfileRequestContext,byte[]> strategy) {
+        checkSetterPreconditions();
+        userHandleGeneratorStrategy =
+                Constraint.isNotNull(strategy, "Challenge Generator cannot be null");
+    }
 
     /** {@inheritDoc} */
     @Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final WebAuthnRegistrationContext context) {
         
-        try {
-            final byte[] userHandle = generateUserHandle();
-            log.trace("Generated userHandle '{}'",userHandle);
-            context.setUserHandle(userHandle);
-            
-        } catch (final NoSuchAlgorithmException e) {
-            log.error("Could not generate a challenge",e);
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.AUTHN_EXCEPTION);
-        }        
-    }
+        final byte[] userHandle = userHandleGeneratorStrategy.apply(profileRequestContext);
+        if (userHandle == null) {
+            log.trace("{} Generated UserHandle was null", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            return;
+        }
+        log.trace("{} Generated UserHandle '{}'",getLogPrefix(),userHandle);
+        context.setUserHandle(userHandle);   
+             
+    }    
     
     /**
-     * Generate a 64 bytes randomized challenge of sufficient entropy. Must be at least 32 bytes long.
+     * Default User Handle generator that generates a 64 byte randomized UserHandle (must be at least 32 bytes long). 
+     * Returns {@code null} iff one can not be generated.
      * 
      * </p>This could contain some form of state if required, but must not contain retrievable PII.</p>
-     * 
-     * @see <a href="https://www.w3.org/TR/webauthn-2/#sctn-user-handle-privacy">user handle</a>
-     * 
-     * @return the challenge in bytes
-     * 
-     * @throws NoSuchAlgorithmException if no secure random algorithm is available
      */
-    @Nonnull private byte[] generateUserHandle() throws NoSuchAlgorithmException {        
-        final byte[] bytes = new byte[64];
-        SecureRandom.getInstanceStrong().nextBytes(bytes);
-        log.trace("Generated '{}' byte challenge",bytes.length);
-        return bytes;
+    private static final class DefaultUserHandleGenerator implements Function<ProfileRequestContext, byte[]>{
+        
+        /** Class logger. */
+        @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultUserHandleGenerator.class);
+
+        /** {@inheritDoc} */
+        @Override
+        @Nullable public byte[] apply(final ProfileRequestContext input) {           
+            try {
+                final byte[] bytes = new byte[64];
+                SecureRandom.getInstanceStrong().nextBytes(bytes);
+                return bytes;
+            } catch (final NoSuchAlgorithmException e) {
+                log.error("Unable to generate UserHandle", e);
+                return null;
+            }            
+        }      
     }
     
     
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java
index 3dd24fc..c11ce90 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java
@@ -34,11 +34,10 @@ import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationCont
 import net.shibboleth.idp.plugin.authn.webauthn.exception.RegistrationFailureException;
 import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnRegistrationAction;
 import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
-import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
- * Validate the public key registration attempt. If valid store it inside the credential repository.
+ * Validate the public key registration attempt. If valid store it inside the registration context.
  */
 public class ValidateAuthenticatorAttestationResponse extends AbstractWebAuthnRegistrationAction {
 
@@ -47,7 +46,7 @@ public class ValidateAuthenticatorAttestationResponse extends AbstractWebAuthnRe
     private final Logger log = LoggerFactory.getLogger(ValidateAuthenticatorAttestationResponse.class);
     
     /** The stashed public key credential creation options used to create a new credential.*/
-    @NonnullBeforeExec @NotEmpty private PublicKeyCredentialCreationOptions pkCredCreationOptions;
+    @NonnullBeforeExec private PublicKeyCredentialCreationOptions pkCredCreationOptions;
     
     /** The stashed authenticator response.*/
     @NonnullBeforeExec
@@ -65,14 +64,14 @@ public class ValidateAuthenticatorAttestationResponse extends AbstractWebAuthnRe
        
         attestation = context.getAuthenticatorAttestationResponse();
         if (attestation == null) {
-            log.error("{} authenticator attestation response was null", getLogPrefix());
+            log.error("{} Authenticator attestation response was null", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return false;
         }
         
         pkCredCreationOptions = context.getPublicKeyCredentialCreationOptions();
         if (pkCredCreationOptions == null) {
-            log.error("{} public key credential creation options was null", getLogPrefix());
+            log.error("{} Public key credential creation options was null", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return false;
         }
@@ -84,14 +83,14 @@ public class ValidateAuthenticatorAttestationResponse extends AbstractWebAuthnRe
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final WebAuthnRegistrationContext context) {
 
-        try {            
+        try {        
             final RegistrationResult credentialPublicKey = 
                 getWebAuthnClient().validateAuthenticatorAttestationResponse(pkCredCreationOptions, attestation);            
             // If valid. Add back to context
             context.setRegistrationResult(credentialPublicKey);
             log.info("Public Key Registration was valid");
         } catch (final RegistrationFailureException e) {
-            log.error("{} public key credential creation options was invalid", getLogPrefix(), e);
+            log.warn("{} Public key credential creation options was invalid", getLogPrefix(), e);
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnRegistrationAction.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnRegistrationAction.java
index d49ae51..c0f07fa 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnRegistrationAction.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnRegistrationAction.java
@@ -39,7 +39,7 @@ import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
- * <p>A base class for WebAuthn registration related admin actions.</p>
+ * <p>A base class for WebAuthn registration related administration actions.</p>
  * 
  * <p>In addition to the work performed by {@link AbstractProfileAction}, this action also looks up
  * and makes available the {@link WebAuthnRegistrationContext}.</p>
@@ -114,7 +114,7 @@ public abstract class AbstractWebAuthnRegistrationAction extends AbstractProfile
     
     
     /**
-     * Set the credential repository used to store webauthn credentials.
+     * Set the credential repository used to store WebAuthn credentials.
      *  
      * @param repository The respository to set.
      */
@@ -124,7 +124,7 @@ public abstract class AbstractWebAuthnRegistrationAction extends AbstractProfile
     }
     
     /**
-     * Get the credential repository used to store webauthn credentials.
+     * Get the credential repository used to store WebAuthn credentials.
      * 
      * @return the credential repository.
      */
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
index 796486d..597c18c 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
@@ -19,8 +19,10 @@ package net.shibboleth.idp.plugin.authn.webauthn.impl;
 
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
+import java.util.function.Function;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventIds;
@@ -28,49 +30,77 @@ import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 
 import net.shibboleth.idp.plugin.authn.webauthn.context.BaseWebAuthnContext;
+import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
+
 /**
  * An action to generate an appropriate server challenge for use.
  * 
  * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
  * @event {@link org.opensaml.profile.action.EventIds#INVALID_PROFILE_CTX}
- * @event {@link net.shibboleth.idp.authn.AuthnEventIds#NO_CREDENTIALS}
- * @event {@link net.shibboleth.idp.authn.AuthnEventIds#AUTHN_EXCEPTION}
- * @post See above.
+ * @post a byte challenge is added to the base context
  */
 public class GenerateServerChallenge extends AbstractWebAuthnBaseAction {
     
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(GenerateServerChallenge.class);
+    
+    /** Strategy used to generate the challenge to use. */
+    @Nonnull private Function<ProfileRequestContext,byte[]> challengeGeneratorStrategy;
+    
+    /** Constructor. */
+    public GenerateServerChallenge() {
+        challengeGeneratorStrategy = new DefaultChallengeGenerator();
+    }
+    
+    /**
+     * Set the strategy used to generate the challenge.
+     * 
+     * @param strategy the strategy
+     */
+    public void setChallengeGeneratorStrategy(
+            @Nonnull final Function<ProfileRequestContext,byte[]> strategy) {
+        checkSetterPreconditions();
+        challengeGeneratorStrategy =
+                Constraint.isNotNull(strategy, "Challenge Generator cannot be null");
+    }
 
     /** {@inheritDoc} */
     @Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final BaseWebAuthnContext context) {
-        
-        try {
-            final byte[] challenge = generateChallenge();
-            log.trace("Generated challenge {}",challenge);
-            context.setServerChallenge(challenge);
-            
-        } catch (final NoSuchAlgorithmException e) {
-            log.error("Could not generate a challenge",e);
+                  
+        final byte[] challenge = challengeGeneratorStrategy.apply(profileRequestContext);
+        if (challenge == null) {
+            log.trace("{} Generated challenge was null",getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
-        }        
+            return;
+        }
+        log.trace("{} Generated server challenge {}",getLogPrefix() ,challenge);
+        context.setServerChallenge(challenge);
     }
     
     /**
-     * Generate a 32 bytes randomized challenge of sufficient entropy. Must be at least 16 bytes long.
-     * 
-     * @return the challenge in bytes
-     * 
-     * @throws NoSuchAlgorithmException if no secure random algorithm is available
+     * Default challenge generator that generates a 32 bytes randomized challenge of sufficient entropy. Returns
+     * {@code null} iff one can not be generated.
      */
-    @Nonnull private byte[] generateChallenge() throws NoSuchAlgorithmException {        
-        final byte[] bytes = new byte[32];
-        SecureRandom.getInstanceStrong().nextBytes(bytes);
-        log.trace("Generated '{}' byte challenge",bytes.length);
-        return bytes;
+    private static final class DefaultChallengeGenerator implements Function<ProfileRequestContext, byte[]>{
+        
+        /** Class logger. */
+        @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultChallengeGenerator.class);
+
+        /** {@inheritDoc} */
+        @Override
+        @Nullable public byte[] apply(final ProfileRequestContext input) {           
+            try {
+                final byte[] bytes = new byte[32];
+                SecureRandom.getInstanceStrong().nextBytes(bytes);
+                return bytes;
+            } catch (final NoSuchAlgorithmException e) {
+                log.error("Unable to generate challenge", e);
+                return null;
+            }            
+        }      
     }
     
     
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java
index b2c2476..d96fd2e 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/LookupRegisteredCredentials.java
@@ -28,7 +28,7 @@ import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
- * Action that lookups existing registered credentials.
+ * An action that lookups existing registered credentials.
  */
 public class LookupRegisteredCredentials extends AbstractWebAuthnBaseAction {
 
@@ -41,7 +41,6 @@ public class LookupRegisteredCredentials extends AbstractWebAuthnBaseAction {
             @Nonnull final BaseWebAuthnContext context) {
         
         final String username = context.getUsername();
-
         if (username == null) {
             log.error("Unable to find username in registration context");
             ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_AUTHN_CTX);
@@ -55,9 +54,7 @@ public class LookupRegisteredCredentials extends AbstractWebAuthnBaseAction {
         log.debug("{} Found '{}' registered credentials for '{}'", getLogPrefix(), 
                 credentials != null ? credentials.size() : "0", username);
         
-        context.setExistingCredentials(credentials);
-        
-        
+        context.setExistingCredentials(credentials);  
     }
 
 }
diff --git a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java
index b87a1df..64732c3 100644
--- a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java
+++ b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java
@@ -202,8 +202,9 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                         .getAttestedCredentialData().get().getCredentialPublicKey())
                 .build();
         
-        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickanme"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty());
+        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickname"), 
+                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(), Optional.of(true),
+                true);
         
         storage.addRegistrationByUsername(USERNAME, reg);
         
@@ -241,8 +242,9 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                         .getAttestedCredentialData().get().getCredentialPublicKey())
                 .build();
         
-        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickanme"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty());
+        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickname"), 
+                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(),Optional.of(true),
+                true);
         
         storage.addRegistrationByUsername(USERNAME, reg);
         
@@ -278,8 +280,9 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                         .getAttestedCredentialData().get().getCredentialPublicKey())
                 .build();
         
-        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickanme"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty());
+        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickname"), 
+                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(),Optional.of(true),
+                true);
         
         storage.addRegistrationByUsername(USERNAME, reg);
         
@@ -317,7 +320,8 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                 .build();
         
         final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickanme"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty());
+                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(),Optional.of(true),
+                true);
         
         storage.addRegistrationByUsername(USERNAME, reg);
         

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list