[java-idp-plugin-webauthn] branch main updated: Add additional UI outputs for registered keys

Phil Smart philip.smart at jisc.ac.uk
Fri Dec 15 11:50:27 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=d5dd5c8f5a2bc8928b6330539fb03b96e9a363d2

The following commit(s) were added to refs/heads/main by this push:
     new d5dd5c8  Add additional UI outputs for registered keys
d5dd5c8 is described below

commit d5dd5c8f5a2bc8928b6330539fb03b96e9a363d2
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Dec 15 11:50:25 2023 +0000

    Add additional UI outputs for registered keys
    
     - If a discoverable credential or unknown
     - If the user was verified during registration
     - Also fix issue not setting RP name
---
 .../webauthn/storage/CredentialRegistration.java   | 24 +++++++++++++++++++++-
 .../admin/impl/StorePublicKeyCredential.java       | 12 ++++++++---
 .../impl/YubicoWebauthnAuthenticationClient.java   | 15 ++++++++++----
 .../client/impl/YubicoWebauthnClientFactory.java   |  3 +--
 .../PopulateWebAuthnAuthenticationContext.java     |  1 +
 .../META-INF/net.shibboleth.idp/postconfig.xml     |  2 +-
 .../authn/webauthn/conf/authn/webauthn.properties  |  1 +
 7 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java
index 6fa19ab..3a569a2 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java
@@ -38,6 +38,7 @@ import com.yubico.webauthn.data.UserIdentity;
  */
 //TODO need our own storage record, so this should be test only and then replaced with the actual one eventually
 //TODO make this more official and undeprecate
+//TODO needs a builder
 @Deprecated
 public class CredentialRegistration {
 
@@ -45,14 +46,19 @@ public class CredentialRegistration {
     Optional<String> credentialNickname;
     @Nonnull SortedSet<AuthenticatorTransport> transports;
     Instant registrationTime;
+    /** Is the credential a discovery type (passkey). Empty if not known.*/
+    Optional<Boolean> discoverable;
     RegisteredCredential credential;
     Optional<Object> attestationMetadata;
+    /** Was the user verified during registration.*/
+    boolean userVerified;
     
     
     public CredentialRegistration(final UserIdentity userIdentity, final Optional<String> credentialNickname,
             @Nonnull final SortedSet<AuthenticatorTransport> transports, final Instant registrationTime, 
             final RegisteredCredential credential,
-            final Optional<Object> attestationMetadata) {
+            final Optional<Object> attestationMetadata, final Optional<Boolean> isDiscoverable,
+            final boolean isUserVerified) {
         super();
         this.userIdentity = userIdentity;
         this.credentialNickname = credentialNickname;
@@ -60,6 +66,20 @@ public class CredentialRegistration {
         this.registrationTime = registrationTime;
         this.credential = credential;
         this.attestationMetadata = attestationMetadata;
+        discoverable = isDiscoverable;
+        userVerified = isUserVerified;
+    }
+    
+    public boolean isUserVerified() {
+        return userVerified;
+    }
+    
+    public Optional<Boolean> isDiscoverable(){
+        return discoverable;
+    }
+    
+    public String getDiscoverableAsString(){
+        return isDiscoverable().isPresent() ? isDiscoverable().get().toString() : "unknown";
     }
     
     public String getNickname(){
@@ -112,6 +132,8 @@ public class CredentialRegistration {
         newReg.registrationTime = registrationTime;
         newReg.transports = transports;
         newReg.credentialNickname = credentialNickname;
+        newReg.discoverable = discoverable;
+        newReg.userVerified = userVerified;
         // With the new credential
         newReg.credential = newRegCred;
         return newReg;
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 8edd48e..418aca3 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
@@ -131,11 +131,17 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
             
             final CredentialRegistration registration = new CredentialRegistration(user, 
                     Optional.of(context.getCredentialNickname()), 
-                    transports, Instant.now(), credential, Optional.empty());
+                    transports, Instant.now(), credential, Optional.empty(), registrationResult.isDiscoverable(),
+                    registrationResult.isUserVerified());
             
             getCredentialRepository().addRegistrationByUsername(username, registration);
-            log.debug("{} Added public key credential registration for user '{}' and key '{}' ", 
-                    getLogPrefix(), username, registrationResult.getKeyId().getId().getBase64Url());
+            
+            
+            log.debug("{} Added public key credential registration for user '{}' and key '{}'. Using a "
+                    + "discoverable credential '{}', and UserVerification '{}'", 
+                    getLogPrefix(), username, registrationResult.getKeyId().getId().getBase64Url(),
+                    registrationResult.isDiscoverable().isPresent() ? registrationResult.isDiscoverable() : "unknown", 
+                            registrationResult.isUserVerified());
 
         } catch (final Exception e) {
             log.error("{} Unable to store registration for key '{}'",getLogPrefix(), 
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 c181c68..28b4cdf 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
@@ -106,7 +106,7 @@ public class YubicoWebauthnAuthenticationClient implements WebAuthnAuthenticatio
         //set default to preferred.
         UserVerificationRequirement userVerificationRequirement = UserVerificationRequirement.PREFERRED;
         if (username == null) {
-            //then require user verification? makes sense, but is that part of the spec?
+            //then require user verification if no username is known. 
             userVerificationRequirement = UserVerificationRequirement.REQUIRED;
         }        
         
@@ -137,7 +137,8 @@ public class YubicoWebauthnAuthenticationClient implements WebAuthnAuthenticatio
         //set default to preferred.
         ResidentKeyRequirement residentKeyRquirement = ResidentKeyRequirement.PREFERRED;        
         if (username == null) {
-            //then require user verification? makes sense, but is that part of the spec?
+            // FIXME Fix this
+            //then require a resident key, but this can not happen during registration
             residentKeyRquirement = ResidentKeyRequirement.REQUIRED;
 
         }
@@ -189,8 +190,14 @@ public class YubicoWebauthnAuthenticationClient implements WebAuthnAuthenticatio
                 .username(Optional.ofNullable(username))
                 .build();
   
-            log.trace("Client Data '{}'",authenticatorAssertionResponse.getResponse().getClientData());
-            log.trace("Signature '{}'",authenticatorAssertionResponse.getResponse().getSignature());
+            if (username == null && userHandle == 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);
+            }
+            
             final AssertionResult result = rp.finishAssertion(FinishAssertionOptions.builder()
                     .request(requestAssertion)
                     .response(authenticatorAssertionResponse)
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 8914421..f487255 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
@@ -181,7 +181,7 @@ public class YubicoWebauthnClientFactory extends AbstractInitializableComponent
      */
     public synchronized void setRelyingPartyName(@Nonnull final String name) {
         checkSetterPreconditions(); 
-        relyingPartyName = Constraint.isNotNull("You must set a relying party name", name);
+        relyingPartyName = Constraint.isNotNull(name, "You must set a relying party name");
     }
     
     /**
@@ -209,7 +209,6 @@ public class YubicoWebauthnClientFactory extends AbstractInitializableComponent
      * 
      * @param allow allow origin port.
      */
-    //TODO Javadoc
     public synchronized void setAllowOriginPort(final boolean allow) {
         checkSetterPreconditions();
         allowOriginPort = allow;
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/PopulateWebAuthnAuthenticationContext.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/PopulateWebAuthnAuthenticationContext.java
index c6e1215..608c47e 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/PopulateWebAuthnAuthenticationContext.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/PopulateWebAuthnAuthenticationContext.java
@@ -113,6 +113,7 @@ public class PopulateWebAuthnAuthenticationContext extends AbstractAuthenticatio
         }
         
         final String username = usernameLookupStrategy.apply(profileRequestContext);
+       // username = "philsmart";
         if (username == null && usernameRequiredPredicate.test(profileRequestContext)) {
             log.error("{} Error creating WebauthnAuthenticationContext, no username found", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
diff --git a/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml b/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
index 89ffa59..afe940f 100644
--- a/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
@@ -88,7 +88,7 @@
     <bean id="shibboleth.authn.webauthn.DefaultWebauthnAuthenticationClientFactory" scope="singleton"
         class="net.shibboleth.idp.plugin.authn.webauthn.client.impl.YubicoWebauthnClientFactory"
         p:relyingPartyId="%{idp.authn.webauthn.relyingPartyId}" 
-        p:relyingPartyName="Shibboleth"
+        p:relyingPartyName="%{idp.authn.webauthn.relyingPartyName}"
         p:allowOriginPort ="%{idp.authn.webauthn.allowOriginPort:false}"
         p:allowOriginSubdomain ="%{idp.authn.webauthn.allowOriginSubdomain:false}"
         p:objectMapper-ref="shibboleth.authn.WebAuthn.JSONObjectMapper" 
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 b5476f2..302fe2d 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
@@ -1,6 +1,7 @@
 ## Thre relying party ID. Must be a valid domain string.
 ## A public key credential is only registered and valid for a single relying party ID.
 idp.authn.webauthn.relyingPartyId = localhost
+idp.authn.webauthn.relyingPartyName = Shibboleth
 ## Allow any port on that origin
 idp.authn.webauthn.allowOriginPort = true
 idp.authn.webauthn.allowOriginSubdomain = false
\ No newline at end of file

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


More information about the commits mailing list