[java-identity-provider] branch main updated: IDP-2196 - Use of a discovery service fails with Administrative flows

Scott Cantor cantor.2 at osu.edu
Wed Nov 29 18:20:50 UTC 2023


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

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

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

The following commit(s) were added to refs/heads/main by this push:
     new d8435882d IDP-2196 - Use of a discovery service fails with Administrative flows
d8435882d is described below

commit d8435882dd1cff1469b069b0482ec8ed000a8854
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Nov 29 13:20:46 2023 -0500

    IDP-2196 - Use of a discovery service fails with Administrative flows
    
    https://shibboleth.atlassian.net/browse/IDP-2196
    
    Revamps determination of entityID for DS request.
    Admin flows use new idp.admin.entityID or fall back to usual property.
    Advanced cases can use a strategy function if needed.
---
 .../admin/BasicAdministrativeFlowDescriptor.java   | 41 +++++++++++++-
 .../impl/DiscoveryProfileRequestFunction.java      | 62 ++++++++++------------
 .../net/shibboleth/idp/conf/admin-system.xml       |  2 +
 .../shibboleth/idp/flows/authn/discovery-beans.xml |  3 +-
 4 files changed, 73 insertions(+), 35 deletions(-)

diff --git a/idp-admin-api/src/main/java/net/shibboleth/idp/admin/BasicAdministrativeFlowDescriptor.java b/idp-admin-api/src/main/java/net/shibboleth/idp/admin/BasicAdministrativeFlowDescriptor.java
index a6fc01f34..ad4e4fc73 100644
--- a/idp-admin-api/src/main/java/net/shibboleth/idp/admin/BasicAdministrativeFlowDescriptor.java
+++ b/idp-admin-api/src/main/java/net/shibboleth/idp/admin/BasicAdministrativeFlowDescriptor.java
@@ -40,6 +40,7 @@ import com.google.common.base.MoreObjects;
 
 import net.shibboleth.idp.authn.principal.PrincipalServiceManager;
 import net.shibboleth.idp.profile.config.AbstractInterceptorAwareProfileConfiguration;
+import net.shibboleth.profile.config.OverriddenIssuerProfileConfiguration;
 import net.shibboleth.shared.annotation.ParameterName;
 import net.shibboleth.shared.annotation.constraint.NonNegative;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
@@ -65,11 +66,14 @@ import net.shibboleth.shared.primitive.StringSupport;
  * @since 3.3.0
  */
 public class BasicAdministrativeFlowDescriptor extends AbstractInterceptorAwareProfileConfiguration
-        implements AdministrativeFlowDescriptor {
+        implements AdministrativeFlowDescriptor, OverriddenIssuerProfileConfiguration {
     
     /** Logging ID. */
     @Nullable private String loggingId;
     
+    /** Issuer lookup strategy. */
+    @Nonnull private Function<ProfileRequestContext,String> issuerLookupStrategy;
+    
     /** Whether this flow supports non-browser clients. */
     @Nonnull private Predicate<ProfileRequestContext> supportsNonBrowserPredicate;
 
@@ -121,6 +125,8 @@ public class BasicAdministrativeFlowDescriptor extends AbstractInterceptorAwareP
     public BasicAdministrativeFlowDescriptor(@Nonnull @NotEmpty @ParameterName(name="id") final String id) {
         super(id);
         
+        issuerLookupStrategy = FunctionSupport.constant(null);
+        
         supportsNonBrowserPredicate = PredicateSupport.alwaysTrue();
         authenticatedPredicate = PredicateSupport.alwaysFalse();
         policyNameLookupStrategy = FunctionSupport.constant(null);
@@ -163,6 +169,37 @@ public class BasicAdministrativeFlowDescriptor extends AbstractInterceptorAwareP
         loggingId = StringSupport.trimOrNull(id);
     }
     
+    /**
+     * {@inheritDoc}
+     * 
+     * @since 5.1.0
+     */
+    @Nullable public String getIssuer(@Nullable final ProfileRequestContext profileRequestContext) {
+        return issuerLookupStrategy.apply(profileRequestContext);
+    }
+    
+    /**
+     * Set the issuer value to use with this profile.
+     * 
+     * @param id issuer value
+     * 
+     * @since 5.1.0
+     */
+    public void setIssuer(@Nullable final String id) {
+        issuerLookupStrategy = FunctionSupport.constant(StringSupport.trimOrNull(id));
+    }
+
+    /**
+     * Set the lookup strategy for the issuer value to use with this profile.
+     * 
+     * @param strategy lookup strategy
+     * 
+     * @since 5.1.0
+     */
+    public void setIssuerLookupStrategy(@Nonnull final Function<ProfileRequestContext,String> strategy) {
+        issuerLookupStrategy = Constraint.isNotNull(strategy, "Issuer lookup strategy cannot be null");
+    }
+    
     /** {@inheritDoc} */
     public boolean isNonBrowserSupported(@Nullable final ProfileRequestContext profileRequestContext) {
         return supportsNonBrowserPredicate.test(profileRequestContext);
@@ -655,4 +692,6 @@ public class BasicAdministrativeFlowDescriptor extends AbstractInterceptorAwareP
         }
     }
 
+    /** {@inheritDoc} */
+
 }
\ No newline at end of file
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/DiscoveryProfileRequestFunction.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/DiscoveryProfileRequestFunction.java
index 7c9fc37d2..16cc73e08 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/DiscoveryProfileRequestFunction.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/proxy/impl/DiscoveryProfileRequestFunction.java
@@ -21,7 +21,6 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
 
-import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.springframework.webflow.execution.RequestContext;
 
@@ -30,8 +29,7 @@ import com.google.common.net.UrlEscapers;
 
 import jakarta.servlet.http.HttpServletRequest;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
-import net.shibboleth.profile.context.RelyingPartyContext;
-import net.shibboleth.profile.relyingparty.RelyingPartyConfiguration;
+import net.shibboleth.profile.context.navigate.IssuerLookupFunction;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
@@ -39,11 +37,7 @@ import net.shibboleth.shared.logic.Constraint;
 
 /**
  * A {@link Function} that produces a discovery request URL using the protocol defined in
- * https://wiki.oasis-open.org/security/IdpDiscoSvcProtonProfile
- * 
- * <p>Since there is no upstream "relying party" yet, the identity of the system is derived
- * from the currently in-effect entityID that will be used to respond to the downstream
- * relying party.</p>
+ * {@linkplain "https://wiki.oasis-open.org/security/IdpDiscoSvcProtonProfile"}.
  */
 @ThreadSafe
 public class DiscoveryProfileRequestFunction extends AbstractInitializableComponent
@@ -52,33 +46,20 @@ public class DiscoveryProfileRequestFunction extends AbstractInitializableCompon
     /** URL query parameter escaper. */
     @Nonnull private Escaper escaper;
     
-    /** Lookup strategy for locating {@link RelyingPartyContext}. */
-    @Nonnull private Function<ProfileRequestContext,RelyingPartyContext> relyingPartyContextLookupStrategy;
-    
     /** Lookup strategy for determining the "base" discovery URL. */
     @NonnullAfterInit private Function<ProfileRequestContext,String> discoveryURLLookupStrategy;
     
     /** Overrides this function via an injected bean. */
     @Nullable private BiFunction<RequestContext,ProfileRequestContext,String> delegatedRequestFunction;
+
+    /** A strategy function to call to obtain the entityID to use when invoking the DS. */
+    @NonnullAfterInit private Function<ProfileRequestContext,String> entityIDLookupStrategy;
     
     /** Constructor. */
     public DiscoveryProfileRequestFunction() {
         final Escaper esc = UrlEscapers.urlFormParameterEscaper();
         assert esc != null;
         escaper = esc;
-        relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class);
-    }
-    
-    /**
-     * Set the lookup strategy for the {@link RelyingPartyContext}.
-     * 
-     * @param strategy lookup strategy
-     */
-    public void setRelyingPartyContextLookupStrategy(
-            @Nonnull final Function<ProfileRequestContext,RelyingPartyContext> strategy) {
-        checkSetterPreconditions();
-        relyingPartyContextLookupStrategy =
-                Constraint.isNotNull(strategy, "RelyingPartyContext lookup strategy cannot be null");
     }
     
     /**
@@ -106,6 +87,21 @@ public class DiscoveryProfileRequestFunction extends AbstractInitializableCompon
         delegatedRequestFunction = delegate;
     }
     
+    /**
+     * Set a lookup strategy for the entityID to use when invoking the DS.
+     * 
+     * <p>In the absence of an alternative source, the default is to obtain the entityID from the
+     * "downstream-facing" profile/RP configurations, which may not result in the correct value.</p>
+     * 
+     * @param strategy lookup strategy
+     * 
+     * @since 5.1.0
+     */
+    public void setEntityIDLookupStrategy(@Nullable final Function<ProfileRequestContext,String> strategy) {
+        checkSetterPreconditions();
+        entityIDLookupStrategy = strategy;
+    }
+    
     /** {@inheritDoc} */
     @Override protected void doInitialize() throws ComponentInitializationException {
         super.doInitialize();
@@ -113,6 +109,10 @@ public class DiscoveryProfileRequestFunction extends AbstractInitializableCompon
         if (discoveryURLLookupStrategy == null) {
             throw new ComponentInitializationException("Discovery URL lookup strategy cannot be null");
         }
+        
+        if (entityIDLookupStrategy == null) {
+            entityIDLookupStrategy = new IssuerLookupFunction();
+        }
     }
     
     /** {@inheritDoc} */
@@ -126,18 +126,14 @@ public class DiscoveryProfileRequestFunction extends AbstractInitializableCompon
         if (springRequestContext == null) {
             throw new IllegalArgumentException("Spring RequestContext cannot be null");
         }
-        
-        final RelyingPartyContext rpCtx = relyingPartyContextLookupStrategy.apply(profileRequestContext);
-        Constraint.isNotNull(rpCtx, "RelyingPartyContext cannot be null");
-        Constraint.isNotNull(rpCtx.getConfiguration(), "RelyingPartyConfiguration cannot be null");
-        
+
+        // We need the entityID to use for the DS request.
+        final String entityID = entityIDLookupStrategy.apply(profileRequestContext);
+        Constraint.isNotNull(entityID, "Unable to obtain entityID to use for DS request");
+
         final String baseURL = discoveryURLLookupStrategy.apply(profileRequestContext);
         Constraint.isNotEmpty(baseURL, "Discovery URL cannot be null or empty");
 
-        final RelyingPartyConfiguration rpConfig = rpCtx.getConfiguration();
-        assert rpConfig!=null;
-        final String entityID =rpConfig.getIssuer(profileRequestContext);
-
         final StringBuilder builder = new StringBuilder(baseURL);
         
         builder.append(baseURL.contains("?") ? '&' : '?').append("entityID=").append(escaper.escape(entityID));
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/admin-system.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/admin-system.xml
index 1d8bb305e..d8b55d95d 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/admin-system.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/admin-system.xml
@@ -233,11 +233,13 @@
     <bean id="shibboleth.AdminFlow" abstract="true"
         class="net.shibboleth.idp.admin.BasicAdministrativeFlowDescriptor"
         p:principalServiceManager-ref="shibboleth.PrincipalServiceManager"
+        p:issuer="%{idp.admin.entityID:%{idp.entityID}}"
         p:nonBrowserSupported="false" />
 
     <bean id="shibboleth.OneTimeAdminFlow" abstract="true"
         class="net.shibboleth.idp.admin.OneTimeAdministrativeFlowDescriptor"
         p:principalServiceManager-ref="shibboleth.PrincipalServiceManager"
+        p:issuer="%{idp.admin.entityID:%{idp.entityID}}"
         p:nonBrowserSupported="false" />
 
     <!-- Function for returning custom access control policies for access to metrics. -->
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/discovery-beans.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/discovery-beans.xml
index d8d3a0e19..5fa07af31 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/discovery-beans.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/authn/discovery-beans.xml
@@ -22,7 +22,8 @@
     <bean id="DiscoveryProfileRequestFunction" lazy-init="true"
         class="net.shibboleth.idp.authn.proxy.impl.DiscoveryProfileRequestFunction"
         p:discoveryURLLookupStrategy="#{getObject('shibboleth.authn.discoveryURLStrategy') ?: getObject('DefaultDiscoveryURLStrategy')}"
-        p:delegatedRequestFunction="#{getObject('shibboleth.authn.discoveryRequestFunction')}" />
+        p:delegatedRequestFunction="#{getObject('shibboleth.authn.discoveryRequestFunction')}"
+        p:entityIDLookupStrategy="#{getObject('shibboleth.authn.entityIDForDiscoveryStrategy')}" />
 
     <!-- Action beans. -->
 

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


More information about the commits mailing list