[java-identity-provider] branch master updated: IDP-1494 - Login flow for proxied SAML authentication

Scott Cantor cantor.2 at osu.edu
Wed Nov 20 17:02:35 EST 2019


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

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

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

The following commit(s) were added to refs/heads/master by this push:
       new  9e63837   IDP-1494 - Login flow for proxied SAML authentication
9e63837 is described below

commit 9e63837734169e613b2ba170adb3fbc9068bcc97
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Nov 20 17:02:28 2019 -0500

    IDP-1494 - Login flow for proxied SAML authentication
    
    https://issues.shibboleth.net/jira/browse/IDP-1494
    
    Move proxy restriction logic into Principal class.
    Refactor handling of login flow reuse conditions.
    Adjust a mis-assigned error event.
---
 .../idp/authn/AbstractValidationAction.java        | 14 +++---
 .../idp/authn/AuthenticationFlowDescriptor.java    | 37 +++++++++-------
 .../shibboleth/idp/authn/AuthenticationResult.java | 30 +++++++++++++
 .../idp/authn/context/AuthenticationContext.java   | 35 ++-------------
 .../principal/ProxyAuthenticationPrincipal.java    | 30 ++++++++++++-
 .../idp/authn/impl/FinalizeAuthentication.java     | 51 ++--------------------
 .../idp/authn/impl/SelectAuthenticationFlow.java   | 23 ++++------
 .../impl/TransitionMultiFactorAuthentication.java  |  2 +-
 .../impl/HTPasswdCredentialValidatorTest.java      |  3 +-
 .../authn/impl/JAASCredentialValidatorTest.java    |  3 +-
 .../authn/impl/LDAPCredentialValidatorTest.java    |  3 +-
 .../impl/ValidateExternalAuthenticationTest.java   |  3 +-
 .../idp/authn/impl/ValidateFunctionResultTest.java |  3 +-
 .../idp/authn/impl/ValidateRemoteUserTest.java     |  3 +-
 .../authn/impl/ValidateUserAgentAddressTest.java   |  3 +-
 .../authn/impl/ValidateX509CertificateTest.java    |  3 +-
 16 files changed, 112 insertions(+), 134 deletions(-)

diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java
index 0bd0999..dd09027 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractValidationAction.java
@@ -51,7 +51,6 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 import org.opensaml.core.metrics.MetricsSupport;
 import org.opensaml.profile.action.ActionSupport;
-import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -63,7 +62,7 @@ import com.google.common.collect.Iterables;
  * A base class for authentication related actions that validate credentials and produce an
  * {@link AuthenticationResult}.
  * 
- * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @event {@link AuthnEventIds#INVALID_AUTHN_CTX}
  * @event {@link AuthnEventIds#REQUEST_UNSUPPORTED}
  * @pre <pre>ProfileRequestContext.getSubcontext(AuthenticationContext.class).getAttemptedFlow() != null</pre>
  */
@@ -296,7 +295,7 @@ public abstract class AbstractValidationAction extends AbstractAuthenticationAct
             return false;
         } else if (authenticationContext.getAttemptedFlow() == null) {
             log.info("{} No attempted flow within authentication context", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_AUTHN_CTX);
             return false;
         }
 
@@ -359,14 +358,13 @@ public abstract class AbstractValidationAction extends AbstractAuthenticationAct
     protected void buildAuthenticationResult(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final AuthenticationContext authenticationContext) {
         
-        if (addDefaultPrincipals && authenticationContext.getAttemptedFlow() != null) {
+        if (addDefaultPrincipals) {
             log.debug("{} Adding custom Principal(s) defined on underlying flow descriptor", getLogPrefix());
-            getSubject().getPrincipals().addAll(
-                    authenticationContext.getAttemptedFlow().getSupportedPrincipals());
+            getSubject().getPrincipals().addAll(authenticationContext.getAttemptedFlow().getSupportedPrincipals());
         }
         
-        final AuthenticationResult result = new AuthenticationResult(authenticationContext.getAttemptedFlow().getId(),
-                populateSubject(getSubject()));
+        final AuthenticationResult result =
+                authenticationContext.getAttemptedFlow().newAuthenticationResult(populateSubject(getSubject()));
         authenticationContext.setAuthenticationResult(result);
         
         // Override cacheability if a predicate is installed.
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java
index 48722a2..965085f 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationFlowDescriptor.java
@@ -172,17 +172,6 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
     }
     
     /**
-     * Get condition controlling whether results from this flow should be reused for SSO.
-     * 
-     * @return whether results from this flow should be reused for SSO
-     * 
-     * @since 3.4.0
-     */
-    @Nonnull public Predicate<ProfileRequestContext> getReuseCondition() {
-        return reuseCondition;
-    }
-    
-    /**
      * Set condition controlling whether results from this flow should be reused for SSO.
      * 
      * <p>Defaults to {@link Predicates#alwaysTrue()}.</p>
@@ -342,7 +331,7 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
     public void setPrincipalWeightMap(@Nullable @NonnullElements final Map<Principal,Integer> map) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         
-        principalWeightMap = map != null ? map : Collections.<Principal,Integer>emptyMap();
+        principalWeightMap = map != null ? map : Collections.emptyMap();
     }    
 
     /** {@inheritDoc} */
@@ -354,6 +343,19 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
         }
     }
 
+    /**
+     * Creates a new instance of a compatible {@link AuthenticationResult} for use with the corresponding flow.
+     * 
+     * @param subject the subject for the result
+     * 
+     * @return the new result
+     */
+    @Nonnull public AuthenticationResult newAuthenticationResult(@Nonnull final Subject subject) {
+        final AuthenticationResult result = new AuthenticationResult(getId(), subject);
+        result.setReuseCondition(reuseCondition);
+        return result;
+    }
+    
     /** {@inheritDoc} */
     @Override @Nonnull @NotEmpty public String serialize(@Nonnull final AuthenticationResult instance)
             throws IOException {
@@ -370,8 +372,12 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
         ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
 
         // Back the expiration off by the inactivity timeout to recover the last activity time.
-        return resultSerializer.deserialize(version, context, key, value, (expiration != null) ? expiration
-                - inactivityTimeout.toMillis() - STORAGE_EXPIRATION_OFFSET.toMillis() : null);
+        final AuthenticationResult result = resultSerializer.deserialize(version, context, key, value,
+                (expiration != null) ?
+                        expiration - inactivityTimeout.toMillis() - STORAGE_EXPIRATION_OFFSET.toMillis() :
+                            null);
+        result.setReuseCondition(reuseCondition);
+        return result;
     }
 
     /**
@@ -395,7 +401,7 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
             return (T) principalArray[principalArray.length - 1];
         }
     }
-
+    
     /** {@inheritDoc} */
     @Override public int hashCode() {
         return getId().hashCode();
@@ -453,4 +459,5 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
     static {
         STORAGE_EXPIRATION_OFFSET = Duration.ofMinutes(10);
     }
+
 }
\ No newline at end of file
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationResult.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationResult.java
index 63fc578..bc22854 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationResult.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AuthenticationResult.java
@@ -24,11 +24,14 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.security.auth.Subject;
 
+import org.opensaml.profile.context.ProfileRequestContext;
+
 import net.shibboleth.idp.authn.principal.PrincipalSupportingComponent;
 import net.shibboleth.idp.authn.principal.UsernamePrincipal;
 import net.shibboleth.utilities.java.support.annotation.constraint.Live;
@@ -40,6 +43,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableSet;
 
 /**
@@ -70,6 +74,9 @@ public class AuthenticationResult implements PrincipalSupportingComponent {
     /** A map of additional data to associate with the result. */
     @Nonnull @NonnullElements private final Map<String,String> additionalData;
     
+    /** Whether this result can be reused. */
+    @Nonnull private Predicate<ProfileRequestContext> reuseCondition;
+    
     /**
      * Constructor.
      * 
@@ -86,6 +93,7 @@ public class AuthenticationResult implements PrincipalSupportingComponent {
         authenticationInstant = Instant.now();
         lastActivityInstant = authenticationInstant;
         additionalData = new HashMap<>();
+        reuseCondition = Predicates.alwaysTrue();
     }
 
     /**
@@ -100,6 +108,28 @@ public class AuthenticationResult implements PrincipalSupportingComponent {
     }
     
     /**
+     * Get condition controlling whether this result should be reused for SSO.
+     * 
+     * @return condition controlling whether result should be reused for SSO
+     * 
+     * @since 4.0.0
+     */
+    @Nonnull public Predicate<ProfileRequestContext> getReuseCondition() {
+        return reuseCondition;
+    }
+    
+    /**
+     * Set condition controlling whether this result should be reused for SSO.
+     * 
+     * @param condition condition to set
+     * 
+     * @since 4.0.0
+     */
+    public void setReuseCondition(@Nonnull final Predicate<ProfileRequestContext> condition) {
+        reuseCondition = Constraint.isNotNull(condition, "Predicate cannot be null");
+    }
+    
+    /**
      * Get the Subject identifying the authenticated entity.
      * 
      * @return a Subject identifying the authenticated entity
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java
index 7a30eac..fc9abd1 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/context/AuthenticationContext.java
@@ -35,7 +35,6 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import net.shibboleth.idp.authn.AuthenticationResult;
-import net.shibboleth.idp.authn.config.AuthenticationProfileConfiguration;
 import net.shibboleth.idp.authn.AuthenticationFlowDescriptor;
 import net.shibboleth.idp.authn.principal.PrincipalEvalPredicateFactoryRegistry;
 import net.shibboleth.idp.authn.principal.PrincipalSupportingComponent;
@@ -48,8 +47,6 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 import org.opensaml.messaging.context.BaseContext;
-import org.opensaml.messaging.context.navigate.ChildContextLookup;
-import org.opensaml.messaging.context.navigate.ParentContextLookup;
 import org.opensaml.profile.context.ProfileRequestContext;
 
 import com.google.common.base.MoreObjects;
@@ -87,9 +84,6 @@ public final class AuthenticationContext extends BaseContext {
     /** Name of a proxied authentication source to use. */
     @Nullable private String authenticatingAuthority;
 
-    /** Strategy used to look up a {@link RelyingPartyContext} for proxy validation. */
-    @Nonnull private Function<AuthenticationContext,RelyingPartyContext> relyingPartyContextLookupStrategy;
-
     /** Lookup strategy for a fixed event to return from validators for testing. */
     @Nullable private Function<ProfileRequestContext,String> fixedEventLookupStrategy;
     
@@ -138,9 +132,6 @@ public final class AuthenticationContext extends BaseContext {
         stateMap = new HashMap<>();
         
         resultCacheable = true;
-        
-        relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class).compose(
-                new ParentContextLookup<>(ProfileRequestContext.class));
     }
 
     /**
@@ -432,17 +423,6 @@ public final class AuthenticationContext extends BaseContext {
         fixedEventLookupStrategy = strategy;
         return this;
     }
-    
-    /**
-     * Set the strategy used to return the associated {@link RelyingPartyContext} for proxy restriction evaluation.
-     * 
-     * @param strategy lookup strategy
-     */
-    public void setRelyingPartyContextLookupStrategy(
-            @Nullable final Function<AuthenticationContext,RelyingPartyContext> strategy) {
-        relyingPartyContextLookupStrategy = strategy;
-    }
-    
 
     /**
      * Get the authentication flow that was attempted in order to authenticate the user.
@@ -754,7 +734,6 @@ public final class AuthenticationContext extends BaseContext {
                 .toString();
     }
 
-// Checkstyle: CyclomaticComplexity OFF
     /**
      * Check for proxy restrictions and evaluate them against the associated {@link RelyingPartyContext}.
      * 
@@ -768,25 +747,19 @@ public final class AuthenticationContext extends BaseContext {
         if (principals == null || principals.isEmpty()) {
             return true;
         }
-        
-        // Check for local flow as relying party.
-        final RelyingPartyContext rpCtx = relyingPartyContextLookupStrategy.apply(this);
-        if (rpCtx == null || !(rpCtx.getProfileConfig() instanceof AuthenticationProfileConfiguration) ||
-                ((AuthenticationProfileConfiguration) rpCtx.getProfileConfig()).isLocal()) {
+
+        final BaseContext prc = getParent();
+        if (!(prc instanceof ProfileRequestContext)) {
             return true;
         }
         
         for (final ProxyAuthenticationPrincipal proxied : principals) {
-            if (proxied.getProxyCount() != null && proxied.getProxyCount() == 0) {
-                return false;
-            } else if (rpCtx.getRelyingPartyId() != null && !proxied.getAudiences().isEmpty() &&
-                    !proxied.getAudiences().contains(rpCtx.getRelyingPartyId())) {
+            if (!proxied.test((ProfileRequestContext) prc)) {
                 return false;
             }
         }
         
         return true;
     }
-// Checkstyle: CyclomaticComplexity ON
     
 }
\ No newline at end of file
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java
index 3bf5f73..77087ef 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/principal/ProxyAuthenticationPrincipal.java
@@ -21,10 +21,15 @@ import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.opensaml.profile.context.ProfileRequestContext;
+
+import net.shibboleth.idp.authn.config.AuthenticationProfileConfiguration;
+import net.shibboleth.idp.profile.context.RelyingPartyContext;
 import net.shibboleth.utilities.java.support.annotation.constraint.Live;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonNegative;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
@@ -37,9 +42,12 @@ import com.google.common.base.MoreObjects;
  * Principal that wraps a set of proxied authentication authorities and any restrictions
  * on subsequent re-use.
  * 
+ * <p>The {@link Predicate} interface implements restriction logic that returns true iff
+ * the restrictions embedded in the object do NOT apply to the input request.</p>
+ * 
  * @since 3.4.0
  */
-public class ProxyAuthenticationPrincipal implements Principal {
+public class ProxyAuthenticationPrincipal implements Principal, Predicate<ProfileRequestContext> {
 
     /** The authorities. */
     @Nonnull @NonnullElements private Collection<String> authorities;
@@ -121,6 +129,26 @@ public class ProxyAuthenticationPrincipal implements Principal {
     }
 
     /** {@inheritDoc} */
+    public boolean test(@Nullable final ProfileRequestContext input) {
+        
+        // Check for local flow as relying party.
+        final RelyingPartyContext rpCtx = input != null ? input.getSubcontext(RelyingPartyContext.class) : null;
+        if (rpCtx == null || !(rpCtx.getProfileConfig() instanceof AuthenticationProfileConfiguration) ||
+                ((AuthenticationProfileConfiguration) rpCtx.getProfileConfig()).isLocal()) {
+            return true;
+        }
+        
+        if (proxyCount != null && proxyCount == 0) {
+            return false;
+        } else if (rpCtx.getRelyingPartyId() != null && !audiences.isEmpty() &&
+                !audiences.contains(rpCtx.getRelyingPartyId())) {
+            return false;
+        }
+        
+        return true;
+    }
+    
+    /** {@inheritDoc} */
     @Override
     public int hashCode() {
         return authorities.hashCode();
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java
index edf870e..7c456d7 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/FinalizeAuthentication.java
@@ -22,7 +22,6 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Function;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -32,7 +31,6 @@ import net.shibboleth.idp.authn.AbstractAuthenticationAction;
 import net.shibboleth.idp.authn.AuthenticationFlowDescriptor;
 import net.shibboleth.idp.authn.AuthenticationResult;
 import net.shibboleth.idp.authn.AuthnEventIds;
-import net.shibboleth.idp.authn.config.AuthenticationProfileConfiguration;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.authn.context.RequestedPrincipalContext;
 import net.shibboleth.idp.authn.context.SubjectCanonicalizationContext;
@@ -44,10 +42,7 @@ import net.shibboleth.idp.authn.principal.ProxyAuthenticationPrincipal;
 import net.shibboleth.idp.profile.IdPEventIds;
 import net.shibboleth.idp.profile.context.RelyingPartyContext;
 import net.shibboleth.idp.session.context.SessionContext;
-import net.shibboleth.utilities.java.support.component.ComponentSupport;
-import net.shibboleth.utilities.java.support.logic.Constraint;
 
-import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
@@ -102,29 +97,8 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(FinalizeAuthentication.class);
     
-    /** Strategy used to look up a {@link RelyingPartyContext} for proxy validation. */
-    @Nonnull private Function<ProfileRequestContext,RelyingPartyContext> relyingPartyContextLookupStrategy;
-    
     /** The principal name extracted from the context tree. */
     @Nullable private String canonicalPrincipalName;
-     
-    /** Constructor. */
-    public FinalizeAuthentication() {
-        relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class);
-    }
-    
-    /**
-     * Set the strategy used to return the {@link RelyingPartyContext}.
-     * 
-     * @param strategy lookup strategy
-     */
-    public void setRelyingPartyContextLookupStrategy(
-            @Nonnull final Function<ProfileRequestContext,RelyingPartyContext> strategy) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
-        relyingPartyContextLookupStrategy =
-                Constraint.isNotNull(strategy, "RelyingPartyContext lookup strategy cannot be null");
-    }
     
     // Checkstyle: CyclomaticComplexity OFF
     /** {@inheritDoc} */
@@ -297,7 +271,6 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
         return flowDescriptor.getHighestWeighted(matches);
     }
 
-// Checkstyle: CyclomaticComplexity OFF
     /**
      * Check for proxy restrictions and evaluate them against the request.
      * 
@@ -314,27 +287,9 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
             return true;
         }
         
-        // Check for admin flow as relying party.
-        final RelyingPartyContext rpCtx = relyingPartyContextLookupStrategy.apply(profileRequestContext);
-        if (rpCtx == null) {
-            log.debug("{} No RelyingPartyContext, ignoring proxy restrictions on result", getLogPrefix());
-            return true;
-        } else if (!(rpCtx.getProfileConfig() instanceof AuthenticationProfileConfiguration) ||
-                ((AuthenticationProfileConfiguration) rpCtx.getProfileConfig()).isLocal()) {
-            log.debug("{} Profile is not proxy-restricted, ignoring proxy restrictions on result",
-                    getLogPrefix());
-            return true;
-        }
-        
         for (final ProxyAuthenticationPrincipal proxied : proxieds) {
-            if (proxied.getProxyCount() != null && proxied.getProxyCount() == 0) {
-                log.warn("{} Result contains a proxy count of zero, disallowing use", getLogPrefix());
-                ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.REQUEST_UNSUPPORTED);
-                return false;
-            } else if (rpCtx.getRelyingPartyId() != null && !proxied.getAudiences().isEmpty() &&
-                    !proxied.getAudiences().contains(rpCtx.getRelyingPartyId())) {
-                log.warn("{} Result contains a proxy restriction disallowing relying party '{}'",
-                        getLogPrefix(), rpCtx.getRelyingPartyId());
+            if (!proxied.test(profileRequestContext)) {
+                log.warn("{} Result contained a proxy restriction disallowing use", getLogPrefix());
                 ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.REQUEST_UNSUPPORTED);
                 return false;
             }
@@ -342,5 +297,5 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
         
         return true;
     }
-// Checkstyle: CyclomaticComplexity ON
+
 }
\ No newline at end of file
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectAuthenticationFlow.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectAuthenticationFlow.java
index 383c99d..4a93b07 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectAuthenticationFlow.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/SelectAuthenticationFlow.java
@@ -184,13 +184,14 @@ public class SelectAuthenticationFlow extends AbstractAuthenticationAction {
         
         log.debug("{} Attempting to honor signaled flow {}", getLogPrefix(), flow.getId());
 
-        // If not forced, or we just did it, check for an active result for that flow.
+        // If not forced, check for an active result for that flow.
 
-        final AuthenticationResult activeResult;
-        if (!authenticationContext.isForceAuthn() && flow.getReuseCondition().test(profileRequestContext)) {
+        AuthenticationResult activeResult = null;
+        if (!authenticationContext.isForceAuthn()) {
             activeResult = authenticationContext.getActiveResults().get(flow.getId());
-        } else {
-            activeResult = null;
+            if (!activeResult.getReuseCondition().test(profileRequestContext)) {
+                activeResult = null;
+            }
         }
         
         if (activeResult != null) {
@@ -277,9 +278,7 @@ public class SelectAuthenticationFlow extends AbstractAuthenticationAction {
         AuthenticationResult resultToSelect = null;
         
         for (final AuthenticationResult activeResult : authenticationContext.getActiveResults().values()) {
-            final AuthenticationFlowDescriptor flow = authenticationContext.getPotentialFlows().get(
-                    activeResult.getAuthenticationFlowId());
-            if (flow != null && flow.getReuseCondition().test(profileRequestContext)) {
+            if (activeResult.getReuseCondition().test(profileRequestContext)) {
                 resultToSelect = activeResult;
                 if (preferredPrincipalCtx == null || preferredPrincipalCtx.isAcceptable(activeResult)) {
                     break;
@@ -448,8 +447,6 @@ public class SelectAuthenticationFlow extends AbstractAuthenticationAction {
         if (favorSSO) {
             log.debug("{} Giving priority to active results that meet request requirements");
             
-            final Map<String,AuthenticationFlowDescriptor> availableFlows = authenticationContext.getAvailableFlows();
-            
             // Check each active result for compatibility with request.
             for (final Principal p : requestedPrincipalCtx.getRequestedPrincipals()) {
                 log.debug("{} Checking for an active result compatible with operator '{}' and principal '{}'",
@@ -457,9 +454,7 @@ public class SelectAuthenticationFlow extends AbstractAuthenticationAction {
                 final PrincipalEvalPredicate predicate = requestedPrincipalCtx.getPredicate(p);
                 if (predicate != null) {
                     for (final AuthenticationResult result : activeResults.values()) {
-                        final AuthenticationFlowDescriptor flow = availableFlows.get(result.getAuthenticationFlowId());
-                        if (flow != null && flow.getReuseCondition().test(profileRequestContext)
-                                && predicate.test(result)) {
+                        if (result.getReuseCondition().test(profileRequestContext) && predicate.test(result)) {
                             selectActiveResult(profileRequestContext, authenticationContext, result);
                             return;
                         }
@@ -495,7 +490,7 @@ public class SelectAuthenticationFlow extends AbstractAuthenticationAction {
                         // Now check for an active result we can use from this flow. Not all results from a flow
                         // will necessarily match the request just because the flow might.
                         final AuthenticationResult result = activeResults.get(descriptor.getId());
-                        if (result == null || !descriptor.getReuseCondition().test(profileRequestContext)
+                        if (result == null || !result.getReuseCondition().test(profileRequestContext)
                                 || !predicate.test(result)) {
                             if (!authenticationContext.isPassive()
                                     || descriptor.isPassiveAuthenticationSupported()) {
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 84754db..59654c9 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
@@ -272,7 +272,7 @@ public class TransitionMultiFactorAuthentication extends AbstractAuthenticationA
         // infinite recursion is the configuration of transitions supplied by the deployer.
         final AuthenticationResult activeResult = mfaContext.getActiveResults().get(flowId);
         if (activeResult != null) {
-            if (flow.getReuseCondition().test(profileRequestContext)) {
+            if (activeResult.getReuseCondition().test(profileRequestContext)) {
                 log.debug("{} Reusing active result for '{}' flow", getLogPrefix(), flowId);
                 activeResult.setLastActivityInstantToNow();
                 previousEvent = EventIds.PROCEED_EVENT_ID;
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/HTPasswdCredentialValidatorTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/HTPasswdCredentialValidatorTest.java
index 4d058a7..2925609 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/HTPasswdCredentialValidatorTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/HTPasswdCredentialValidatorTest.java
@@ -36,7 +36,6 @@ import net.shibboleth.idp.authn.principal.UsernamePrincipal;
 import net.shibboleth.idp.authn.principal.impl.ExactPrincipalEvalPredicateFactory;
 import net.shibboleth.idp.profile.ActionTestingSupport;
 
-import org.opensaml.profile.action.EventIds;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -77,7 +76,7 @@ public class HTPasswdCredentialValidatorTest extends BaseAuthenticationContextTe
         action.initialize();
 
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
 
     @Test public void testMissingUser() throws Exception {
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/JAASCredentialValidatorTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/JAASCredentialValidatorTest.java
index fd65226..14607df 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/JAASCredentialValidatorTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/JAASCredentialValidatorTest.java
@@ -42,7 +42,6 @@ import net.shibboleth.idp.profile.ActionTestingSupport;
 import net.shibboleth.utilities.java.support.collection.Pair;
 import net.shibboleth.utilities.java.support.net.URISupport;
 
-import org.opensaml.profile.action.EventIds;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.webflow.execution.Event;
 import org.testng.Assert;
@@ -111,7 +110,7 @@ public class JAASCredentialValidatorTest extends BaseAuthenticationContextTest {
         action.initialize();
 
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
 
     @Test public void testMissingUser() throws Exception {
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java
index 1477529..a88c1b0 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java
@@ -48,7 +48,6 @@ import org.ldaptive.auth.SearchDnResolver;
 import org.ldaptive.auth.ext.PasswordPolicyAccountState;
 import org.ldaptive.control.PasswordPolicyControl;
 import org.ldaptive.jaas.LdapPrincipal;
-import org.opensaml.profile.action.EventIds;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.webflow.execution.Event;
 import org.testng.Assert;
@@ -140,7 +139,7 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
         action.initialize();
 
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
 
     @Test public void testMissingUser() throws Exception {
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateExternalAuthenticationTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateExternalAuthenticationTest.java
index 8ae5cf8..3eb3431 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateExternalAuthenticationTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateExternalAuthenticationTest.java
@@ -35,7 +35,6 @@ import net.shibboleth.idp.authn.principal.TestPrincipal;
 import net.shibboleth.idp.authn.principal.UsernamePrincipal;
 import net.shibboleth.idp.profile.ActionTestingSupport;
 
-import org.opensaml.profile.action.EventIds;
 import org.springframework.webflow.execution.Event;
 import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
@@ -64,7 +63,7 @@ public class ValidateExternalAuthenticationTest extends BaseAuthenticationContex
         prc.getSubcontext(AuthenticationContext.class).setAttemptedFlow(null);
         
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
     
     @Test public void testMissingContext() {
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateFunctionResultTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateFunctionResultTest.java
index 6bf4ab7..638dd03 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateFunctionResultTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateFunctionResultTest.java
@@ -29,7 +29,6 @@ import net.shibboleth.idp.profile.ActionTestingSupport;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.logic.FunctionSupport;
 
-import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.springframework.webflow.execution.Event;
 import org.testng.Assert;
@@ -58,7 +57,7 @@ public class ValidateFunctionResultTest extends BaseAuthenticationContextTest {
         action.initialize();
         
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
     
     @Test public void testNoCredentials() throws ComponentInitializationException {
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateRemoteUserTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateRemoteUserTest.java
index e185d72..58d3b45 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateRemoteUserTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateRemoteUserTest.java
@@ -29,7 +29,6 @@ import net.shibboleth.idp.authn.principal.UsernamePrincipal;
 import net.shibboleth.idp.profile.ActionTestingSupport;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 
-import org.opensaml.profile.action.EventIds;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.webflow.execution.Event;
 import org.testng.Assert;
@@ -54,7 +53,7 @@ public class ValidateRemoteUserTest extends BaseAuthenticationContextTest {
 
     @Test public void testMissingFlow() {
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
     
     @Test public void testMissingUser() {
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateUserAgentAddressTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateUserAgentAddressTest.java
index f363b17..6f56cfb 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateUserAgentAddressTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateUserAgentAddressTest.java
@@ -33,7 +33,6 @@ import net.shibboleth.idp.profile.ActionTestingSupport;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.net.IPRange;
 
-import org.opensaml.profile.action.EventIds;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.webflow.execution.Event;
 import org.testng.Assert;
@@ -58,7 +57,7 @@ public class ValidateUserAgentAddressTest extends BaseAuthenticationContextTest
 
     @Test public void testMissingFlow() {
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
     
     @Test public void testMissingAddress() {
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateX509CertificateTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateX509CertificateTest.java
index 73f94f0..31d349e 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateX509CertificateTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateX509CertificateTest.java
@@ -29,7 +29,6 @@ import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.idp.profile.ActionTestingSupport;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 
-import org.opensaml.profile.action.EventIds;
 import org.opensaml.security.credential.CredentialResolver;
 import org.opensaml.security.credential.impl.StaticCredentialResolver;
 import org.opensaml.security.trust.TrustEngine;
@@ -103,7 +102,7 @@ public class ValidateX509CertificateTest extends BaseAuthenticationContextTest {
 
     @Test public void testMissingFlow() {
         final Event event = action.execute(src);
-        ActionTestingSupport.assertEvent(event, EventIds.INVALID_PROFILE_CTX);
+        ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_AUTHN_CTX);
     }
     
     @Test public void testMissingCert() {

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


More information about the commits mailing list