[java-idp-oidc] 02/02: JOIDC-31 Adjust order and whether to try SAML metadata lookup

Henri Mikkonen henri.mikkonen at iki.fi
Fri Feb 19 10:21:02 UTC 2021


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

hjmikkon pushed a commit to branch main
in repository java-idp-oidc.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-oidc.git;a=commit;h=75b759ab4955e177d3b355c8adcb94a02232b8a6

commit 75b759ab4955e177d3b355c8adcb94a02232b8a6
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Feb 19 12:20:42 2021 +0200

    JOIDC-31 Adjust order and whether to try SAML metadata lookup
    
    https://issues.shibboleth.net/jira/browse/JOIDC-31
---
 .../idp/flows/oidc/authorize/authorize-flow.xml    |  1 -
 .../oidc/metadata-lookup/metadata-lookup-beans.xml |  7 +++---
 .../oidc/metadata-lookup/metadata-lookup-flow.xml  | 25 +++++++++++++---------
 .../idp/plugin/oidc/op/conf/oidc.properties        |  3 +++
 .../oidc/op/profile/flow/AuthorizeFlowTest.java    | 12 +++++++++++
 5 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-flow.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-flow.xml
index 92e0009a..557d82b9 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-flow.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-flow.xml
@@ -18,7 +18,6 @@
     </action-state>
 
     <action-state id="SelectConfiguration">
-        <evaluate expression="InitializeRelyingPartyContext" />
         <evaluate expression="SelectRelyingPartyConfiguration" />
         <evaluate expression="SelectProfileConfiguration" />
         <evaluate expression="PostLookupPopulateAuditContext" />
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-beans.xml
index 6343d67a..73f440d0 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-beans.xml
@@ -12,7 +12,7 @@
 
     <bean id="SAMLProtocolAndRole"
             class="net.shibboleth.idp.profile.impl.WebFlowMessageHandlerAdaptor" scope="prototype"
-            c:executionDirection="INBOUND">
+            c:executionDirection="INBOUND" p:activationCondition-ref="%{idp.oidc.metadata.saml:shibboleth.Conditions.TRUE}">
         <constructor-arg name="messageHandler">
             <bean class="org.opensaml.saml.common.binding.impl.SAMLProtocolAndRoleHandler" scope="prototype"
                 p:protocol="http://openid.net/specs/openid-connect-core-1_0.html"
@@ -23,7 +23,8 @@
     <bean id="SetEntityIdToSAMLPeerEntityContext"
         class="net.shibboleth.idp.plugin.oidc.op.profile.impl.SetEntityIdToSAMLPeerEntityContext"
         p:clientIDLookupStrategy-ref="shibboleth.ClientIDLookupStrategy"
-        p:entityContextClass="org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext" />
+        p:entityContextClass="org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext"
+        p:activationCondition-ref="%{idp.oidc.metadata.saml:shibboleth.Conditions.TRUE}" />
 
     <bean id="InitializeRelyingPartyContextFromSAMLPeer"
         class="net.shibboleth.idp.saml.profile.impl.InitializeRelyingPartyContextFromSAMLPeer" scope="prototype" />
@@ -34,7 +35,7 @@
         
     <bean id="SAMLMetadataLookup"
         class="net.shibboleth.idp.profile.impl.WebFlowMessageHandlerAdaptor" scope="prototype"
-        c:executionDirection="INBOUND">
+        c:executionDirection="INBOUND" p:activationCondition-ref="%{idp.oidc.metadata.saml:shibboleth.Conditions.TRUE}">
         <constructor-arg name="messageHandler">
             <bean class="org.opensaml.saml.common.binding.impl.SAMLMetadataLookupHandler" scope="prototype"
                 p:entityContextClass="org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext">
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-flow.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-flow.xml
index 1f25b99c..5e00e3f5 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-flow.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/metadata-lookup/metadata-lookup-flow.xml
@@ -5,9 +5,21 @@
 
     <action-state id="DoMetadataLookup">
         <evaluate expression="'proceed'" />
-        <transition on="proceed" to="LookupFromSAMLMetadataService" />
+        <transition on="proceed" to="LookupFromClientInformationService" />
     </action-state>
     
+    <action-state id="LookupFromClientInformationService">
+        <evaluate expression="OIDCMetadataLookup" />
+        <evaluate expression="InitializeRelyingPartyContext" />
+        <evaluate expression="'proceed'" />
+        <transition on="proceed" to="CheckIfFoundFromClientInformationService" />
+    </action-state>
+
+    <decision-state id="CheckIfFoundFromClientInformationService">
+        <if test="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.profile.context.RelyingPartyContext)).isVerified()"
+            then="SelectConfiguration" else="LookupFromSAMLMetadataService" />
+    </decision-state>
+
     <action-state id="LookupFromSAMLMetadataService">
         <evaluate expression="SAMLProtocolAndRole" />
         <evaluate expression="SetEntityIdToSAMLPeerEntityContext" />
@@ -17,8 +29,8 @@
     </action-state>        
 
     <decision-state id="CheckIfFoundFromSAMLMetadata">
-        <if test="opensamlProfileRequestContext.getInboundMessageContext().getSubcontext(T(org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext)).containsSubcontext(T(org.opensaml.saml.common.messaging.context.SAMLMetadataContext))"
-            then="PopulateOIDCMetadataContextFromSAML" else="LookupFromClientInformationService" />
+        <if test="opensamlProfileRequestContext.getInboundMessageContext().containsSubcontext(T(org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext)) and opensamlProfileRequestContext.getInboundMessageContext().getSubcontext(T(org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext)).containsSubcontext(T(org.opensaml.saml.common.messaging.context.SAMLMetadataContext))"
+            then="PopulateOIDCMetadataContextFromSAML" else="SelectConfiguration" />
     </decision-state>
     
     <action-state id="PopulateOIDCMetadataContextFromSAML">
@@ -27,13 +39,6 @@
         <evaluate expression="'proceed'" />
         <transition on="proceed" to="SelectConfiguration" />
     </action-state>
-
-    <action-state id="LookupFromClientInformationService">        
-        <evaluate expression="OIDCMetadataLookup" />
-        <evaluate expression="InitializeRelyingPartyContext" />
-        <evaluate expression="'proceed'" />
-        <transition on="proceed" to="SelectConfiguration" />
-    </action-state>
     
     <bean-import resource="../../oidc/metadata-lookup/metadata-lookup-beans.xml" />
         
diff --git a/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties b/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties
index a75cc438..da44fdbc 100644
--- a/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties
+++ b/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties
@@ -66,3 +66,6 @@ idp.oidc.subject.sourceAttribute = uid
 # Do *NOT* share the salt with other people, it's like divulging your private key.
 # It is suggested you move this property into credentials/secrets.properties
 idp.oidc.subject.salt = this_too_should_be_ch4ng3d
+
+# Bean to determine whether SAML metadata should be exploited for trusted OIDC RP resolution
+#idp.oidc.metadata.saml = shibboleth.Conditions.TRUE
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java
index cd0fe9a1..9aa2142c 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java
@@ -131,6 +131,18 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
         Assert.assertNull(successResponse.getAccessToken());
         Assert.assertNotNull(successResponse.getAuthorizationCode());
     }
+    
+    @Test
+    public void testWithAuthorizationCodeFlowUsingUntrustedRP() throws IOException, ParseException, SessionException {
+        request.setMethod("GET");
+        request.setQueryString("client_id=notTrusted&response_type=code&scope=openid%20profile&redirect_uri="
+                + redirectUri);
+        initializeThreadLocals();
+
+        FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        Assert.assertEquals(result.getOutcome().getId(), "ErrorView");
+    }
 
     @AfterMethod
     public void removeMetadata() throws IOException {

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


More information about the commits mailing list