[java-idp-oidc] branch main updated: JOIDC-150 - Improve configuration for the refresh token issuance

Henri Mikkonen henri.mikkonen at iki.fi
Fri Apr 14 11:50:42 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=db5a9440f4d92a87e5aa44b1bbf1f480931318c3

The following commit(s) were added to refs/heads/main by this push:
     new db5a9440 JOIDC-150 - Improve configuration for the refresh token issuance
db5a9440 is described below

commit db5a9440f4d92a87e5aa44b1bbf1f480931318c3
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Apr 14 14:50:20 2023 +0300

    JOIDC-150 - Improve configuration for the refresh token issuance
    
    https://shibboleth.atlassian.net/browse/JOIDC-150
    
    Updated the refresh token issuance logic into the following:
    - The refreshTokensEnabled -profile configuration option must be true (which is the default)
    - The requested grant must not be client_credentials
      - in practise this currently means that either authorization_code or refresh_token is used
    - If 'openid' scope is used, then also 'offline_access' scope must be used
      - Otherwise, no scope requirements
    
    A custom activation condition can be wired via idp.oauth2.refreshToken.activation -property.
---
 .../impl/SetRefreshTokenToResponseContext.java     |  7 ---
 .../profile/logic/OfflineAccessScopeCondition.java | 70 ++++++++++++++++++++++
 .../idp/flows/oidc/token/token-beans.xml           | 44 ++++++++++----
 .../idp/plugin/oidc/op/conf/oidc.properties        |  5 +-
 .../plugin/oidc/op/profile/flow/TokenFlowTest.java | 58 +++++++++++++++---
 .../impl/SetRefreshTokenToResponseContextTest.java | 19 ------
 6 files changed, 154 insertions(+), 49 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java
index 61ff814a..3478724f 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java
@@ -35,7 +35,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.nimbusds.jwt.JWTClaimsSet;
-import com.nimbusds.openid.connect.sdk.OIDCScopeValue;
 
 import net.shibboleth.idp.plugin.oidc.op.messaging.context.OIDCAuthenticationResponseContext;
 import net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultTokenRevocationLifetimeLookupStrategy;
@@ -210,12 +209,6 @@ public class SetRefreshTokenToResponseContext extends AbstractOIDCResponseAction
             return false;
         }
         
-        if (getOidcResponseContext().getScope() == null ||
-                !getOidcResponseContext().getScope().contains(OIDCScopeValue.OFFLINE_ACCESS)) {
-            log.debug("{} No offline_access scope, nothing to do", getLogPrefix());
-            return false;
-        }
-        
         refreshTokenLifetime = refreshTokenLifetimeLookupStrategy.apply(profileRequestContext);
         if (refreshTokenLifetime == null) {
             log.warn("{} No lifetime supplied for refresh token", getLogPrefix());
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/OfflineAccessScopeCondition.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/OfflineAccessScopeCondition.java
new file mode 100644
index 00000000..7377a3f6
--- /dev/null
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/OfflineAccessScopeCondition.java
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements.  See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.idp.plugin.oidc.op.profile.logic;
+
+import java.util.function.Function;
+import java.util.function.Predicate;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+
+import com.nimbusds.oauth2.sdk.Scope;
+import com.nimbusds.openid.connect.sdk.OIDCScopeValue;
+
+import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.ValidatedScopeLookupFunction;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+
+/**
+ * A predicate returning true if validated scope contains 'offline_access' value.
+ */
+public class OfflineAccessScopeCondition implements Predicate<ProfileRequestContext> {
+
+    /** Lookup strategy for the validated scope. */
+    @Nonnull private Function<ProfileRequestContext, Scope> validatedScopeLookupStrategy;
+
+    /**
+     * Constructor.
+     */
+    public OfflineAccessScopeCondition() {
+        validatedScopeLookupStrategy = new ValidatedScopeLookupFunction();
+    }
+
+    /**
+     * Set the lookup strategy for the validated scope.
+     * 
+     * @param strategy What to set.
+     */
+    public void setValidatedScopeLookupStrategy(final @Nonnull Function<ProfileRequestContext, Scope> strategy) {
+        validatedScopeLookupStrategy = Constraint.isNotNull(strategy, "Lookup strategy cannot be null");
+    }
+    
+    /** {@inheritDoc} */
+    @Override
+    public boolean test(final @Nullable ProfileRequestContext input) {
+        if (input == null) {
+            return false;
+        }
+        final Scope validatedScope = validatedScopeLookupStrategy.apply(input);
+        if (validatedScope == null || validatedScope.isEmpty()) {
+            return false;
+        }
+        return validatedScope.contains(OIDCScopeValue.OFFLINE_ACCESS);
+    }
+}
\ No newline at end of file
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 2d603ca2..1b5ab3bd 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
@@ -366,25 +366,43 @@
     <bean id="SetRefreshTokenToResponseContext"
             class="net.shibboleth.idp.plugin.oidc.op.profile.impl.SetRefreshTokenToResponseContext" scope="prototype"
             c:sealer-ref="#{'%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim()}"
-            p:revocationCache-ref="shibboleth.oidc.RevocationCache">
+            p:revocationCache-ref="shibboleth.oidc.RevocationCache"
+            p:activationCondition-ref="#{'%{idp.oauth2.refreshToken.activation:DefaultRefreshTokenActivationCondition}'.trim()}">
             
-        <property name="activationCondition">
-            <bean parent="shibboleth.Conditions.AND">
-                <constructor-arg>
-                    <list>
-                        <ref bean="IssueIDTokenCondition" />
-                        <bean class="net.shibboleth.oidc.profile.config.logic.RefreshTokensEnabledPredicate" />
-                    </list>
-                </constructor-arg>
-            </bean>
-        </property>
-
         <property name="tokenRevocationLifetimeLookupStrategy">
             <bean class="net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultTokenRevocationLifetimeLookupStrategy"
                 p:clockSkew="%{idp.policy.clockSkew:PT5M}" />
         </property>
     </bean>
 
+    <bean id="DefaultRefreshTokenActivationCondition" parent="shibboleth.Conditions.AND">
+        <constructor-arg>
+            <list>
+                <bean class="net.shibboleth.oidc.profile.config.logic.RefreshTokensEnabledPredicate" />
+                <ref bean="NotClientCredentialsGrantCondition" />
+                <bean parent="shibboleth.Conditions.OR">
+                    <constructor-arg>
+                        <list>
+                            <bean parent="shibboleth.Conditions.NOT">
+                                <constructor-arg>
+                                    <ref bean="IssueIDTokenCondition" />
+                                </constructor-arg>
+                            </bean>
+                            <bean parent="shibboleth.Conditions.AND">
+                                <constructor-arg>
+                                    <list>
+                                        <ref bean="IssueIDTokenCondition" />
+                                        <bean class="net.shibboleth.idp.plugin.oidc.op.profile.logic.OfflineAccessScopeCondition" />
+                                    </list>
+                                </constructor-arg>
+                            </bean>
+                        </list>
+                    </constructor-arg>
+                </bean>
+            </list>
+        </constructor-arg>
+    </bean>
+
     <!-- ID token actions. -->
 
     <bean id="PopulateIDTokenSignatureSigningParameters"
@@ -610,7 +628,7 @@
             <bean parent="shibboleth.Conditions.NOT" c:_0-ref="IssueIDTokenCondition" />
         </property>
     </bean>
-            
+
     <bean id="AudienceEncryptionOptionalPredicate"
         class="net.shibboleth.oidc.profile.config.logic.EncryptionOptionalPredicate"
         p:relyingPartyContextLookupStrategy-ref="AudienceRelyingPartyCreationStrategy" />    
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 2e131d9b..aae37f31 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
@@ -160,4 +160,7 @@ idp.oidc.subject.salt = this_too_should_be_ch4ng3d
 # Bean used to validate audience claim in the JWT authentication.
 #idp.oauth2.jwtAuth.audienceValidator = DefaultAuthenticationAudienceClaimsValidator
 # The default pattern also accepts token endpoint URL as the audience in introspection and revocation endpoints.
-#idp.oauth2.jwtAuth.audienceValidator.endpointTargets = /profile/oauth2/introspection,/profile/oauth2/revocation
\ No newline at end of file
+#idp.oauth2.jwtAuth.audienceValidator.endpointTargets = /profile/oauth2/introspection,/profile/oauth2/revocation
+
+# Bean to determine whether refresh token is issuance is activated
+#idp.oauth2.refreshToken.activation = DefaultRefreshTokenActivationCondition
\ No newline at end of file
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 7b0e54bb..b1a656bb 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
@@ -230,6 +230,46 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
         Assert.assertNull(getSidFromJWT(response.getOIDCTokens().getIDToken()));
     }
 
+    @Test
+    public void testValidGrantNoOpenId() throws Exception {
+        initializeGrantAndRequest(clientId, createRequestParameters(redirectUri, "authorization_code",
+                buildAuthorizationCode(clientId, null, "profile email offline_access"), clientId));
+        storeConsent(storageService, "jdoe", clientId, "mail");
+
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        final AccessTokenResponse response = parseSuccessResponse(result, AccessTokenResponse.class);
+        Assert.assertNotNull(response.getTokens().getAccessToken());
+        Assert.assertNotNull(response.getTokens().getRefreshToken());
+        Assert.assertNull(getSidFromAccessToken(response.getTokens().getAccessToken()));
+        Assert.assertNull(getSidFromRefreshToken(response.getTokens().getRefreshToken()));
+    }
+
+    @Test
+    public void testValidGrantNoOpenIdRefreshTokensDisabledInSSOProfile() throws Exception {
+        final String clientId = "mockClientIdNoRefreshTokensInSSOProfile";
+        initializeGrantAndRequest(clientId, createRequestParameters(redirectUri,
+                "authorization_code",
+                buildAuthorizationCode(clientId, null, "profile email offline_access"), clientId));
+        storeConsent(storageService, "jdoe", clientId, "mail");
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        final AccessTokenResponse response = parseSuccessResponse(result, AccessTokenResponse.class);
+        Assert.assertNotNull(response.getTokens().getAccessToken());
+        Assert.assertNull(response.getTokens().getRefreshToken());
+    }
+
+    @Test
+    public void testValidGrantNoOpenIdRefreshTokensDisabledInTokenProfile() throws Exception {
+        final String clientId = "mockClientIdNoRefreshTokensInTokenProfile";
+        initializeGrantAndRequest(clientId, createRequestParameters(redirectUri,
+                "authorization_code",
+                buildAuthorizationCode(clientId, null, "profile email offline_access"), clientId));
+        storeConsent(storageService, "jdoe", clientId, "mail");
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        final AccessTokenResponse response = parseSuccessResponse(result, AccessTokenResponse.class);
+        Assert.assertNotNull(response.getTokens().getAccessToken());
+        Assert.assertNull(response.getTokens().getRefreshToken());
+    }
+
     @Test
     public void testValidGrantWithSid() throws Exception {
         final String sid = idGenerator.generateIdentifier();
@@ -454,7 +494,8 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
     }
 
     protected String buildAuthorizationCodeWithSid(final String clientId, final String sid) throws Exception {
-        return buildAuthorizationCodeWithSid(clientId, null, null, null, null, sid);
+        return buildAuthorizationCodeWithSid(clientId, null, null, null, null, "openid profile email offline_access",
+                sid);
     }
 
     protected String buildAuthorizationCode(final String clientId, final String verifier) throws Exception {
@@ -476,14 +517,13 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
     protected String buildAuthorizationCode(final String clientId, final String verifier,
             final JSONObject deliveryClaims, final JSONObject deliveryClaimsIDToken,
             final JSONObject deliveryClaimsUserInfo, final String scope) throws Exception {
-        return ValidateGrantTest.buildAuthorizationCode(clientId, "https://op.example.org", "jdoe", "mock",
-                redirectUri, verifier, deliveryClaims, deliveryClaimsIDToken, deliveryClaimsUserInfo, 
-                scope).toString();
+        return buildAuthorizationCodeWithSid(clientId, verifier, deliveryClaims, deliveryClaimsIDToken,
+                deliveryClaimsUserInfo, scope, null);
     }
 
     protected String buildAuthorizationCodeWithSid(final String clientId, final String verifier,
             final JSONObject deliveryClaims, final JSONObject deliveryClaimsIDToken,
-            final JSONObject deliveryClaimsUserInfo, final String sid) throws Exception {
+            final JSONObject deliveryClaimsUserInfo, final String scope, final String sid) throws Exception {
         final AuthorizeCodeClaimsSet.Builder builder = new AuthorizeCodeClaimsSet.Builder();
         builder.setJWTID(new SecureRandomIdentifierGenerationStrategy())
             .setClientID(new ClientID(clientId))
@@ -494,9 +534,9 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
             .setExpiresAt(Instant.now().plusSeconds(100))
             .setAuthenticationTime(Instant.now())
             .setRedirectURI(new URI(redirectUri))
-            .setScope(scope)
-            .setSessionIdentifier(sid);
-
+            .setScope(Scope.parse(scope))
+            .setSessionIdentifier(sid)
+            .setCodeChallenge(verifier);
         if (deliveryClaims != null) {
             builder.setDlClaims(new ClaimsSet(deliveryClaims));
         }
@@ -822,7 +862,6 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
         Assert.assertNotNull(accessToken);
         Assert.assertTrue(unwrapAccessToken(response).getClaimsSet().getAudience().contains("https://rp.example.org"));
         Assert.assertNotNull(response.getOIDCTokens().getIDToken());
-        System.out.println("JWT: " + response.getOIDCTokens().getIDToken().serialize());
         Assert.assertNotNull(response.getOIDCTokens().getIDToken().getJWTClaimsSet().getClaim("at_hash"));
         validateConsentFromAccessToken(response, false);
         Assert.assertFalse(revocationCache.isRevoked(RevocationCacheContexts.SINGLE_ACCESS_OR_REFRESH_TOKENS, id));
@@ -1031,6 +1070,7 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
 
     protected void assertSuccessResponse(final FlowExecutionResult result) {
         final OIDCTokenResponse response = parseSuccessResponse(result, OIDCTokenResponse.class);
+        Assert.assertNotNull(response);
         Assert.assertNotNull(response.getTokens().getAccessToken());
         Assert.assertNotNull(response.getOIDCTokens().getIDToken());
     }
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java
index 90925a9e..7be83c93 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java
@@ -265,31 +265,12 @@ public class SetRefreshTokenToResponseContextTest extends BaseOIDCResponseAction
                 .setACR(new ACR("0"))
                 .setRootTokenIdentifier(rootTokenId)
                 .build();
-        final String jit = claims.getID();
         respCtx.setAuthorizationGrantClaimsSet(claims);
         enforceRotation = true;
         final Event event = action.execute(requestCtx);
         ActionTestingSupport.assertEvent(event, IdPEventIds.INVALID_PROFILE_CONFIG);
     }
 
-    /**
-     * There is no offline_access scope.
-     * 
-     * @throws ComponentInitializationException 
-     * @throws NoSuchAlgorithmException 
-     * @throws URISyntaxException 
-     * @throws ParseException 
-     * @throws DataSealerException 
-     */
-    @Test
-    public void testNoToken() throws ComponentInitializationException, NoSuchAlgorithmException, URISyntaxException,
-            ParseException, DataSealerException {
-        respCtx.setScope(new Scope());
-        final Event event = action.execute(requestCtx);
-        ActionTestingSupport.assertProceedEvent(event);
-        Assert.assertNull(respCtx.getRefreshToken());
-    }
-
     /**
      * fails as there is no rp ctx.
      * 

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


More information about the commits mailing list