[java-idp-plugin-webauthn] branch main updated: Change userHandle to userId on registration request

Phil Smart philip.smart at jisc.ac.uk
Fri Feb 16 17:24:23 UTC 2024


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=657b515a1986ea70f141f7023c61d576b4d658eb

The following commit(s) were added to refs/heads/main by this push:
     new 657b515  Change userHandle to userId on registration request
657b515 is described below

commit 657b515a1986ea70f141f7023c61d576b4d658eb
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Feb 16 17:24:21 2024 +0000

    Change userHandle to userId on registration request
    
     - Even if these are equivalent. It is the user.id that is the source of
    the userHandle. I had it the wrong way round.
---
 .../client/WebAuthnAuthenticationClient.java       |  7 ++--
 .../webauthn/context/BaseWebAuthnContext.java      | 23 +++++++------
 .../{GenerateUserHandle.java => AddUserId.java}    | 39 +++++++++++-----------
 .../CreatePublicKeyCredentialCreationOptions.java  |  9 ++---
 .../admin/impl/StorePublicKeyCredential.java       |  5 +--
 .../impl/YubicoWebauthnAuthenticationClient.java   | 11 +++---
 .../client/impl/YubicoWebauthnClientFactory.java   |  4 +--
 .../webauthn/impl/ValidateWebAuthnAssertion.java   |  2 +-
 .../webauthn-registration-beans.xml                |  4 +--
 .../webauthn-registration-flow.xml                 |  2 +-
 .../authn/webauthn/conf/authn/webauthn.properties  |  2 +-
 11 files changed, 57 insertions(+), 51 deletions(-)

diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java
index 97025d9..bbf3da3 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java
@@ -63,8 +63,8 @@ public interface WebAuthnAuthenticationClient {
      /**
       * Validate the Authenticator Assertion Response from an authentication request.
       * 
-      * @param username ....TODO
-      * @param userHandle ....TODO
+      * @param username the username of the users account on the IdP. Should map one-to-one with the user.id.
+      * @param userId the user.id of the users account on the IdP. Should match the userHandle in the assertion response
       * @param publicKeyCredentialRequestOptions the options used when generating an assertion for authentication.
       * @param authenticatorAssertionResponse the assertion response.
       * 
@@ -73,8 +73,9 @@ public interface WebAuthnAuthenticationClient {
       * @throws AssertionFailureException if the assertion is not valid
       */
      //TODO would need our own AssertionResult to make this usuable beyond Yubico.
+     //TODO do we need a userId supplied here?
      AssertionResult validateAuthenticatorAssertionResponse(@Nullable final String username, 
-             @Nullable final byte[] userHandle, 
+             @Nullable final byte[] userId, 
              @Nonnull final PublicKeyCredentialRequestOptions publicKeyCredentialRequestOptions, 
              @Nonnull final PublicKeyCredential<AuthenticatorAssertionResponse, ClientAssertionExtensionOutputs> 
              authenticatorAssertionResponse) throws AssertionFailureException;
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 a297505..f724601 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
@@ -52,8 +52,8 @@ public class BaseWebAuthnContext extends BaseContext {
     /** The challenge sent to the authenticator in both registration and authentication ceremonies.*/
     @Nullable private byte[] serverChallenge;  
     
-    /** The userhandle supplied to the authenticator during registration. As generated by the IdP.*/
-    @Nullable private byte[] userHandle;    
+    /** The user.id supplied to the authenticator as a userHandle during registration. Generated by the IdP.*/
+    @Nullable private byte[] userId;    
     
     /** Does the authentication/registration require user verification.*/
     @Nullable private UserVerificationRequirement userVerificationRequirement;
@@ -152,26 +152,27 @@ public class BaseWebAuthnContext extends BaseContext {
     
     
     /**
-     * Set the user handle used to map public key credentials to user accounts. Maximum 64 bytes 
+     * Set the user.id used to map public key credentials to user accounts. Maximum 64 bytes 
      * 
      * @param handle The userHandle to set.
      * 
      * @return this context
      */
-    @Nonnull public BaseWebAuthnContext setUserHandle(@Nonnull final byte[] handle) {
-        Constraint.isNotEmpty(handle,"UserHandle can not be null or empty");
-        Constraint.isLessThan(65, handle.length, "UserHandle must be maximum 64 bytes");
-        userHandle = handle;
+    @Nonnull public BaseWebAuthnContext setUserId(@Nonnull final byte[] id) {
+        Constraint.isNotEmpty(id,"UserID can not be null or empty");
+        Constraint.isLessThan(65, id.length, "UserID must be maximum 64 bytes");
+        userId = id;
         return this;
     }
     
     /**
-     * Get the userId used to map public key credentials to user accounts.
+     * Get the user.id used to map public key credentials to user accounts. Send to the authenticator during credential
+     * creation. Referred to as the userHandle in responses from the authenticator.
      * 
-     * @return the userHandle.
+     * @return the userId.
      */
-    @Nullable public byte[] getUserHandle() {
-        return userHandle;
+    @Nullable public byte[] getUserId() {
+        return userId;
     }
     
     
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/AddUserId.java
similarity index 80%
rename from webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/GenerateUserHandle.java
rename to webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
index 4c3bb4e..a2f258a 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/AddUserId.java
@@ -39,7 +39,7 @@ import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
- * An action to generate or lookup a UserHandle as the user.id. This is used by the IdP to map a public key credential 
+ * An action to generate or lookup a user.id used as a userHandle. 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).
  * 
@@ -49,31 +49,31 @@ import net.shibboleth.shared.primitive.LoggerFactory;
  * @event {@link org.opensaml.profile.action.EventIds#INVALID_PROFILE_CTX}
  * @post a UserHandle is added to the registration context
  */
-public class GenerateUserHandle extends AbstractWebAuthnRegistrationAction {
+public class AddUserId extends AbstractWebAuthnRegistrationAction {
     
     /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(GenerateUserHandle.class);
+    @Nonnull private final Logger log = LoggerFactory.getLogger(AddUserId.class);
    
     /** Strategy used to generate the UserHandle. */
-    @Nonnull private Function<ProfileRequestContext,byte[]> userHandleGeneratorStrategy;
+    @Nonnull private Function<ProfileRequestContext,byte[]> userIdGeneratorStrategy;
     
     /** The stashed username.*/
     @NonnullBeforeExec private String username;
     
     /** Constructor. */
-    public GenerateUserHandle() {
-        userHandleGeneratorStrategy = new DefaultUserHandleGenerator();
+    public AddUserId() {
+        userIdGeneratorStrategy = new DefaultUserIdGenerator();
     }
     
     /**
-     * Set the strategy used to generate the UserHandle.
+     * Set the strategy used to generate the user.id.
      * 
      * @param strategy the strategy
      */
-    public void setUserHandleGeneratorStrategy(
+    public void setUserIdGeneratorStrategy(
             @Nonnull final Function<ProfileRequestContext,byte[]> strategy) {
         checkSetterPreconditions();
-        userHandleGeneratorStrategy =
+        userIdGeneratorStrategy =
                 Constraint.isNotNull(strategy, "Challenge Generator cannot be null");
     }
     
@@ -100,35 +100,36 @@ public class GenerateUserHandle extends AbstractWebAuthnRegistrationAction {
     @Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final WebAuthnRegistrationContext context) {
         
+        // Any existing user handle is the user.id  
         final Optional<ByteArray> existingUserHandle = getCredentialRepository().getUserHandleForUsername(username);
-        
+              
         if (existingUserHandle.isPresent()) {
             final byte[] handleAsBytes = existingUserHandle.get().getBytes();
             assert handleAsBytes != null;
-            context.setUserHandle(handleAsBytes); 
+            context.setUserId(handleAsBytes); 
         } else {            
-            final byte[] userHandle = userHandleGeneratorStrategy.apply(profileRequestContext);
-            if (userHandle == null) {
-                log.trace("{} Generated UserHandle was null", getLogPrefix());
+            final byte[] userId = userIdGeneratorStrategy.apply(profileRequestContext);
+            if (userId == null) {
+                log.trace("{} Generated UserID was null", getLogPrefix());
                 ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
                 return;
             }
-            log.trace("{} Generated UserHandle '{}'",getLogPrefix(),userHandle);
-            context.setUserHandle(userHandle);   
+            log.trace("{} Generated UserID '{}'",getLogPrefix(),userId);
+            context.setUserId(userId);   
         }
              
     }    
     
     /**
-     * Default User Handle generator that generates a 64 byte randomized UserHandle (must be at least 32 bytes long). 
+     * Default user.id 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>
      */
-    private static final class DefaultUserHandleGenerator implements Function<ProfileRequestContext, byte[]>{
+    private static final class DefaultUserIdGenerator implements Function<ProfileRequestContext, byte[]>{
         
         /** Class logger. */
-        @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultUserHandleGenerator.class);
+        @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultUserIdGenerator.class);
 
         /** {@inheritDoc} */
         @Override
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 6ca356f..2b2e245 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
@@ -108,9 +108,9 @@ public class CreatePublicKeyCredentialCreationOptions extends AbstractWebAuthnRe
             ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
             return;
         }
-        final byte[] userHandle = context.getUserHandle();
-        if (userHandle == null) {
-            log.error("{} UserHandle is null",getLogPrefix());
+        final byte[] userId = context.getUserId();
+        if (userId == null) {
+            log.error("{} UserID is null",getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
             return;
         }
@@ -137,7 +137,8 @@ public class CreatePublicKeyCredentialCreationOptions extends AbstractWebAuthnRe
                     .withExcludeCredentials(existingCredentialDescriptors)
                     .withUsername(username)
                     .withResidentKeyRequirement(residentKeyRequirement)
-                    .withUserHandle(userHandle)
+                    // Set the userId as the user handle
+                    .withUserHandle(userId)
                     .withAttestationConveyancePreference(attestationPreference)
                     .withAuthenticatorAttachment(
                             context.getAuthenticatorAttachmentRequirement())
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java
index 25a6f20..76497ad 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java
@@ -106,16 +106,17 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
             return;
         } 
         try {
+            //TODO does userHandle come back from the attestation (registration) result. Do not think so.
             final RegisteredCredential credential = RegisteredCredential.builder()
                     .credentialId(registrationResult.getKeyId().getId())
-                    .userHandle(new ByteArray(context.getUserHandle()))
+                    .userHandle(new ByteArray(context.getUserId()))
                     .publicKeyCose(registrationResult.getPublicKeyCose())
                     .build();
             
             final UserIdentity user = UserIdentity.builder()
                     .name(username)
                     .displayName(username)
-                    .id(new ByteArray(context.getUserHandle()))
+                    .id(new ByteArray(context.getUserId()))
                     .build();
             
             final CredentialRegistration registration = CredentialRegistration.builder()
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClient.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClient.java
index b11419d..bc485a5 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClient.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClient.java
@@ -68,7 +68,7 @@ public class YubicoWebauthnAuthenticationClient implements WebAuthnAuthenticatio
     @Nonnull private final RelyingParty rp;
     
     /** List of acceptable public key algorithms.*/
-    private final List<PublicKeyCredentialParameters> preferredPublickeyParams;
+    @Nonnull @NonnullElements @NotLive private final List<PublicKeyCredentialParameters> preferredPublickeyParams;
     
     /**
      * 
@@ -150,7 +150,7 @@ public class YubicoWebauthnAuthenticationClient implements WebAuthnAuthenticatio
 
     @Override
     public AssertionResult validateAuthenticatorAssertionResponse(@Nullable final String username, 
-            @Nullable final byte[] userHandle,
+            @Nullable final byte[] userId,
             @Nonnull final PublicKeyCredentialRequestOptions publicKeyCredentialRequestOptions,
             @Nonnull final PublicKeyCredential<AuthenticatorAssertionResponse, ClientAssertionExtensionOutputs> 
             authenticatorAssertionResponse) throws AssertionFailureException {
@@ -159,16 +159,17 @@ public class YubicoWebauthnAuthenticationClient implements WebAuthnAuthenticatio
             
             final AssertionRequest requestAssertion = AssertionRequest.builder()
                 .publicKeyCredentialRequestOptions(publicKeyCredentialRequestOptions)
-                .userHandle(Optional.ofNullable(userHandle != null ? new ByteArray(userHandle) : null))
+                //TODO userHandle will always be null here. Check this?
+                .userHandle(Optional.ofNullable(userId != null ? new ByteArray(userId) : null))
                 .username(Optional.ofNullable(username))
                 .build();
   
-            if (username == null && userHandle == null) {
+            if (username == null && userId == null) {
                 log.debug("Attempting validation of assumed discoverable credential with userHandle from response '{}'",
                         authenticatorAssertionResponse.getResponse().getUserHandle());
             } else {
                 log.debug("Attempting validation of credential with known username '{}' and userHandle '{}'",
-                        username, userHandle);
+                        username, userId);
             }
 
             final AssertionResult result = rp.finishAssertion(FinishAssertionOptions.builder()
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnClientFactory.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnClientFactory.java
index 0c7c4fa..fe5065a 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnClientFactory.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnClientFactory.java
@@ -156,7 +156,7 @@ public class YubicoWebauthnClientFactory extends AbstractInitializableComponent
             @Nonnull @NonnullElements final List<String> publickeyParams) {
         checkSetterPreconditions();
         final Collection<String> publicKeyParamsNormalized = StringSupport.normalizeStringCollection(publickeyParams);
-        preferredPublickeyParams = publicKeyParamsNormalized.stream()
+        preferredPublickeyParams =CollectionSupport.copyToList(publicKeyParamsNormalized.stream()
             .map(coseAlg -> {
                 switch (coseAlg) {
                     case "EdDSA" : 
@@ -180,7 +180,7 @@ public class YubicoWebauthnClientFactory extends AbstractInitializableComponent
             })
             .filter(Objects::nonNull)
             .collect(CollectionSupport.nonnullCollector(Collectors.toList()))
-            .get();
+            .get());
     }
     
     /**
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/ValidateWebAuthnAssertion.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/ValidateWebAuthnAssertion.java
index 2165d30..25312b9 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/ValidateWebAuthnAssertion.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/ValidateWebAuthnAssertion.java
@@ -119,7 +119,7 @@ public class ValidateWebAuthnAssertion extends AbstractValidationAction {
         try {
             
             final AssertionResult result = webAuthnClient.validateAuthenticatorAssertionResponse(
-                    context.getUsername(), context.getUserHandle(), publicKeyCredentialRequestOptions, assertion);
+                    context.getUsername(), context.getUserId(), publicKeyCredentialRequestOptions, assertion);
             
             log.info("{} WebAuthn authentication succeeded for '{}'",getLogPrefix(),result.getUsername());
             context.setUsername(result.getUsername());
diff --git a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
index dde01ff..34f82ac 100644
--- a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
@@ -52,8 +52,8 @@
         class="net.shibboleth.idp.plugin.authn.webauthn.impl.GenerateServerChallenge"
         p:webAuthnBaseContextLookupStrategy-ref="shibboleth.ChildLookup.WebAuthnRegistrationContext" />
 
-    <bean id="GenerateUserHandle" parent="AbstractWebAuthnRegistrationAction"
-        class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.GenerateUserHandle" />
+    <bean id="AddUserId" parent="AbstractWebAuthnRegistrationAction"
+        class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.AddUserId" />
 
     <bean id="CreatePublicKeyCredentialCreationOptions" parent="AbstractWebAuthnRegistrationAction"
         class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.CreatePublicKeyCredentialCreationOptions"
diff --git a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml
index a0633da..04e4cfa 100644
--- a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-flow.xml
@@ -49,7 +49,7 @@
     
      <action-state id="GeneratePublicKeyCredentialCreationOptions">        
         <evaluate expression="GenerateServerChallenge"/>
-        <evaluate expression="GenerateUserHandle"/>
+        <evaluate expression="AddUserId"/>
         <evaluate expression="AddResidentKeyRequirement"/>
         <evaluate expression="AddAuthenticatorAttachmentRequirement"/>
          <evaluate expression="AddAttestationConveyancePreference"/>
diff --git a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties
index 37e3021..3963157 100644
--- a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties
+++ b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/conf/authn/webauthn.properties
@@ -34,7 +34,7 @@ idp.authn.webauthn.supportedPrincipals = \
 # Registration properties.
 
 ### Which authentication flows to require 
-idp.authn.webauthn.admin.registration.defaultAuthenticationMethods=saml2/urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport
+#idp.authn.webauthn.admin.registration.defaultAuthenticationMethods=saml2/urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport
 
 # Require a residentKey (passkey) to be created when registering a credential. One-of 'discouraged', 'preferred', 'required'
 #idp.authn.webauthn.registration.residentKey = preferred

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


More information about the commits mailing list