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

Scott Cantor cantor.2 at osu.edu
Mon Dec 2 22:02:12 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=931fb3078acecd866a6301245ed2a2cda577ed0e

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

commit 931fb3078acecd866a6301245ed2a2cda577ed0e
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Dec 2 22:02:09 2019 -0500

    IDP-1494 - Login flow for proxied SAML authentication
    
    https://issues.shibboleth.net/jira/browse/IDP-1494
    
    Eliminate seperate proxy login flow.
    Move discovery into a callable subflow for use by other login flows.
---
 .../idp/authn/AuthenticationFlowDescriptor.java    | 30 ++++++++++++++-
 .../authn/proxy/impl/ExtractDiscoveryResponse.java |  7 +---
 ...proxy-authn-config.xml => discovery-config.xml} | 31 ++++++++--------
 .../main/resources/conf/authn/general-authn.xml    | 16 +++++---
 .../resources/conf/authn/saml-authn-config.xml     | 10 ++++-
 .../main/resources/system/conf/webflow-config.xml  |  4 +-
 .../system/flows/authn/authn-abstract-flow.xml     | 12 ++++++
 .../{proxy-authn-beans.xml => discovery-beans.xml} | 10 ++---
 .../system/flows/authn/discovery-flow.xml          | 25 +++++++++++++
 .../system/flows/authn/proxy-authn-flow.xml        | 43 ----------------------
 .../system/flows/authn/saml-authn-flow.xml         | 12 +++++-
 .../session/impl/PrepareInboundMessageContext.java |  4 ++
 12 files changed, 124 insertions(+), 80 deletions(-)

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 8f45d20..936b368 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
@@ -84,6 +84,9 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
 
     /** Whether this flow should honor proxy scoping restrictions toward IdPs. */
     private boolean proxyScopingEnforced;
+    
+    /** Whether this flow should invoke discovery if no authenticating authority populated. */
+    private boolean discoveryRequired;
 
     /** Whether this flow allows reuse of its results. */
     @Nonnull private Predicate<ProfileRequestContext> reuseCondition;
@@ -113,7 +116,6 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
     public AuthenticationFlowDescriptor() {
         supportsNonBrowser = true;
         proxyRestrictionsEnforced = true;
-        proxyScopingEnforced = false;
         reuseCondition = new ProxyCountPredicate();
         supportedPrincipals = new Subject();
         activationCondition = Predicates.alwaysTrue();
@@ -228,6 +230,32 @@ public class AuthenticationFlowDescriptor extends AbstractIdentifiableInitializa
     public void setProxyScopingEnforced(final boolean flag) {
         proxyScopingEnforced = flag;
     }
+    
+    /**
+     * Gets whether to invoke discovery subflow if {@link AuthenticationContext#getAuthenticatingAuthority()}
+     * is null.
+     * 
+     * @return whether to invoke discovery
+     * 
+     * @since 4.0.0
+     */
+    public boolean isDiscoveryRequired() {
+        return discoveryRequired;
+    }
+    
+    /**
+     * Sets whether to invoke discovery subflow if {@link AuthenticationContext#getAuthenticatingAuthority()}
+     * is null.
+     * 
+     * <p>Defaults to false.</p>
+     * 
+     * @param flag flag to set
+     * 
+     * @since 4.0.0
+     */
+    public void setDiscoveryRequired(final boolean flag) {
+        discoveryRequired = flag;
+    }
 
     /**
      * Set condition controlling whether results from this flow should be reused for SSO.
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/ExtractDiscoveryResponse.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/ExtractDiscoveryResponse.java
index a05d9c5..6b57360 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/ExtractDiscoveryResponse.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/ExtractDiscoveryResponse.java
@@ -21,11 +21,9 @@ import javax.annotation.Nonnull;
 import javax.servlet.http.HttpServletRequest;
 
 import net.shibboleth.idp.authn.AbstractAuthenticationAction;
-import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
-import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -34,7 +32,6 @@ import org.slf4j.LoggerFactory;
  * An action that extracts a discovery service result and copies it to the {@link AuthenticationContext}.
  * 
  * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
- * @event {@link AuthnEventIds#NO_CREDENTIALS}
  * @pre <pre>ProfileRequestContext.getSubcontext(AuthenticationContext.class) != null</pre>
  * @post If getHttpServletRequest() != null, the content of the "entityID" parameter will be
  * added via {@link AuthenticationContext#setAuthenticatingAuthority(String)}.
@@ -51,8 +48,7 @@ public class ExtractDiscoveryResponse extends AbstractAuthenticationAction {
 
         final HttpServletRequest request = getHttpServletRequest();
         if (request == null) {
-            log.debug("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
+            log.error("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
             return;
         }
 
@@ -60,7 +56,6 @@ public class ExtractDiscoveryResponse extends AbstractAuthenticationAction {
         
         if (entityID == null) {
             log.debug("{} No entityID parameter found", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
             return;
         }
 
diff --git a/idp-conf/src/main/resources/conf/authn/proxy-authn-config.xml b/idp-conf/src/main/resources/conf/authn/discovery-config.xml
similarity index 63%
rename from idp-conf/src/main/resources/conf/authn/proxy-authn-config.xml
rename to idp-conf/src/main/resources/conf/authn/discovery-config.xml
index 531234c..e21e3fd 100644
--- a/idp-conf/src/main/resources/conf/authn/proxy-authn-config.xml
+++ b/idp-conf/src/main/resources/conf/authn/discovery-config.xml
@@ -12,22 +12,23 @@
        default-init-method="initialize"
        default-destroy-method="destroy">
 
-    <!--  General Configuration -->
-
-    <!-- Optional Function<ProfileRequest,String> to supply name of proxied IdP. -->
-    
-    <bean id="shibboleth.authn.Proxy.discoveryFunction" parent="shibboleth.Functions.Constant"
-        c:target="urn:mace:incommon:osu.edu" />
-
-    <!-- Alternatively use a standardized discovery service. -->
+    <!-- Specify discovery service location. -->
     
-    <!--
-    <bean id="shibboleth.authn.Proxy.discoveryURL" class="java.lang.String"
+    <bean id="shibboleth.authn.discoveryURL" class="java.lang.String"
         c:_0="https://ds.example.org/shibboleth-ds/index.html" />
-     -->
-        
-    <util:list id="shibboleth.authn.Proxy.precedence">
-        <value>SAML</value>
-    </util:list>
+ 
+    <!-- Alternatively specify a Function<ProfileRequestContext,String> to return the URL. -->
+    <!--
+    <bean id="shibboleth.authn.discoveryURLStrategy"
+            parent="shibboleth.ContextFunction.Scripted" factory-method="inlineScript">
+        <constructor-arg>
+            <value>
+            <![CDATA[
+                "https://ds.example.org/shibboleth-ds/index.html";
+            ]]>
+            </value>
+        </constructor-arg>
+    </bean>
+    -->
  
 </beans>
diff --git a/idp-conf/src/main/resources/conf/authn/general-authn.xml b/idp-conf/src/main/resources/conf/authn/general-authn.xml
index f3b660f..b936f97 100644
--- a/idp-conf/src/main/resources/conf/authn/general-authn.xml
+++ b/idp-conf/src/main/resources/conf/authn/general-authn.xml
@@ -126,15 +126,19 @@
             </property>
         </bean>
 
-        <bean id="authn/Proxy" parent="shibboleth.AuthenticationFlow"
-                p:nonBrowserSupported="false"
-                p:passiveAuthenticationSupported="true"
-                p:forcedAuthenticationSupported="true"
-                p:proxyScopingEnforced="true" />
+        <!-- This is a flow for proxied SAML authentication to another IdP. -->
+
+        <bean id="authn/SAML" parent="shibboleth.AuthenticationFlow"
+            p:nonBrowserSupported="false"
+            p:passiveAuthenticationSupported="true"
+            p:forcedAuthenticationSupported="true"
+            p:proxyScopingEnforced="true"
+            p:discoveryRequired="true" />
 
         <!--
         These flows are often, though not exclusively, used to proxy authentication, so may need
-        the proxyScopingEnforced property enabled by hand to honor RP/local proxy count limits.
+        the proxyScopingEnforced flag enabled by hand to honor RP/local proxy count limits,
+        and may optionally trigger discovery via the discoveryRequired flag.
         -->
 
         <bean id="authn/External" parent="shibboleth.AuthenticationFlow"
diff --git a/idp-conf/src/main/resources/conf/authn/saml-authn-config.xml b/idp-conf/src/main/resources/conf/authn/saml-authn-config.xml
index 72ed977..23c6992 100644
--- a/idp-conf/src/main/resources/conf/authn/saml-authn-config.xml
+++ b/idp-conf/src/main/resources/conf/authn/saml-authn-config.xml
@@ -12,7 +12,15 @@
        default-init-method="initialize"
        default-destroy-method="destroy">
 
-
+    <!--
+    Optional Function<ProfileRequest,String> to supply name of proxied IdP,
+    otherwise flow assumes IdP discovery has been performed already.
+    -->
+    <!--
+    <bean id="shibboleth.authn.SAML.discoveryFunction" parent="shibboleth.Functions.Constant"
+        c:target="https://idp.example.org/idp/shibboleth" />
+    -->
+    
     <!--
     Add authentication flow descriptor's supportedPrincipals collection to the
     resulting Subject? This may be problematic if it happens without regard for
diff --git a/idp-conf/src/main/resources/system/conf/webflow-config.xml b/idp-conf/src/main/resources/system/conf/webflow-config.xml
index 5f65c3c..daec687 100644
--- a/idp-conf/src/main/resources/system/conf/webflow-config.xml
+++ b/idp-conf/src/main/resources/system/conf/webflow-config.xml
@@ -86,9 +86,11 @@
                 <entry key="authn/Duo" value="../system/flows/authn/duo-authn-flow.xml" />
                 <entry key="authn/MFA" value="../system/flows/authn/mfa-authn-flow.xml" />
                 <entry key="authn/Function" value="../system/flows/authn/function-authn-flow.xml" />
-                <entry key="authn/Proxy" value="../system/flows/authn/proxy-authn-flow.xml" />
                 <entry key="authn/SAML" value="../system/flows/authn/saml-authn-flow.xml" />
           
+                <!-- One-off flow for discovery. -->
+                <entry key="authn/Discovery" value="../system/flows/authn/discovery-flow.xml" />
+          
                 <!-- Master flow for subject c14n. -->
                 <entry key="c14n.events" value="../conf/c14n/subject-c14n-events-flow.xml" />
                 <entry key="c14n.abstract" value="../system/flows/c14n/subject-c14n-abstract-flow.xml" />
diff --git a/idp-conf/src/main/resources/system/flows/authn/authn-abstract-flow.xml b/idp-conf/src/main/resources/system/flows/authn/authn-abstract-flow.xml
index 1531b04..5491c5b 100644
--- a/idp-conf/src/main/resources/system/flows/authn/authn-abstract-flow.xml
+++ b/idp-conf/src/main/resources/system/flows/authn/authn-abstract-flow.xml
@@ -39,6 +39,18 @@
     <end-state id="SubjectCanonicalizationError" />
     <end-state id="InvalidCSRFToken" />
 
+    <!-- Support for discovery implemented via branch to DoDiscovery action. -->
+    <decision-state id="DoDiscovery">
+        <if test="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.authn.context.AuthenticationContext)).getAuthenticatingAuthority() == null"
+            then="CallDiscovery"
+            else="PostDiscovery" />
+    </decision-state>
+    
+    <subflow-state id="CallDiscovery" subflow="authn/Discovery">
+        <input name="calledAsSubflow" value="true" />
+        <transition on="proceed" to="PostDiscovery" />
+    </subflow-state>
+  
     <action-state id="LogRuntimeException">
         <evaluate expression="T(org.slf4j.LoggerFactory).getLogger('net.shibboleth.idp.authn').error('Uncaught runtime exception', flowExecutionException.getCause() ?: flowExecutionException)" />
         <transition to="RuntimeException" />
diff --git a/idp-conf/src/main/resources/system/flows/authn/proxy-authn-beans.xml b/idp-conf/src/main/resources/system/flows/authn/discovery-beans.xml
similarity index 83%
rename from idp-conf/src/main/resources/system/flows/authn/proxy-authn-beans.xml
rename to idp-conf/src/main/resources/system/flows/authn/discovery-beans.xml
index 32396bb..27fa15f 100644
--- a/idp-conf/src/main/resources/system/flows/authn/proxy-authn-beans.xml
+++ b/idp-conf/src/main/resources/system/flows/authn/discovery-beans.xml
@@ -19,16 +19,14 @@
     <bean class="net.shibboleth.idp.profile.impl.ProfileActionBeanPostProcessor" />
 
     <!-- Default strategy function to obtain the external path. -->
-    <bean id="shibboleth.authn.Proxy.discoveryURLStrategy" parent="shibboleth.Functions.Constant" lazy-init="true"
-        c:target-ref="shibboleth.authn.Proxy.discoveryURL" />
+    <bean id="shibboleth.authn.discoveryURLStrategy" parent="shibboleth.Functions.Constant" lazy-init="true"
+        c:target-ref="shibboleth.authn.discoveryURL" />
 
-    
-
-    <import resource="../../../conf/authn/proxy-authn-config.xml" />
+    <import resource="../../../conf/authn/discovery-config.xml" />
 
     <bean id="DiscoveryProfileRequestFunction" lazy-init="true"
         class="net.shibboleth.idp.authn.proxy.impl.DiscoveryProfileRequestFunction"
-        p:discoveryURLLookupStrategy-ref="shibboleth.authn.Proxy.discoveryURLStrategy" />
+        p:discoveryURLLookupStrategy-ref="shibboleth.authn.discoveryURLStrategy" />
 
     <!-- Action beans. -->
 
diff --git a/idp-conf/src/main/resources/system/flows/authn/discovery-flow.xml b/idp-conf/src/main/resources/system/flows/authn/discovery-flow.xml
new file mode 100644
index 0000000..05d451b
--- /dev/null
+++ b/idp-conf/src/main/resources/system/flows/authn/discovery-flow.xml
@@ -0,0 +1,25 @@
+<flow xmlns="http://www.springframework.org/schema/webflow"
+      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+      xsi:schemaLocation="http://www.springframework.org/schema/webflow http://www.springframework.org/schema/webflow/spring-webflow.xsd"
+      parent="authn.abstract">
+
+    <!-- This is a one-off flow for performing IdP Discovery using a standard SAML discovery service. -->
+
+    <on-start>
+        <evaluate expression="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.authn.context.AuthenticationContext)).setAuthenticatingAuthority(null)" />
+    </on-start>
+
+    <view-state id="IssueDiscoveryRequest" view="externalRedirect:#{DiscoveryProfileRequestFunction.apply(new net.shibboleth.utilities.java.support.collection.Pair(flowRequestContext, opensamlProfileRequestContext))}">
+        <attribute name="csrf_excluded" value="true" type="boolean"/>
+        <transition on="proceed" to="ExtractDiscoveryResponse" />
+    </view-state>
+    
+    <action-state id="ExtractDiscoveryResponse">
+        <evaluate expression="ExtractDiscoveryResponse" />
+        <evaluate expression="'proceed'" />
+        
+        <transition on="proceed" to="proceed" />
+    </action-state>
+    
+    <bean-import resource="discovery-beans.xml" />
+</flow>
diff --git a/idp-conf/src/main/resources/system/flows/authn/proxy-authn-flow.xml b/idp-conf/src/main/resources/system/flows/authn/proxy-authn-flow.xml
deleted file mode 100644
index 94de98c..0000000
--- a/idp-conf/src/main/resources/system/flows/authn/proxy-authn-flow.xml
+++ /dev/null
@@ -1,43 +0,0 @@
-<flow xmlns="http://www.springframework.org/schema/webflow"
-      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-      xsi:schemaLocation="http://www.springframework.org/schema/webflow http://www.springframework.org/schema/webflow/spring-webflow.xsd"
-      parent="authn.abstract">
-
-    <!-- This is a login flow for oroxied authentication implemented via protocol-specific subflows. -->
-
-    <on-start>
-        <set name="flowScope.protocolIterator" value="flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.authn.Proxy.precedence').iterator()" />
-        <evaluate expression="flowRequestContext.getActiveFlow().getApplicationContext().containsBean('shibboleth.authn.Proxy.discoveryFunction') ? opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.authn.context.AuthenticationContext)).setAuthenticatingAuthority(flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.authn.Proxy.discoveryFunction').apply(opensamlProfileRequestContext)) : null" />
-    </on-start>
-
-    <decision-state id="CheckDiscoveryDone">
-        <if test="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.authn.context.AuthenticationContext)).getAuthenticatingAuthority() == null"
-            then="DoDiscovery"
-            else="SelectProxyFlow" />
-    </decision-state>
-
-    <view-state id="DoDiscovery" view="externalRedirect:#{DiscoveryProfileRequestFunction.apply(new net.shibboleth.utilities.java.support.collection.Pair(flowRequestContext, opensamlProfileRequestContext))}">
-        <attribute name="csrf_excluded" value="true" type="boolean"/>
-        <transition on="proceed" to="ExtractDiscoveryResponse" />
-    </view-state>
-    
-    <action-state id="ExtractDiscoveryResponse">
-        <evaluate expression="ExtractDiscoveryResponse" />
-        <evaluate expression="'proceed'" />
-        
-        <transition on="proceed" to="SelectProxyFlow" />
-        <transition to="ReselectFlow" />
-    </action-state>
-    
-    <decision-state id="SelectProxyFlow">
-        <if test="protocolIterator.hasNext()" then="CallProxyFlow" else="ReselectFlow" />
-    </decision-state>
-    
-    <subflow-state id="CallProxyFlow" subflow="authn/#{protocolIterator.next()}">
-        <input name="calledAsSubflow" value="true" />
-        <transition on="proceed" to="proceed" />
-        <transition on="ReselectFlow" to="SelectProxyFlow" />
-    </subflow-state>
-
-    <bean-import resource="proxy-authn-beans.xml" />
-</flow>
diff --git a/idp-conf/src/main/resources/system/flows/authn/saml-authn-flow.xml b/idp-conf/src/main/resources/system/flows/authn/saml-authn-flow.xml
index 239594a..133aa5b 100644
--- a/idp-conf/src/main/resources/system/flows/authn/saml-authn-flow.xml
+++ b/idp-conf/src/main/resources/system/flows/authn/saml-authn-flow.xml
@@ -5,7 +5,17 @@
 
     <!-- This is a login flow for oroxied authentication implemented via SAML 2.0. -->
 
-    <action-state id="IssueRequest">
+    <on-start>
+        <evaluate expression="flowRequestContext.getActiveFlow().getApplicationContext().containsBean('shibboleth.authn.SAML.discoveryFunction') ? opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.authn.context.AuthenticationContext)).setAuthenticatingAuthority(flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.authn.SAML.discoveryFunction').apply(opensamlProfileRequestContext)) : null" />
+    </on-start>
+
+    <decision-state id="CheckDiscovery">
+        <if test="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.authn.context.AuthenticationContext)).getAttemptedFlow().isDiscoveryRequired()"
+            then="DoDiscovery"
+            else="PostDiscovery"/>
+    </decision-state>
+
+    <action-state id="PostDiscovery">
         <evaluate expression="InitializeProxyProfileRequestContext" />
         <evaluate expression="FlowStartPopulateAuditContext" />
         
diff --git a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/session/impl/PrepareInboundMessageContext.java b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/session/impl/PrepareInboundMessageContext.java
index 17febf8..80b2470 100644
--- a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/session/impl/PrepareInboundMessageContext.java
+++ b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/session/impl/PrepareInboundMessageContext.java
@@ -110,6 +110,10 @@ public class PrepareInboundMessageContext extends AbstractProfileAction {
             if (relyingPartyId != null) {
                 return true;
             }
+            
+            log.warn("{} No relying party ID returned from lookup function", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            return false;
         }
         
         final LogoutPropagationContext logoutPropCtx = logoutPropContextLookupStrategy.apply(profileRequestContext);

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


More information about the commits mailing list