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

Scott Cantor cantor.2 at osu.edu
Tue Nov 19 22:13:03 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=3daa702e0a4d53fc0651ce7b73f622b965ef95a7

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

commit 3daa702e0a4d53fc0651ce7b73f622b965ef95a7
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Nov 19 22:12:57 2019 -0500

    IDP-1494 - Login flow for proxied SAML authentication
    
    https://issues.shibboleth.net/jira/browse/IDP-1494
    
    Extend proxy restriction eval to core result testing logic.
---
 .../idp/admin/AdministrativeFlowDescriptor.java    |  5 ++
 .../config/AuthenticationProfileConfiguration.java | 12 +++-
 .../idp/authn/context/AuthenticationContext.java   | 69 +++++++++++++++++++++-
 idp-authn-impl/pom.xml                             | 12 ++--
 .../idp/authn/impl/FinalizeAuthentication.java     | 13 ++--
 .../idp/authn/impl/FinalizeAuthenticationTest.java |  3 +
 6 files changed, 100 insertions(+), 14 deletions(-)

diff --git a/idp-admin-api/src/main/java/net/shibboleth/idp/admin/AdministrativeFlowDescriptor.java b/idp-admin-api/src/main/java/net/shibboleth/idp/admin/AdministrativeFlowDescriptor.java
index b07d759..114acbf 100644
--- a/idp-admin-api/src/main/java/net/shibboleth/idp/admin/AdministrativeFlowDescriptor.java
+++ b/idp-admin-api/src/main/java/net/shibboleth/idp/admin/AdministrativeFlowDescriptor.java
@@ -88,4 +88,9 @@ public interface AdministrativeFlowDescriptor extends FlowDescriptor, Authentica
      */
     boolean isResolveAttributes(@Nullable final ProfileRequestContext profileRequestContext);
 
+    /** {@inheritDoc} */
+    @Override
+    default boolean isLocal() {
+        return true;
+    }
 }
\ No newline at end of file
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/AuthenticationProfileConfiguration.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/AuthenticationProfileConfiguration.java
index 17c45d1..4ee4f91 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/AuthenticationProfileConfiguration.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/AuthenticationProfileConfiguration.java
@@ -20,7 +20,6 @@ package net.shibboleth.idp.authn.config;
 import java.security.Principal;
 import java.util.List;
 import java.util.Set;
-import java.util.function.Function;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -94,4 +93,15 @@ public interface AuthenticationProfileConfiguration extends ProfileConfiguration
      */
     boolean isForceAuthn(@Nullable final ProfileRequestContext profileRequestContext);
     
+    /**
+     * Get whether this profile is for functionality local to the IdP.
+     * 
+     * <p>Most authentication profiles are non-local, designed to issue security tokens to other
+     * systems, so this is generally false.</p>
+     * 
+     * @return true iff the use of the associated profile is local to the IdP
+     */
+    default boolean isLocal() {
+        return false;
+    }
 }
\ No newline at end of file
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 19c209c..7a30eac 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
@@ -29,14 +29,18 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 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;
+import net.shibboleth.idp.authn.principal.ProxyAuthenticationPrincipal;
+import net.shibboleth.idp.profile.context.RelyingPartyContext;
 import net.shibboleth.utilities.java.support.annotation.constraint.Live;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
@@ -44,6 +48,8 @@ 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;
@@ -81,6 +87,9 @@ 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;
     
@@ -129,6 +138,9 @@ public final class AuthenticationContext extends BaseContext {
         stateMap = new HashMap<>();
         
         resultCacheable = true;
+        
+        relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class).compose(
+                new ParentContextLookup<>(ProfileRequestContext.class));
     }
 
     /**
@@ -420,6 +432,17 @@ 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.
@@ -568,7 +591,7 @@ public final class AuthenticationContext extends BaseContext {
             return rpCtx.isAcceptable(component);
         }
         // No requirements so anything is acceptable.
-        return true;
+        return checkProxyRestrictions(component.getSupportedPrincipals(ProxyAuthenticationPrincipal.class));
     }
         
     /**
@@ -586,7 +609,11 @@ public final class AuthenticationContext extends BaseContext {
             return rpCtx.isAcceptable(principals);
         }
         // No requirements so anything is acceptable.
-        return true;
+        return checkProxyRestrictions(principals
+                .stream()
+                .filter(ProxyAuthenticationPrincipal.class::isInstance)
+                .map(ProxyAuthenticationPrincipal.class::cast)
+                .collect(Collectors.toUnmodifiableList()));
     }
 
     /**
@@ -605,6 +632,9 @@ public final class AuthenticationContext extends BaseContext {
             return rpCtx.isAcceptable(principal);
         }
         // No requirements so anything is acceptable.
+        if (principal instanceof ProxyAuthenticationPrincipal) {
+            return checkProxyRestrictions(Collections.singletonList((ProxyAuthenticationPrincipal) principal));
+        }
         return true;
     }
 
@@ -724,4 +754,39 @@ public final class AuthenticationContext extends BaseContext {
                 .toString();
     }
 
+// Checkstyle: CyclomaticComplexity OFF
+    /**
+     * Check for proxy restrictions and evaluate them against the associated {@link RelyingPartyContext}.
+     * 
+     * @param principals the proxy restrictions
+     * 
+     * @return true iff proxying is permissible or inapplicable
+     */
+    private boolean checkProxyRestrictions(
+            @Nullable @NonnullElements final Collection<ProxyAuthenticationPrincipal> principals) {
+        
+        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()) {
+            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())) {
+                return false;
+            }
+        }
+        
+        return true;
+    }
+// Checkstyle: CyclomaticComplexity ON
+    
 }
\ No newline at end of file
diff --git a/idp-authn-impl/pom.xml b/idp-authn-impl/pom.xml
index 106e461..232e345 100644
--- a/idp-authn-impl/pom.xml
+++ b/idp-authn-impl/pom.xml
@@ -24,11 +24,6 @@
         <!-- Compile Dependencies -->
         <dependency>
             <groupId>${project.groupId}</groupId>
-            <artifactId>idp-admin-api</artifactId>
-            <version>${project.version}</version>
-        </dependency>
-        <dependency>
-            <groupId>${project.groupId}</groupId>
             <artifactId>idp-attribute-api</artifactId>
             <version>${project.version}</version>
         </dependency>
@@ -215,6 +210,13 @@
 
         <dependency>
             <groupId>${project.groupId}</groupId>
+            <artifactId>idp-admin-api</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>${project.groupId}</groupId>
             <artifactId>idp-profile-api</artifactId>
             <version>${project.version}</version>
             <scope>test</scope>
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 53dd433..edf870e 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
@@ -28,11 +28,11 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.security.auth.Subject;
 
-import net.shibboleth.idp.admin.AdministrativeFlowDescriptor;
 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;
@@ -316,11 +316,12 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
         
         // Check for admin flow as relying party.
         final RelyingPartyContext rpCtx = relyingPartyContextLookupStrategy.apply(profileRequestContext);
-        if (rpCtx == null || rpCtx.getRelyingPartyId() == null) {
-            log.debug("{} No RelyingParty identity, ignoring proxy restrictions on result", getLogPrefix());
+        if (rpCtx == null) {
+            log.debug("{} No RelyingPartyContext, ignoring proxy restrictions on result", getLogPrefix());
             return true;
-        } else if (rpCtx.getProfileConfig() instanceof AdministrativeFlowDescriptor) {
-            log.debug("{} Relying party is an admin flow, ignoring proxy restrictions on result",
+        } 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;
         }
@@ -330,7 +331,7 @@ public class FinalizeAuthentication extends AbstractAuthenticationAction {
                 log.warn("{} Result contains a proxy count of zero, disallowing use", getLogPrefix());
                 ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.REQUEST_UNSUPPORTED);
                 return false;
-            } else if (!proxied.getAudiences().isEmpty() &&
+            } 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());
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java
index 0bd663c..947a631 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/FinalizeAuthenticationTest.java
@@ -35,6 +35,7 @@ import net.shibboleth.idp.authn.principal.impl.ExactPrincipalEvalPredicateFactor
 import net.shibboleth.idp.profile.ActionTestingSupport;
 import net.shibboleth.idp.profile.IdPEventIds;
 import net.shibboleth.idp.profile.context.RelyingPartyContext;
+import net.shibboleth.idp.saml.saml2.profile.config.BrowserSSOProfileConfiguration;
 
 import org.springframework.webflow.execution.Event;
 import org.testng.Assert;
@@ -49,6 +50,8 @@ public class FinalizeAuthenticationTest extends BaseAuthenticationContextTest {
     @BeforeMethod public void setUp() throws Exception {
         super.setUp();
         
+        prc.getSubcontext(RelyingPartyContext.class).setProfileConfig(new BrowserSSOProfileConfiguration());
+        
         action = new FinalizeAuthentication();
         action.initialize();
     }

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


More information about the commits mailing list