[java-idp-plugin-duo] branch dev/JDUO-80 updated: Add factor filtering to validation step, auto-add of username principal.

Scott Cantor cantor.2 at osu.edu
Thu Dec 28 19:35:06 UTC 2023


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

scantor pushed a commit to branch dev/JDUO-80
in repository java-idp-plugin-duo.

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

The following commit(s) were added to refs/heads/dev/JDUO-80 by this push:
     new b47c230c Add factor filtering to validation step, auto-add of username principal.
b47c230c is described below

commit b47c230c1132249e8a4fff37e2b00ab3028afb19
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Dec 28 14:35:03 2023 -0500

    Add factor filtering to validation step, auto-add of username principal.
---
 .../authn/duo/DefaultDuoOIDCIntegration.java       | 31 ++++++++--
 .../idp/plugin/authn/duo/DuoOIDCIntegration.java   | 17 +++++-
 .../authn/duo/DynamicDuoOIDCIntegration.java       |  1 -
 .../impl/ValidateDuoTokenAuthenticationResult.java | 38 +++++++++---
 .../authn/duo/impl/AbstractDuoActionTest.java      | 33 +++++++++--
 .../duo/impl/DefaultDuoOIDCClientRegistryTest.java |  1 +
 .../DefaultRedirectURICreationStrategyTest.java    |  1 +
 .../impl/DuoAudienceClaimLookupStrategyTest.java   |  1 +
 .../duo/impl/DuoIssuerClaimLookupStrategyTest.java |  1 +
 .../duo/impl/DuoNonceClaimLookupStrategyTest.java  |  3 +-
 .../idp/plugin/authn/duo/impl/DuoSupportTest.java  |  1 +
 .../impl/DuoUsernameClaimLookupStrategyTest.java   |  5 +-
 .../duo/impl/ExchangeCodeForDuoTokenTest.java      |  1 +
 .../duo/impl/ValidateDuoResponseStateTest.java     |  1 +
 .../ValidateDuoTokenAuthenticationResultTest.java  | 67 +++++++++++++++++-----
 .../authn/duo/impl/ValidateTokenClaimsTest.java    |  1 +
 .../authn/duo/impl/ValidateTokenSignatureTest.java |  1 +
 17 files changed, 165 insertions(+), 39 deletions(-)

diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java
index 4f189459..936df113 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultDuoOIDCIntegration.java
@@ -74,7 +74,10 @@ public final class DefaultDuoOIDCIntegration
     @GuardedBy("this") @Nullable private String registeredRedirectURI;
     
     /** A set of 'allowed' origins that can be used as the scheme, host, and port portion of the redirectURI.*/
-    @GuardedBy("this") @Nonnull @NonnullElements @Unmodifiable private Set<String> allowedOrigins;
+    @GuardedBy("this") @Nonnull @NonnullElements private Set<String> allowedOrigins;
+    
+    /** A set of 'allowed' factors. */
+    @GuardedBy("this") @Nullable @NonnullElements private Set<String> allowedFactors;
     
     /** The URL path to the health endpoint.*/
     @GuardedBy("this") @NonnullAfterInit @NotEmpty private String healthEndpoint;
@@ -120,15 +123,31 @@ public final class DefaultDuoOIDCIntegration
      * @param hosts the hostnames to allow.
      */
     public synchronized void setAllowedOrigins(@Nullable @NonnullElements final Collection<String> hosts) {
-        checkSetterPreconditions();        
-        allowedOrigins = CollectionSupport.copyToSet(StringSupport.normalizeStringCollection(
-                Constraint.isNotNull(hosts, "Types cannot be null")));
+        checkSetterPreconditions();
+        allowedOrigins = CollectionSupport.copyToSet(StringSupport.normalizeStringCollection(hosts));
     }
     
     /** {@inheritDoc} */
     @Nonnull @NotLive @Unmodifiable public synchronized Set<String> getAllowedOrigins() {
-        //set is unmodifiable and string is immutable - so not live. 
-        return CollectionSupport.copyToSet(allowedOrigins);
+        return allowedOrigins;
+    }
+    
+    /**
+     * Set the allowable factors.
+     * 
+     * @param factors the factors to allow
+     * 
+     * @since 2.1.0
+     */
+    public synchronized void setAllowedFactors(@Nullable @NonnullElements final Collection<String> factors) {
+        checkSetterPreconditions();
+        allowedFactors = factors != null ?
+                CollectionSupport.copyToSet(StringSupport.normalizeStringCollection(factors)) : null;
+    }
+    
+    /** {@inheritDoc} */
+    @Nullable @NotLive @Unmodifiable public synchronized Set<String> getAllowedFactors() {
+        return allowedFactors;
     }
 
     /** {@inheritDoc} */
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
index 9d49885e..4bf6ec95 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoOIDCIntegration.java
@@ -14,12 +14,16 @@
 
 package net.shibboleth.idp.plugin.authn.duo;
 
+import java.util.Set;
+
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import net.shibboleth.idp.authn.principal.PrincipalSupportingComponent;
+import net.shibboleth.shared.annotation.constraint.NonnullElements;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
-
+import net.shibboleth.shared.annotation.constraint.NotLive;
+import net.shibboleth.shared.annotation.constraint.Unmodifiable;
 
 /**
  * Interface to a particular Duo OIDC integration point. In part replaces
@@ -91,4 +95,15 @@ public interface DuoOIDCIntegration extends PrincipalSupportingComponent {
         return false;
     }
 
+    /**
+     * Gets the set of allowable factors to enforce during validation.
+     * 
+     * @return allowable factors, or null for any
+     * 
+     * @since 2.1.0
+     */
+    default @Nullable @NonnullElements @Unmodifiable @NotLive Set<String> getAllowedFactors() {
+        return null;
+    }
+    
 }
\ No newline at end of file
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DynamicDuoOIDCIntegration.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DynamicDuoOIDCIntegration.java
index 3b013f5a..44d02758 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DynamicDuoOIDCIntegration.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DynamicDuoOIDCIntegration.java
@@ -53,7 +53,6 @@ public interface DynamicDuoOIDCIntegration extends DuoOIDCIntegration{
      */
     boolean isRedirectURIPreregistered();
     
-    
     /**
      * <p>Set the redirectURI from the one given in a thread-safe way.</p>
      * 
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 7e7d7dab..e64d8826 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
@@ -18,6 +18,7 @@ import java.security.Principal;
 import java.text.ParseException;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -39,6 +40,7 @@ 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.authn.principal.UsernamePrincipal;
 import net.shibboleth.idp.plugin.authn.duo.DuoException;
 import net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI;
 import net.shibboleth.idp.plugin.authn.duo.DuoOIDCIntegration;
@@ -85,6 +87,9 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractAuditingValida
     /** Attempted username. */
     @NonnullBeforeExec @NotEmpty private String username;
     
+    /** Factor used. */
+    @Nullable private String factorUsed;
+    
     /** Hook to map context information (often Duo factors in the Duo token) to principal collections.*/
     @Nullable private Function<ProfileRequestContext,Collection<Principal>> contextToPrincipalMappingStrategy;
     
@@ -204,11 +209,21 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractAuditingValida
                 statusMsgObj instanceof final String authResultStatusMsg) {
             
             if (DuoOIDCAuthAPI.DUO_AUTH_RESULT_ALLOW.equalsIgnoreCase(authResultStatus)){
-                if (log.isInfoEnabled()) {
-                    final String factorUsed = extractFactor();
-                    log.info("{} Duo 2FA authentication succeeded for '{}', using second-factor '{}'",
-                            getLogPrefix(),duoContext.getUsername(), factorUsed != null ? factorUsed : "unspecified");
+                factorUsed = extractFactor();
+                
+                // Check if factor is allowed.
+                final Set<String> allowedFactors = duoIntegration.getAllowedFactors();
+                if (allowedFactors != null && !allowedFactors.contains(factorUsed)) {
+                    log.error("{} Duo 2FA authentication failed for '{}', factor '{}' disallowed",getLogPrefix(),
+                            username, factorUsed);
+                    handleError(profileRequestContext, authenticationContext, AuthnEventIds.INVALID_CREDENTIALS,
+                            AuthnEventIds.INVALID_CREDENTIALS);
+                    recordFailure(profileRequestContext);
+                    return;
                 }
+                
+                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.
@@ -270,13 +285,20 @@ public class ValidateDuoTokenAuthenticationResult extends AbstractAuditingValida
     @Nonnull protected Subject populateSubject(@Nonnull final Subject subject) {
         
         // 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);
+        
+        // Add UsernamePrincipal if passwordless.
+        if (duoIntegration.isPasswordless()) {
+            subject.getPrincipals().add(new UsernamePrincipal(localUsername));
+        }
+        
+        final String localFactor = factorUsed;
+        if (localFactor != null) {
+            subject.getPrincipals().add(new DuoFactorPrincipal(localFactor));
+            log.trace("{} Added DuoFactorPrincipal '{}' to DuoPrincipal '{}'", getLogPrefix(), localFactor,
+                    localUsername);
         }
 
         // Always add any principals specified on the integration
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java
index 31a220f1..d3184a0a 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoActionTest.java
@@ -21,13 +21,13 @@ import static org.testng.Assert.fail;
 
 import java.text.ParseException;
 import java.time.Instant;
+import java.util.Set;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.opensaml.profile.context.EventContext;
 import org.opensaml.profile.context.ProfileRequestContext;
-import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.webflow.execution.Event;
 import org.springframework.webflow.execution.RequestContext;
 
@@ -42,8 +42,6 @@ import com.nimbusds.jwt.JWTClaimsSet;
 import com.nimbusds.jwt.PlainJWT;
 import com.nimbusds.jwt.SignedJWT;
 
-import jakarta.servlet.http.HttpServletRequest;
-import net.shibboleth.idp.authn.AbstractAuthenticationAction;
 import net.shibboleth.idp.authn.AuthenticationFlowDescriptor;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.plugin.authn.duo.DefaultDuoOIDCIntegration;
@@ -879,10 +877,33 @@ public abstract class AbstractDuoActionTest {
         ac.addSubcontext(dc);
     }
     
-    /** Add fabricated duo integration to the duo context. */
-    protected void addDuoIntegrationToContext() {    
+    /**
+     * Add fabricated duo integration to the duo context.
+     *  
+     * @throws ComponentInitializationException on error
+     */
+    protected void addDuoIntegrationToContext() throws ComponentInitializationException {    
+        assertNotNull(dc,"try addDuoContext() before adding the duo integration");
+        final var integ = createDummyDuoIntegration();
+        integ.initialize();
+        dc.setIntegration(integ);
+    }
+
+    /**
+     * Add fabricated duo integration to the duo context.
+     * 
+     *  @param factors allowed factors
+     * 
+     * @throws ComponentInitializationException on error
+     */
+    protected void addPasswordlessDuoIntegrationToContext(@Nullable final Set<String> factors)
+            throws ComponentInitializationException {    
         assertNotNull(dc,"try addDuoContext() before adding the duo integration");
-        dc.setIntegration(createDummyDuoIntegration());
+        final DefaultDuoOIDCIntegration integ = createDummyDuoIntegration();
+        integ.setPasswordless(true);
+        integ.setAllowedFactors(factors);
+        integ.initialize();
+        dc.setIntegration(integ);
     }
     
     /**
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java
index 3ce30685..91a8caa6 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoOIDCClientRegistryTest.java
@@ -42,6 +42,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Tests for the {@link DefaultDuoOIDCClientRegistry}.
  */
+ at SuppressWarnings("javadoc")
 public class DefaultDuoOIDCClientRegistryTest {
 
     /** The registry to test. */
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java
index c4473cc3..db988cb7 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultRedirectURICreationStrategyTest.java
@@ -44,6 +44,7 @@ import net.shibboleth.idp.plugin.authn.duo.DynamicDuoOIDCIntegration;
 
 
 /** Tests for the DefaultDuoOIDCIntegration class.*/
+ at SuppressWarnings("javadoc")
 public class DefaultRedirectURICreationStrategyTest {
     
     /** Static callback path from servlet request.*/
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java
index 10c8138d..39c0ff62 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoAudienceClaimLookupStrategyTest.java
@@ -14,6 +14,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Test for the {@link DuoAudienceClaimLookupStrategy}.
  */
+ at SuppressWarnings("javadoc")
 public class DuoAudienceClaimLookupStrategyTest extends AbstractDuoActionTest{
 
     /** The strategy.*/
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java
index 97eaca12..7f89f7db 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoIssuerClaimLookupStrategyTest.java
@@ -27,6 +27,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Test for the {@link DuoIssuerClaimLookupStrategy}.
  */
+ at SuppressWarnings("javadoc")
 public class DuoIssuerClaimLookupStrategyTest extends AbstractDuoActionTest{
 
     /** The strategy.*/
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategyTest.java
index 86766e6e..3fd592de 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoNonceClaimLookupStrategyTest.java
@@ -26,6 +26,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Test for the {@link DuoNonceClaimLookupStrategy}.
  */
+ at SuppressWarnings("javadoc")
 public class DuoNonceClaimLookupStrategyTest extends AbstractDuoActionTest{
 
     /** The strategy.*/
@@ -42,7 +43,7 @@ public class DuoNonceClaimLookupStrategyTest extends AbstractDuoActionTest{
     }
 
     @Test
-    public void applySuccess() {
+    public void applySuccess() throws ComponentInitializationException {
         addDuoContext();
         addDuoIntegrationToContext();
         dc.setNonce("testnonce");
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupportTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupportTest.java
index 0ab88a02..6424628a 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupportTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoSupportTest.java
@@ -23,6 +23,7 @@ import net.shibboleth.idp.plugin.authn.duo.DuoException;
 /** 
  * Tests for the DuoSupport class.
  */
+ at SuppressWarnings("javadoc")
 public class DuoSupportTest {
     
     @Test public void testCreateAndExtractKeyAndNonceFromState() throws DuoException {
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategyTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategyTest.java
index 76bba890..1f04204d 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategyTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DuoUsernameClaimLookupStrategyTest.java
@@ -27,6 +27,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Test for the {@link DuoUsernameClaimLookupStrategy}.
  */
+ at SuppressWarnings("javadoc")
 public class DuoUsernameClaimLookupStrategyTest extends AbstractDuoActionTest{
 
     /** The strategy.*/
@@ -43,7 +44,7 @@ public class DuoUsernameClaimLookupStrategyTest extends AbstractDuoActionTest{
     }
 
     @Test
-    public void applySuccess() {
+    public void applySuccess() throws ComponentInitializationException {
         addDuoContext();
         addDuoIntegrationToContext();
         dc.setUsername("username");
@@ -52,7 +53,7 @@ public class DuoUsernameClaimLookupStrategyTest extends AbstractDuoActionTest{
     }
     
     @Test
-    public void applyNoUsername() {
+    public void applyNoUsername() throws ComponentInitializationException {
         addDuoContext();
         addDuoIntegrationToContext();
         dc.setUsername(null);
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java
index 90d1f03c..2edfb70a 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ExchangeCodeForDuoTokenTest.java
@@ -30,6 +30,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Tests for {@link ExchangeCodeForDuoToken}.
  */
+ at SuppressWarnings("javadoc")
 public class ExchangeCodeForDuoTokenTest extends AbstractDuoActionTest {
 
     /** The action to test. */
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
index b5f5709b..563a5436 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoResponseStateTest.java
@@ -26,6 +26,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Tests for the {@link ValidateDuoResponseState} action.
  */
+ at SuppressWarnings("javadoc")
 public class ValidateDuoResponseStateTest extends AbstractDuoActionTest{
 
     /** The action to test. */
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java
index 9916c1e4..d584436c 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateDuoTokenAuthenticationResultTest.java
@@ -17,7 +17,6 @@ package net.shibboleth.idp.plugin.authn.duo.impl;
 
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 
 import java.security.Principal;
@@ -39,14 +38,18 @@ import org.testng.annotations.Test;
 
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
+import net.shibboleth.idp.authn.principal.UsernamePrincipal;
 import net.shibboleth.idp.plugin.authn.duo.DuoOIDCAuthAPI;
 import net.shibboleth.idp.plugin.authn.duo.context.DuoOIDCAuthenticationContext;
+import net.shibboleth.idp.profile.testing.ActionTestingSupport;
 import net.shibboleth.idp.saml.authn.principal.AuthnContextClassRefPrincipal;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.ComponentInitializationException;
 
 /**
  * Tests for the {@link ValidateDuoTokenAuthenticationResult} action.
  */
+ at SuppressWarnings("javadoc")
 public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionTest {
 
     /** The action to test. */
@@ -56,10 +59,8 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
     public void setUp() throws Exception {
         super.setup();
         action = new ValidateDuoTokenAuthenticationResult();
-
     }
   
-
     /**
      * Test successful execution.
      * 
@@ -76,8 +77,51 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
         action.initialize();
         
         final Event event = action.execute(src);
-        // success here is a null event
-        assertNull(event);
+        ActionTestingSupport.assertProceedEvent(event);
+    }
+    
+    /**
+     * Test successful execution with factor enforcement.
+     * 
+     * @throws ComponentInitializationException on error.
+     */
+    @Test
+    public void testExecuteSuccessWithFactorCheck() throws ComponentInitializationException {
+        addDuoContext();
+        addPasswordlessDuoIntegrationToContext(CollectionSupport.singleton("duo_push"));
+        addAttemptedFlow("authn/DuoOIDC");
+        dc.setAuthToken(createPlainDummyToken(DuoOIDCAuthAPI.DUO_AUTH_RESULT_ALLOW,"Login Succesful",CLIENT_ID,
+                Instant.now().plus(1,ChronoUnit.MINUTES),Instant.now(), Instant.now(), "api.duosecurity.com", "duo_push"));
+        dc.setUsername("jdoe");
+        action.initialize();
+        
+        final Event event = action.execute(src);
+        ActionTestingSupport.assertProceedEvent(event);
+
+        //check the correct subject has been populated.
+        final var authnResult = ac.getAuthenticationResult();
+        assert authnResult != null;
+        final Subject sbj = authnResult.getSubject();
+        assertTrue(sbj.getPrincipals().contains(new UsernamePrincipal("jdoe")));
+    }
+
+    /**
+     * Test failed execution with factor enforcement.
+     * 
+     * @throws ComponentInitializationException on error.
+     */
+    @Test
+    public void testExecuteFailedWithFactorCheck() throws ComponentInitializationException {
+        addDuoContext();
+        addPasswordlessDuoIntegrationToContext(CollectionSupport.singleton("sms"));
+        addAttemptedFlow("authn/DuoOIDC");
+        dc.setAuthToken(createPlainDummyToken(DuoOIDCAuthAPI.DUO_AUTH_RESULT_ALLOW,"Login Succesful",CLIENT_ID,
+                Instant.now().plus(1,ChronoUnit.MINUTES),Instant.now(), Instant.now(), "api.duosecurity.com", "duo_push"));
+        dc.setUsername("jdoe");
+        action.initialize();
+        
+        final Event event = action.execute(src);
+        assertEventId(event, AuthnEventIds.INVALID_CREDENTIALS);
     }
     
     /**
@@ -135,8 +179,7 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
         action.initialize();
         
         final Event event = action.execute(src);
-        // success here is a null event
-        assertNull(event);
+        ActionTestingSupport.assertProceedEvent(event);
     }
     
     /**
@@ -154,7 +197,7 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
     }
     
     /**
-     * Test validation of a token who's 2FA request was denied.
+     * Test validation of a token whose 2FA request was denied.
      * 
      * @throws ComponentInitializationException on error.
      */
@@ -192,7 +235,6 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
         action.initialize();
         
         final Event event = action.execute(src);
-     
         assertEventId(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
     
@@ -216,7 +258,6 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
         action.initialize();
         
         final Event event = action.execute(src);
-     
         assertEventId(event, AuthnEventIds.INVALID_CREDENTIALS);
     }
     
@@ -261,8 +302,7 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
         action.initialize();
         
         final Event event = action.execute(src);
-        // success here is a null event
-        assertNull(event);
+        ActionTestingSupport.assertProceedEvent(event);
         
         //check the correct subject has been populated.
         final var ac = prc.getSubcontext(AuthenticationContext.class);
@@ -318,8 +358,7 @@ public class ValidateDuoTokenAuthenticationResultTest extends AbstractDuoActionT
         action.initialize();
         
         final Event event = action.execute(src);
-        // success here is a null event
-        assertNull(event);
+        ActionTestingSupport.assertProceedEvent(event);
         
         //check the correct subject has been populated.
         final var ac = prc.getSubcontext(AuthenticationContext.class);
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java
index 541c4e12..590b05c7 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenClaimsTest.java
@@ -49,6 +49,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 /**
  * Tests for the {@link ValidateTokenClaims} class.
  */
+ at SuppressWarnings("javadoc")
 public class ValidateTokenClaimsTest extends AbstractDuoActionTest {
 
     /** The action to test. */
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
index f7965214..e2f0e916 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/ValidateTokenSignatureTest.java
@@ -49,6 +49,7 @@ import net.shibboleth.shared.logic.ConstraintViolationException;
 /**
  * Tests for the {@link ValidateTokenSignature} class.
  */
+ at SuppressWarnings("javadoc")
 public class ValidateTokenSignatureTest extends AbstractDuoActionTest {
 
     /** The action to test. */

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


More information about the commits mailing list