[java-identity-provider] 01/02: IDP-1519 - MFA transition action mishandles previous event determination

[Scott Cantor]

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

scantor pushed a commit to branch maint-3.4
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=1d2a9af150a61db061ffdcfed9859ca191790158

commit 1d2a9af150a61db061ffdcfed9859ca191790158
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Nov 13 14:20:45 2019 -0500

    IDP-1519 - MFA transition action mishandles previous event determination
    
    https://issues.shibboleth.net/jira/browse/IDP-1519
---
 .../impl/TransitionMultiFactorAuthentication.java  | 31 +++++++++++++---------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/TransitionMultiFactorAuthentication.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/TransitionMultiFactorAuthentication.java
index aceb31e10..dca75c2bc 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/TransitionMultiFactorAuthentication.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/TransitionMultiFactorAuthentication.java
@@ -101,6 +101,9 @@ public class TransitionMultiFactorAuthentication extends AbstractAuthenticationA
 
     /** A subordinate {@link MultiFactorAuthenticationContext}, if any. */
     @Nullable private MultiFactorAuthenticationContext mfaContext;
+    
+    /** Holds the last event processed by the system. */
+    @Nullable private String previousEvent;
 
     /** Constructor. */
     TransitionMultiFactorAuthentication() {
@@ -180,10 +183,13 @@ public class TransitionMultiFactorAuthentication extends AbstractAuthenticationA
         // Swap MFA flow back into top-level context so that other components see only MFA flow.
         authenticationContext.setAttemptedFlow(mfaContext.getAuthenticationFlowDescriptor());
 
-        // Event transitions require normalizing empty/null events into "proceed".
-        final EventContext eventCtx = eventContextLookupStrategy.apply(profileRequestContext);
-        final String previousEvent = eventCtx != null && eventCtx.getEvent() != null
-                ? eventCtx.getEvent().toString() : EventIds.PROCEED_EVENT_ID;
+        // If the holding variable is already set, this is a recursive invocation.
+        if (previousEvent == null) {
+            // Event transitions require normalizing empty/null events into "proceed".
+            final EventContext eventCtx = eventContextLookupStrategy.apply(profileRequestContext);
+            previousEvent = eventCtx != null && eventCtx.getEvent() != null
+                    ? eventCtx.getEvent().toString() : EventIds.PROCEED_EVENT_ID;
+        }
 
         // Check for an authentication result and move it into the MFA context.
         final AuthenticationResult result = authenticationContext.getAuthenticationResult();
@@ -233,7 +239,7 @@ public class TransitionMultiFactorAuthentication extends AbstractAuthenticationA
     }
 // Checkstyle: CyclomaticComplexity ON
 
-// Checkstyle: CyclomaticComplexity|ReturnCount OFF
+// Checkstyle: CyclomaticComplexity OFF
     /**
      * Respond to a signal to transition the MFA process to a new flow.
      * 
@@ -270,6 +276,7 @@ public class TransitionMultiFactorAuthentication extends AbstractAuthenticationA
             if (flow.getReuseCondition().apply(profileRequestContext)) {
                 log.debug("{} Reusing active result for '{}' flow", getLogPrefix(), flowId);
                 activeResult.setLastActivityInstantToNow();
+                previousEvent = EventIds.PROCEED_EVENT_ID;
                 ActionSupport.buildProceedEvent(profileRequestContext);
                 doExecute(profileRequestContext, authenticationContext);
                 return;
@@ -281,19 +288,19 @@ public class TransitionMultiFactorAuthentication extends AbstractAuthenticationA
      
         if (validateLoginTransitions) {
             if (authenticationContext.isPassive() && !flow.isPassiveAuthenticationSupported()) {
-                log.error("{} Targeted login flow '{}' does not support passive authentication",
-                        getLogPrefix(), flowId);
+                log.error("{} Targeted login flow '{}' does not support passive authentication", getLogPrefix(),
+                        flowId);
                 ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_PASSIVE);
                 return;
             } else if ((authenticationContext.isForceAuthn() || authenticationContext.getMaxAge() > 0)
                     && !flow.isForcedAuthenticationSupported()) {
-                log.error("{} Targeted login flow '{}' does not support forced re-authentication",
-                        getLogPrefix(), flowId);
+                log.error("{} Targeted login flow '{}' does not support forced re-authentication", getLogPrefix(),
+                        flowId);
                 ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.REQUEST_UNSUPPORTED);
                 return;
             } else if (!profileRequestContext.isBrowserProfile() && !flow.isNonBrowserSupported()) {
-                log.error("{} Targeted login flow '{}' does not support non-browser authentication",
-                        getLogPrefix(), flowId);
+                log.error("{} Targeted login flow '{}' does not support non-browser authentication", getLogPrefix(),
+                        flowId);
                 ActionSupport.buildEvent(profileRequestContext, authenticationContext.isPassive() ?
                         AuthnEventIds.NO_PASSIVE : AuthnEventIds.REQUEST_UNSUPPORTED);
                 return;
@@ -304,6 +311,6 @@ public class TransitionMultiFactorAuthentication extends AbstractAuthenticationA
         authenticationContext.setAttemptedFlow(flow);
         ActionSupport.buildProceedEvent(profileRequestContext);
     }
-// Checkstyle: CyclomaticComplexity|ReturnCount ON
+// Checkstyle: CyclomaticComplexity ON
     
 }
\ No newline at end of file

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