[java-idp-plugin-webauthn] branch main updated: Allow configuration of username collection in the registration flow

Phil Smart philip.smart at jisc.ac.uk
Wed May 15 14:50:11 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=a859f57a516a67f7fdf3a2e53ea82cf2dfbbc814

The following commit(s) were added to refs/heads/main by this push:
     new a859f57  Allow configuration of username collection in the registration flow
a859f57 is described below

commit a859f57a516a67f7fdf3a2e53ea82cf2dfbbc814
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed May 15 15:50:09 2024 +0100

    Allow configuration of username collection in the registration flow
    
     - Add the ability to disable username collection on the registration
    flow. If disabled, all flow decisions can come from the authentication
    flow e.g. triggering an even if the user has no credentials or the
    userHandle does not match to a user with existing credentials.
     - Allows possibly a simplified MFA configuration when trying to use
    both WebAuthn and another 'fallback' flow.
---
 .../logic/IsAdminUsernameCollectionEnabled.java    | 73 ++++++++++++++++++++++
 .../impl/AllowCurrentUserAccessPredicate.java      | 49 +++++++++++----
 .../webauthn-registration-beans.xml                |  5 +-
 .../webauthn-registration-flow.xml                 |  9 ++-
 .../authn/webauthn/conf/authn/webauthn.properties  |  3 +
 .../authn/webauthn/views/webauthn-register.vm      |  2 +-
 6 files changed, 126 insertions(+), 15 deletions(-)

diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/logic/IsAdminUsernameCollectionEnabled.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/logic/IsAdminUsernameCollectionEnabled.java
new file mode 100644
index 0000000..88e6281
--- /dev/null
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/logic/IsAdminUsernameCollectionEnabled.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.idp.plugin.authn.webauthn.context.logic;
+
+import java.util.function.Predicate;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+
+import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
+import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.logic.PredicateSupport;
+import net.shibboleth.shared.primitive.LoggerFactory;
+
+/**
+ * A predicate that determines if username collection is required as a first step to the registration process. This is
+ * determined either from a static flag, or a dynamic function.
+ */
+public class IsAdminUsernameCollectionEnabled extends AbstractIdentifiableInitializableComponent
+                    implements Predicate<ProfileRequestContext>  {
+    
+    /** Class logger. */
+    @Nonnull private final Logger log = LoggerFactory.getLogger(IsDiscoverableCredentialRequired.class);
+    
+    /** The predicate that determines if username collection is required.*/
+    private Predicate<ProfileRequestContext> usernameCollectionRequiredPredicate;
+    
+    /**
+     * Set the predicate that determines if username collection is required.
+     * 
+     * @param predicate is username collection required, predicate.
+     */
+    public void setUsernameCollectionRequiredPredicate(
+            final Predicate<ProfileRequestContext> predicate) {
+        checkSetterPreconditions();
+        usernameCollectionRequiredPredicate = Constraint.isNotNull(usernameCollectionRequiredPredicate,
+                "usernameCollectionRequiredPredicate can not be null");
+    }
+    
+    /**
+     * Set the flag that determines if username collection is required.
+     * 
+     * @param flag is username collection required?.
+     */
+    public void setUsernameCollectionRequired(final boolean flag) {
+        checkSetterPreconditions();
+        usernameCollectionRequiredPredicate = flag ? PredicateSupport.alwaysTrue() : PredicateSupport.alwaysFalse();
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public boolean test(@Nullable final ProfileRequestContext input) {
+        final boolean usernameCollectionRequired = usernameCollectionRequiredPredicate.test(input);
+        log.trace("{}: Username collection was {}'",getId(), usernameCollectionRequired ? "required" : "not required");
+        return usernameCollectionRequired;
+    }
+    
+}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java
index 290a0f5..94299de 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java
@@ -25,7 +25,9 @@ import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 
+import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.authn.context.SubjectContext;
+import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnAuthenticationContext;
 import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.collection.Pair;
@@ -35,8 +37,8 @@ import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * An access control predicate that should implement comparison logic to allow access only to authenticated users who 
- * initiate the WebAuthn registration process. If the authenticated user is not the same as the user who started the 
- * registration process (as determined by the initial username collection step), access should be denied. 
+ * initiate the WebAuthn registration/authentication process. If the authenticated user is not the same as the user who 
+ * started the registration/authentication process (as determined by the initial username collection step), access should be denied. 
  * 
  * <p>It is important to prevent users from changing their username between the registration and authentication 
  * flows. If this is allowed, it would create a loophole in the WebAuthn authentication process, which could 
@@ -58,6 +60,9 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
     @Nonnull 
     private Function<ProfileRequestContext,WebAuthnRegistrationContext> webauthnRegistrationContextLookupStrategy;
     
+    /** Lookup strategy to locate the WebAuthn authentication context. */
+    @Nonnull private Function<ProfileRequestContext,WebAuthnAuthenticationContext> webauthnContextLookupStrategy;
+    
     /** Strategy function to lookup SubjectContext. */
     @Nonnull private Function<ProfileRequestContext,SubjectContext> subjectContextLookupStrategy;
     
@@ -67,9 +72,24 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
     /** Constructor.*/
     public AllowCurrentUserAccessPredicate() {
         webauthnRegistrationContextLookupStrategy = new ChildContextLookup<>(WebAuthnRegistrationContext.class);
+        webauthnContextLookupStrategy = new ChildContextLookup<>(WebAuthnAuthenticationContext.class).
+                compose(new ChildContextLookup<>(AuthenticationContext.class));
         subjectContextLookupStrategy = new ChildContextLookup<>(SubjectContext.class);
         comparisonPredicate = new DefaultCurrentUserComparisonPredicate();
     }
+    
+    /**
+     * Set the WebAuthn authentication context lookup strategy to use.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setWebauthnContextLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext,WebAuthnAuthenticationContext> strategy) {
+        checkSetterPreconditions();
+
+        webauthnContextLookupStrategy = 
+                Constraint.isNotNull(strategy, "WebAuthnContextLookuplookup strategy cannot be null");
+    }
 
     /**
      * Set the lookup strategy to use to locate the {@link SubjectContext}.
@@ -116,12 +136,17 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
         final WebAuthnRegistrationContext regContext = 
                 webauthnRegistrationContextLookupStrategy.apply(profileRequestContext);
         
-        if (regContext == null) {
-            log.debug("{}: Registration context not found, access requires a registration context", getId());
+        final WebAuthnAuthenticationContext webAuthnContext = 
+                webauthnContextLookupStrategy.apply(profileRequestContext);
+        
+        if (regContext == null && webAuthnContext == null) {
+            log.debug("{}: Registration or authentication context not found, access requires either a registration or "
+                    + "authentication context", getId());
             return false;
         }
         
-        final String usernameFromRegistrationContext = regContext.getUsername();
+        // Prioritise the registration context
+        final String usernameFromContext = regContext != null ? regContext.getUsername() : webAuthnContext.getUsername();
         
         final SubjectContext subjectContext = subjectContextLookupStrategy.apply(profileRequestContext);
         if (subjectContext == null) {
@@ -131,7 +156,7 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
         final String usernameFromSubjectContext = subjectContext.getPrincipalName();
         
         return comparisonPredicate.test(profileRequestContext, 
-                new Pair<>(usernameFromSubjectContext, usernameFromRegistrationContext));
+                new Pair<>(usernameFromSubjectContext, usernameFromContext));
 
     }
     
@@ -154,10 +179,10 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
             }
 
             final String usernameFromSubjectContext = usernamePair.getFirst();
-            final String usernameFromRegistrationContext = usernamePair.getSecond();
+            final String usernameFromWebAuthnContext = usernamePair.getSecond();
            
-            if (usernameFromRegistrationContext == null) {
-                log.debug("No username in registration context, granting access");
+            if (usernameFromWebAuthnContext == null) {
+                log.debug("No username in WebAuthn context, granting access");
                 return true;
             }
             
@@ -165,9 +190,9 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
                 log.debug("No username in subject context, access requires authentication");
                 return false;
             }
-            final boolean match = usernameFromSubjectContext.equals(usernameFromRegistrationContext);
-            log.debug("Username in registration context '{}' {} with the authenticated principal '{}'", 
-                    usernameFromRegistrationContext, match ? "matched" : "did not match",
+            final boolean match = usernameFromSubjectContext.equals(usernameFromWebAuthnContext);
+            log.debug("Username in WebAuthn context '{}' {} with the authenticated principal '{}'", 
+                    usernameFromWebAuthnContext, match ? "matched" : "did not match",
                             usernameFromSubjectContext);
             return match;
         }
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 51b2ac7..25fcb01 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
@@ -31,7 +31,10 @@
     <bean id="PopulateInitialWebAuthnRegistrationContext" scope="prototype"
         class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.PopulateWebAuthnRegistrationContext"
         p:usernameRequired="false">
-    </bean>    
+    </bean>
+    
+    <bean id="IsAdminUsernameCollectionEnabled" class="net.shibboleth.idp.plugin.authn.webauthn.context.logic.IsAdminUsernameCollectionEnabled"
+        p:usernameCollectionRequired="%{idp.authn.webauthn.registration.collectUsername:true}"/>  
         
     <bean id="ExtractUsernameFromForm" scope="prototype"
         class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.ExtractUsernameFromRegistrationForm"
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 1afa84e..5caf889 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
@@ -10,8 +10,15 @@
         <evaluate expression="FlowStartPopulateAuditContext" />
         <evaluate expression="'proceed'" />        
 
-        <transition on="proceed" to="CollectUsernameView" />
+        <transition on="proceed" to="DecideIfUsernameCollectionIsRequired" />
     </action-state>
+    
+    <!-- Has username collection been configured -->
+    <decision-state id="DecideIfUsernameCollectionIsRequired">
+        <if test="IsAdminUsernameCollectionEnabled.test(opensamlProfileRequestContext)"
+            then="CollectUsernameView" 
+            else="DoAdminPreamble" />   
+    </decision-state>
 
     
     <view-state id="CollectUsernameView" view="webauthn/webauthn-register-username">
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 584f137..93fe889 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
@@ -36,6 +36,9 @@ idp.authn.webauthn.supportedPrincipals = \
 
 #### Registration properties.
 
+# Should the registration process collect a username before initiating authentication?
+#idp.authn.webauthn.registration.collectUsername = true
+
 # Does the registration page require authentication. Should always be true.
 #idp.authn.webauthn.admin.registration.authenticate = true
 # Access policy for the registration flow
diff --git a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm
index 5f3ef40..1ac6697 100644
--- a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm
+++ b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm
@@ -118,7 +118,7 @@
                         <p id="error_message"></p>
                   </div>                
                   <div>
-                     <h1>Registered Keys</h1>
+                     <h1>'$encoder.encodeForHTML($webauthnRegContext.username)' #springMessageText("idp.webauthn.register.registered.keys","Registered Keys")</h1>
                      #if ($webauthnRegContext.existingCredentials)
                          <table>
                             <tr>

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


More information about the commits mailing list