[java-idp-oidc] branch main updated: JOIDC-171 - Support unregistered client policies in userinfo/token/introspection/revocation

Henri Mikkonen henri.mikkonen at iki.fi
Tue Sep 5 08:56:31 UTC 2023


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=f8fb17acd4b2cc7f7894614dc0f1dc9592086a3b

The following commit(s) were added to refs/heads/main by this push:
     new f8fb17ac JOIDC-171 - Support unregistered client policies in userinfo/token/introspection/revocation
f8fb17ac is described below

commit f8fb17acd4b2cc7f7894614dc0f1dc9592086a3b
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Tue Sep 5 11:55:24 2023 +0300

    JOIDC-171 - Support unregistered client policies in userinfo/token/introspection/revocation
    
    https://shibboleth.atlassian.net/browse/JOIDC-171
    
    Added client ID validation against policy to the token endpoint. The validation is done if
    metadata was not resolved to the requesting RP.
---
 .../impl/ValidateClientIDAgainstPolicy.java        |  3 +-
 .../idp/flows/oidc/token/token-beans.xml           |  5 +++
 .../shibboleth/idp/flows/oidc/token/token-flow.xml |  1 +
 .../flow/ClientCredentialsTokenFlowTest.java       |  6 +++-
 .../plugin/oidc/op/profile/flow/TokenFlowTest.java | 40 +++++++++++++++++++++-
 .../src/test/resources/credentials/htpasswd.txt    |  2 ++
 .../idp/module/conf/attribute-filter.xml           |  1 +
 .../shibboleth/idp/module/conf/relying-party.xml   |  2 +-
 8 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateClientIDAgainstPolicy.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateClientIDAgainstPolicy.java
index 34620603..25ee34e3 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateClientIDAgainstPolicy.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateClientIDAgainstPolicy.java
@@ -135,7 +135,8 @@ public class ValidateClientIDAgainstPolicy extends AbstractProfileAction {
 
         policies = unregisteredClientPolicyLookupStrategy.apply(profileRequestContext);
         if (policies == null) {
-            log.debug("{} No policy defined", getLogPrefix());
+            log.warn("{} No policy defined and no OIDC metadata context populated", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.ACCESS_DENIED);
             return false;
         }
         return true;
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
index 9787483f..641d4e61 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
@@ -37,6 +37,11 @@
         class="net.shibboleth.idp.plugin.oidc.op.profile.impl.InitializeOutboundTokenResponseMessageContext"
         scope="prototype" />
 
+    <bean id="ValidateClientIDAgainstPolicy"
+        class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.ValidateClientIDAgainstPolicy"
+        p:clientIDLookupStrategy-ref="shibboleth.ClientIDLookupStrategy"
+        scope="prototype" />
+
     <bean id="ValidateGrantType" class="net.shibboleth.idp.plugin.oidc.op.profile.impl.ValidateGrantType"
         scope="prototype" />
 
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-flow.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-flow.xml
index 7add378e..e669ae99 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-flow.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-flow.xml
@@ -34,6 +34,7 @@
     <!-- Authentication subflow happens here. -->
 
     <action-state id="ResumeAfterAuthentication">
+        <evaluate expression="ValidateClientIDAgainstPolicy" />
         <evaluate expression="ValidateGrantType" />
         <evaluate expression="'proceed'" />
         
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/ClientCredentialsTokenFlowTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/ClientCredentialsTokenFlowTest.java
index 0ef5017f..218c06fc 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/ClientCredentialsTokenFlowTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/ClientCredentialsTokenFlowTest.java
@@ -138,6 +138,7 @@ public class ClientCredentialsTokenFlowTest extends AbstractOidcClientAuthentica
     
     @Test
     public void testNoScopeUnverifiedClient() throws Exception {
+        final String clientId = "policyAcceptedClient1";
         setHttpFormRequest("POST", createRequestParameters(clientId, scope, resource));
         setBasicAuth(clientId, clientSecret);
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
@@ -151,6 +152,7 @@ public class ClientCredentialsTokenFlowTest extends AbstractOidcClientAuthentica
 
     @Test
     public void testNoScopeUnverifiedClientBadAudience() throws Exception {
+        final String clientId = "policyAcceptedClient1";
         setHttpFormRequest("POST", createRequestParameters(clientId, scope, resource + "/invalid"));
         setBasicAuth(clientId, clientSecret);
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
@@ -194,6 +196,7 @@ public class ClientCredentialsTokenFlowTest extends AbstractOidcClientAuthentica
 
     @Test
     public void testRequestedScopeUnverifiedClient() throws Exception {
+        final String clientId = "policyAcceptedClient1";
         setHttpFormRequest("POST", createRequestParameters(clientId, scope, resource));
         setBasicAuth(clientId, clientSecret);
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
@@ -237,6 +240,7 @@ public class ClientCredentialsTokenFlowTest extends AbstractOidcClientAuthentica
 
     @Test
     public void testRequestedScopeJWTUnverifiedClient() throws Exception {
+        final String clientId = "policyAcceptedClient1";
         setHttpFormRequest("POST", createRequestParameters(clientId, scope, resource));
         storeMetadata(storageService, resource, null, null);
         setBasicAuth(clientId, clientSecret);
@@ -428,7 +432,7 @@ public class ClientCredentialsTokenFlowTest extends AbstractOidcClientAuthentica
        assertEquals(claims.getSubject(), cid);
        if (customClaims != null) {
            for (final String c : customClaims) {
-               assertNotNull(claims.getClaim(c));
+               assertNotNull(claims.getClaim(c), "The claim " + c + " should not be null");
            }
        }
    }
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/TokenFlowTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/TokenFlowTest.java
index 2195a7e4..90ce2644 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/TokenFlowTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/TokenFlowTest.java
@@ -141,7 +141,7 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
         assertErrorCode(result, OAuth2Error.INVALID_CLIENT_CODE);
     }
-    
+
     @Test
     public void testUnauthorizedGrant() throws IOException, ParseException {
         setHttpFormRequest("POST", createRequestParameters(redirectUri, "authorization_code", "mockCode", clientId));
@@ -233,6 +233,44 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
         }
     }
 
+    @Test
+    public void testValidGrant_unregisterdClient_policyCompliant() throws Exception {
+        final String clientId = "policyAcceptedClient1";
+        setHttpFormRequest("POST", createRequestParameters(redirectUri, "authorization_code",
+                buildAuthorizationCode(clientId), clientId));
+        setBasicAuth(clientId, clientSecret);
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        final OIDCTokenResponse response = parseSuccessResponse(result, OIDCTokenResponse.class);
+        Assert.assertNotNull(response.getTokens().getAccessToken());
+        // test that the other requested scopes (profile email offline_access) are stripped out
+        Assert.assertEquals(response.getTokens().getAccessToken().getScope().toString(), "openid");
+        Assert.assertNull(response.getTokens().getRefreshToken());
+        Assert.assertNotNull(response.getOIDCTokens().getIDToken());
+        Assert.assertNotNull(response.getOIDCTokens().getIDToken().getJWTClaimsSet().getClaim("at_hash"));
+        Assert.assertNull(getSidFromAccessToken(response.getTokens().getAccessToken()));
+        Assert.assertNull(getSidFromJWT(response.getOIDCTokens().getIDToken()));
+    }
+
+    @Test
+    public void testValidGrant_unregisterdClient_policyNonCompliantClientID() throws Exception {
+        final String clientId = "mockClientId";
+        setHttpFormRequest("POST", createRequestParameters(redirectUri, "authorization_code",
+                buildAuthorizationCode(clientId), clientId));
+        setBasicAuth(clientId, clientSecret);
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertErrorCode(result, OAuth2Error.ACCESS_DENIED_CODE);
+    }
+
+    @Test
+    public void testValidGrant_unregisterdClient_policyNonCompliantRedirectURI() throws Exception {
+        final String clientId = "policyAcceptedClient1";
+        setHttpFormRequest("POST", createRequestParameters("https://notviapolicy.org/cb", "authorization_code",
+                buildAuthorizationCode(clientId), clientId));
+        setBasicAuth(clientId, clientSecret);
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+    }
+
     @Test
     public void testValidGrantWithPostAuthn() throws Exception {
         for (final String clientId : clientIds) {
diff --git a/idp-oidc-extension-impl/src/test/resources/credentials/htpasswd.txt b/idp-oidc-extension-impl/src/test/resources/credentials/htpasswd.txt
index 876887de..d434b437 100644
--- a/idp-oidc-extension-impl/src/test/resources/credentials/htpasswd.txt
+++ b/idp-oidc-extension-impl/src/test/resources/credentials/htpasswd.txt
@@ -1 +1,3 @@
 mockClientId:$apr1$brkieso1$ipdqb1TlahcXf.Kjanhqu.
+policyAcceptedClient1:$apr1$brkieso1$ipdqb1TlahcXf.Kjanhqu.
+
diff --git a/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/attribute-filter.xml b/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/attribute-filter.xml
index 6ad3c07e..a8e99210 100644
--- a/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/attribute-filter.xml
+++ b/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/attribute-filter.xml
@@ -99,6 +99,7 @@
             <Rule xsi:type="AND">
                 <Rule xsi:type="OR">
                     <Rule xsi:type="Requester" value="mockClientId" />
+                    <Rule xsi:type="Requester" value="policyAcceptedClient1" />
                     <Rule xsi:type="Requester" value="mockSamlClientId" />
                 </Rule>
                 <Rule xsi:type="ProxiedRequester" value="https://rp.example.org" />
diff --git a/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/relying-party.xml b/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/relying-party.xml
index b13e57c1..5ee364ce 100644
--- a/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/relying-party.xml
+++ b/idp-oidc-extension-impl/src/test/resources/net/shibboleth/idp/module/conf/relying-party.xml
@@ -40,7 +40,7 @@
                 <ref bean="OIDC.Registration" />
                 <ref bean="OIDC.Configuration" />
                 <bean parent="OIDC.SSO" p:unregisteredClientPolicyLookupStrategy-ref="shibboleth.oidc.DefaultUnregisteredPolicyLookupStrategy"/>
-                <ref bean="OAUTH2.Token" />
+                <bean parent="OAUTH2.Token" p:unregisteredClientPolicyLookupStrategy-ref="shibboleth.oidc.DefaultUnregisteredPolicyLookupStrategy"/>
                 <bean parent="OAUTH2.TokenAudience" p:encryptionOptional="true" /> 
                 <ref bean="OAUTH2.Introspection" />
                 <ref bean="OAUTH2.Revocation" />

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


More information about the commits mailing list