[java-idp-plugin-duo] branch main updated: JDUO-78 - Persist and expose Duo claims response

Phil Smart philip.smart at jisc.ac.uk
Fri Nov 10 14:12:17 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-duo.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=171e195daa96dfceee22041b2be70486d80641b3

The following commit(s) were added to refs/heads/main by this push:
     new 171e195  JDUO-78 - Persist and expose Duo claims response
171e195 is described below

commit 171e195daa96dfceee22041b2be70486d80641b3
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Nov 10 14:12:09 2023 +0000

    JDUO-78 - Persist and expose Duo claims response
    
     - Add new DuoFactorPrincipal
     - Add duoFactor SimplePrincipalSerializer in config
     - Create DuoFactorPrincipal from the 'factor' returned from Duo
    
    https://shibboleth.atlassian.net/browse/JDUO-78
---
 .../{DuoPrincipal.java => DuoFactorPrincipal.java} | 39 ++++++++++----------
 .../net/shibboleth/idp/authn/duo/DuoPrincipal.java |  9 ++---
 .../impl/ValidateDuoTokenAuthenticationResult.java | 42 +++++++++++++---------
 .../META-INF/net.shibboleth.idp/postconfig.xml     | 18 +++++-----
 pom.xml                                            |  2 +-
 5 files changed, 61 insertions(+), 49 deletions(-)

diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoPrincipal.java b/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoFactorPrincipal.java
similarity index 61%
copy from idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoPrincipal.java
copy to idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoFactorPrincipal.java
index 97253ed..1c7b241 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoPrincipal.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoFactorPrincipal.java
@@ -16,38 +16,38 @@ package net.shibboleth.idp.authn.duo;
 
 import javax.annotation.Nonnull;
 
+import com.google.common.base.MoreObjects;
+
 import net.shibboleth.idp.authn.principal.CloneablePrincipal;
 import net.shibboleth.shared.annotation.ParameterName;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
 
-import com.google.common.base.MoreObjects;
-
-/** Principal based on a Duo authentication. */
-public class DuoPrincipal implements CloneablePrincipal {
-
-    /** The username. */
-    @Nonnull @NotEmpty private String username;
+/** Principal based on the Duo authentication factor used for 2FA. */
+public class DuoFactorPrincipal implements CloneablePrincipal {
+    
+    /** The authentication factor used.*/
+    @Nonnull @NotEmpty private String factor;
 
     /**
      * Constructor.
      * 
-     * @param name the username
+     * @param duoFactor the second authentication factor used
      */
-    public DuoPrincipal(@Nonnull @NotEmpty @ParameterName(name="name") final String name) {
-        username = Constraint.isNotNull(StringSupport.trimOrNull(name), "Username cannot be null or empty");
+    public DuoFactorPrincipal(@Nonnull @NotEmpty @ParameterName(name="factor") final String duoFactor) {
+        factor = Constraint.isNotNull(StringSupport.trimOrNull(duoFactor), "DuoFactor cannot be null or empty");
     }
 
     /** {@inheritDoc} */
     @Nonnull @NotEmpty public String getName() {
-        return username;
+        return factor;
     }
-
+    
     /** {@inheritDoc} */
     @Override
     public int hashCode() {
-        return username.hashCode();
+        return factor.hashCode();
     }
 
     /** {@inheritDoc} */
@@ -61,8 +61,8 @@ public class DuoPrincipal implements CloneablePrincipal {
             return true;
         }
 
-        if (other instanceof DuoPrincipal) {
-            return username.equals(((DuoPrincipal) other).getName());
+        if (other instanceof final DuoFactorPrincipal principalOther) {
+            return factor.equals(principalOther.getName());
         }
 
         return false;
@@ -71,13 +71,14 @@ public class DuoPrincipal implements CloneablePrincipal {
     /** {@inheritDoc} */
     @Override
     public String toString() {
-        return MoreObjects.toStringHelper(this).add("username", username).toString();
+        return MoreObjects.toStringHelper(this).add("factor", factor).toString();
     }
     
     /** {@inheritDoc} */
-    @Nonnull public DuoPrincipal clone() throws CloneNotSupportedException {
-        final DuoPrincipal copy = (DuoPrincipal) super.clone();
-        copy.username = username;
+    @Override
+    @Nonnull public DuoFactorPrincipal clone() throws CloneNotSupportedException {
+        final DuoFactorPrincipal copy = (DuoFactorPrincipal) super.clone();
+        copy.factor = factor;
         return copy;
     }
 }
\ No newline at end of file
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoPrincipal.java b/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoPrincipal.java
index 97253ed..922762f 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoPrincipal.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/authn/duo/DuoPrincipal.java
@@ -16,14 +16,14 @@ package net.shibboleth.idp.authn.duo;
 
 import javax.annotation.Nonnull;
 
+import com.google.common.base.MoreObjects;
+
 import net.shibboleth.idp.authn.principal.CloneablePrincipal;
 import net.shibboleth.shared.annotation.ParameterName;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
 
-import com.google.common.base.MoreObjects;
-
 /** Principal based on a Duo authentication. */
 public class DuoPrincipal implements CloneablePrincipal {
 
@@ -61,8 +61,8 @@ public class DuoPrincipal implements CloneablePrincipal {
             return true;
         }
 
-        if (other instanceof DuoPrincipal) {
-            return username.equals(((DuoPrincipal) other).getName());
+        if (other instanceof final DuoPrincipal otherPrincipal) {
+            return username.equals(otherPrincipal.getName());
         }
 
         return false;
@@ -75,6 +75,7 @@ public class DuoPrincipal implements CloneablePrincipal {
     }
     
     /** {@inheritDoc} */
+    @Override
     @Nonnull public DuoPrincipal clone() throws CloneNotSupportedException {
         final DuoPrincipal copy = (DuoPrincipal) super.clone();
         copy.username = username;
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java
index 2e68a72..7a5dc34 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResult.java
@@ -37,6 +37,7 @@ import net.shibboleth.idp.authn.AuthenticationResult;
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.authn.context.SubjectCanonicalizationContext;
+import net.shibboleth.idp.authn.duo.DuoFactorPrincipal;
 import net.shibboleth.idp.authn.duo.DuoPrincipal;
 import net.shibboleth.idp.authn.impl.AbstractAuditingValidationAction;
 import net.shibboleth.idp.plugin.authn.duo.DuoException;
@@ -202,13 +203,15 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractAuditingValida
         final Object statusMsgObj = authStatusObject.get(DuoOIDCAuthAPI.DUO_AUTH_RESULT_STATUS_MSG_JSON_OBJECT);
         
         //instanceof includes null check
-        if (statusObj instanceof String && statusMsgObj instanceof String) {
-            final String authResultStatus = (String)statusObj;
-            final String authResultStatusMsg = (String)statusMsgObj;
+        if (statusObj instanceof final String authResultStatus && 
+                statusMsgObj instanceof final String authResultStatusMsg) {
             
             if (DuoOIDCAuthAPI.DUO_AUTH_RESULT_ALLOW.equalsIgnoreCase(authResultStatus)){
-                log.info("{} Duo 2FA authentication succeeded for '{}', using second-factor '{}'",
-                        getLogPrefix(),duoContext.getUsername(), extractFactor());
+                if (log.isInfoEnabled()) {
+                    final String factorUsed = extractFactor();
+                    log.info("{} Duo 2FA authentication succeeded for '{}', using second-factor '{}'",
+                            getLogPrefix(),duoContext.getUsername(), factorUsed != null ? factorUsed : "unspecified");
+                }
                 //must build authentication before recording success. recordSuccess runs
                 //the cleanup hook which removes the Duo context and prevents useful operation
                 //of the contextToPrincipalMappingStrategy.
@@ -240,14 +243,13 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractAuditingValida
     }
     
     /**
-     * Extract the second-factor use for authentication. Taken from the auth_context. Will return
-     * {@literal 'unspecified'} if not found.
+     * Extract the second-factor used for authentication as taken from the auth_context. Will return
+     * {@code null} if not found.
      * 
-     * @return the second-factor used, or {@literal 'unspecified'} if not found. Should always be found.
+     * @return the second-factor used, or {@code null} if not found. Should always be found.
      */
-     @Nonnull private String extractFactor() {
+     @Nullable private String extractFactor() {
         
-        String factor = "unspecified";
         try {
             final Map<String, Object> authnContextClaimObj = 
                     claimsSet.getJSONObjectClaim(DuoOIDCAuthAPI.DUO_AUTH_CONTEXT_JSON_OBJECT);
@@ -255,22 +257,30 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractAuditingValida
             if (authnContextClaimObj != null) {         
                 final Object factorClaimObj = 
                         authnContextClaimObj.get(DuoOIDCAuthAPI.DUO_AUTH_FACTOR_JSON_OBJECT);
-                if (factorClaimObj instanceof String) {
-                    factor = (String)factorClaimObj;
+                if (factorClaimObj instanceof final String factor) {
+                    return factor;
                 }
             }
         } catch (final ParseException e) {
-            // Do nothing, just return unknown
+            // Do nothing, just return null
         }
-        return factor;
+        return null;
 
     }
     
     /** {@inheritDoc} */
     @Override protected Subject populateSubject(@Nonnull final Subject subject) {
         
-        // Always add the custom Duo principal
-        subject.getPrincipals().add(new DuoPrincipal(username));
+        // Always add the DuoPrincipal and the DuoFactorPrincipal if present
+        final String factor = extractFactor();
+        final String localUsername = username;
+        assert localUsername != null;
+        subject.getPrincipals().add(new DuoPrincipal(localUsername));
+        if (factor != null) {
+            subject.getPrincipals().add(new DuoFactorPrincipal(factor));
+            log.trace("{} Added DuoFactorPrincipal '{}' to DuoPrincipal '{}'", getLogPrefix(), factor, localUsername);
+        }
+
         // Always add any principals specified on the integration
         subject.getPrincipals().addAll(duoIntegration.getSupportedPrincipals(Principal.class));
         
diff --git a/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml b/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
index 0e3fa3d..08bb665 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
@@ -39,18 +39,18 @@
                 c:claz="net.shibboleth.idp.authn.duo.DuoPrincipal" c:name="DUO" />
         </constructor-arg>
     </bean>
-
-    <!-- Controller implementation -->
-    <bean id="shibboleth.DuoOIDCAuthnController"
-        class="net.shibboleth.idp.plugin.authn.duo.impl.DuoOIDCAuthnController" />
-        
-    <bean p:id="duo" class="net.shibboleth.idp.authn.principal.GenericPrincipalService"
-            c:claz="net.shibboleth.idp.authn.duo.DuoPrincipal">
+   
+    <bean p:id="duoFactor" class="net.shibboleth.idp.authn.principal.GenericPrincipalService"
+            c:claz="net.shibboleth.idp.authn.duo.DuoFactorPrincipal">
         <constructor-arg name="serializer">
             <bean class="net.shibboleth.idp.authn.principal.SimplePrincipalSerializer"
-                c:claz="net.shibboleth.idp.authn.duo.DuoPrincipal" c:name="DUO" />
+                c:claz="net.shibboleth.idp.authn.duo.DuoFactorPrincipal" c:name="DUOFACTOR" />
         </constructor-arg>
     </bean>    
-        
+
+    <!-- Controller implementation -->
+    <bean id="shibboleth.DuoOIDCAuthnController"
+        class="net.shibboleth.idp.plugin.authn.duo.impl.DuoOIDCAuthnController" />
+
 
 </beans>
\ No newline at end of file
diff --git a/pom.xml b/pom.xml
index d959ce1..fb381f6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -26,7 +26,7 @@
         <opensaml.groupId>org.opensaml</opensaml.groupId>
         <opensaml.version>5.0.0</opensaml.version>
         <oidc-common.groupId>net.shibboleth.oidc</oidc-common.groupId>
-        <oidc-common.version>3.0.1-SNAPSHOT</oidc-common.version>
+        <oidc-common.version>3.0.0</oidc-common.version>
         <shib-shared.groupId>net.shibboleth</shib-shared.groupId>
         <shib-shared.version>9.0.0</shib-shared.version>
         <shib-profile.groupId>net.shibboleth</shib-profile.groupId>

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


More information about the commits mailing list