[java-idp-oidc] 14/35: JOIDC-229 - Provide method for strict scope validation
Henri Mikkonen
henri.mikkonen at iki.fi
Thu Oct 24 14:01:39 UTC 2024
This is an automated email from the git hooks/post-receive script.
hjmikkon pushed a commit to branch dev/JOIDC-222
in repository java-idp-oidc.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-oidc.git;a=commit;h=31844651fdcaa54026e5fdd6b893019976dd1d85
commit 31844651fdcaa54026e5fdd6b893019976dd1d85
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Wed Oct 2 15:34:12 2024 +0300
JOIDC-229 - Provide method for strict scope validation
https://shibboleth.atlassian.net/browse/JOIDC-229
- Exploit the strictScopeValidation profile config setting
- OIDC.SSO and OAUTH2.Token: idp.oidc.strictScopeValidation -property, defaults to false
- OAUTH2.PAR: defaults to true
---
.../oidc/op/oauth2/profile/impl/ValidateScope.java | 53 ++++++++++++++--
.../idp/service/relying-party/postconfig.xml | 19 +++++-
.../idp/plugin/oidc/op/conf/oidc.properties | 6 +-
.../oidc/op/profile/flow/AuthorizeFlowTest.java | 74 ++++++++++++++++++++++
.../flow/ClientCredentialsTokenFlowTest.java | 11 ++++
.../plugin/oidc/op/profile/flow/TokenFlowTest.java | 14 ++++
.../shibboleth/idp/module/conf/relying-party.xml | 9 +++
7 files changed, 177 insertions(+), 9 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateScope.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateScope.java
index 0a090c62..78cf415c 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateScope.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateScope.java
@@ -16,6 +16,7 @@ package net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl;
import java.util.Iterator;
import java.util.function.Function;
+import java.util.function.Predicate;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -36,6 +37,7 @@ import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.ClientInfoScop
import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultOIDCMetadataContextLookupFunction;
import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.OIDCAuthenticationResponseContextLookupFunction;
import net.shibboleth.idp.plugin.oidc.op.token.support.TokenClaimsSet;
+import net.shibboleth.oidc.profile.config.logic.StrictScopeValidationPredicate;
import net.shibboleth.oidc.profile.core.OidcEventIds;
import net.shibboleth.profile.context.navigate.RelyingPartyIdLookupFunction;
import net.shibboleth.shared.logic.Constraint;
@@ -80,6 +82,9 @@ public class ValidateScope extends AbstractOAuthAuthorizationResponseAction {
/** Lookup strategy for fetching the requested response type. */
@Nullable private Function<ProfileRequestContext, ResponseType> requestedResponseTypeLookupStrategy;
+ /** Condition whether to apply strict scope validation, i.e. unallowed scope is an error. */
+ @Nonnull private Predicate<ProfileRequestContext> strictScopeValidationCondition;
+
/** Strategy used to locate the {@link OIDCAuthenticationResponseTokenClaimsContext}. */
@Nonnull
private Function<ProfileRequestContext,OIDCAuthenticationResponseTokenClaimsContext>
@@ -98,6 +103,7 @@ public class ValidateScope extends AbstractOAuthAuthorizationResponseAction {
new OIDCAuthenticationResponseContextLookupFunction());
assert tccls != null;
tokenClaimsContextLookupStrategy = tccls;
+ strictScopeValidationCondition = new StrictScopeValidationPredicate();
}
/**
@@ -173,6 +179,17 @@ public class ValidateScope extends AbstractOAuthAuthorizationResponseAction {
requestedResponseTypeLookupStrategy = strategy;
}
+ /**
+ * Set the condition whether to apply strict scope validation, i.e. unallowed scope is an error.
+ *
+ * @param predicate What to set.
+ */
+ public void setStrictScopeValidationCondition(@Nonnull final Predicate<ProfileRequestContext> predicate) {
+ checkSetterPreconditions();
+ strictScopeValidationCondition = Constraint.isNotNull(predicate,
+ "StrictScopeValidationCondition cannot be null");
+ }
+
// Checkstyle: CyclomaticComplexity OFF
// Checkstyle: MethodLength OFF
/** {@inheritDoc} */
@@ -227,6 +244,14 @@ public class ValidateScope extends AbstractOAuthAuthorizationResponseAction {
}
}
}
+
+ final boolean strictValidation = strictScopeValidationCondition.test(profileRequestContext);
+ if (strictValidation && requestedScopes != null && !requestedScopes.isEmpty() &&
+ (allowedScopes == null || allowedScopes.isEmpty())) {
+ log.warn("{} Strict scope validation is active and no allowed scopes for RP {}", getLogPrefix(), clientId);
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_SCOPE);
+ return;
+ }
if (allowedScopes == null || allowedScopes.isEmpty()) {
log.debug("{} No allowed scope for client {}, nothing to do", getLogPrefix(), clientId);
return;
@@ -241,14 +266,28 @@ public class ValidateScope extends AbstractOAuthAuthorizationResponseAction {
for (Iterator<Scope.Value> i = requestedScopes.iterator(); i.hasNext();) {
final Scope.Value scope = i.next();
if (!allowedScopes.contains(scope)) {
- log.warn("{} Removing requested but unregistered scope {} for RP {}", getLogPrefix(), scope.getValue(),
- clientId);
- i.remove();
+ if (strictValidation) {
+ log.warn("{} Requested unregistered scope {} for RP {} with strict validation", getLogPrefix(),
+ scope.getValue(), clientId);
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_SCOPE);
+ return;
+ } else {
+ log.warn("{} Removing requested but unregistered scope {} for RP {}", getLogPrefix(),
+ scope.getValue(), clientId);
+ i.remove();
+ }
} else if (previouslyGrantedScopes != null && !previouslyGrantedScopes.contains(scope)) {
- log.warn("{} Removing requested but previously ungranted scope {} for RP {}", getLogPrefix(),
- scope.getValue(), clientId);
- i.remove();
- reducedRequestedScopes = true;
+ if (strictValidation) {
+ log.warn("{} Requested previously ungranted scope {} for RP {}", getLogPrefix(),
+ scope.getValue(), clientId);
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_SCOPE);
+ return;
+ } else {
+ log.warn("{} Removing requested but previously ungranted scope {} for RP {}", getLogPrefix(),
+ scope.getValue(), clientId);
+ i.remove();
+ reducedRequestedScopes = true;
+ }
}
}
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml
index 43d5bbf6..10841f9e 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml
@@ -100,7 +100,8 @@
p:dpopProofClaimsValidator-ref="DefaultDPoPProofClaimsValidator"
p:unregisteredClientPolicy="#{getObject('shibboleth.oidc.DefaultUnregisteredClientPolicy')}"
p:requestUriType="%{idp.oauth2.par.requestUriType:}"
- p:requestUriLifetime="%{idp.oauth2.par.requestUriLifetime:PT1M}"/>
+ p:requestUriLifetime="%{idp.oauth2.par.requestUriLifetime:PT1M}"
+ p:strictScopeValidation="true"/>
<bean id="OIDFED.Configuration" parent="AbstractOIDCProfile" lazy-init="true"
class="net.shibboleth.idp.plugin.oidc.op.oidfed.config.DefaultOIDFederationEntityConfiguration"
@@ -308,6 +309,14 @@
p:propertyType="#{T(java.util.function.Function)}"
p:defaultValue-ref="DefaultOAuth2DPoPNonceGenerator" />
</property>
+ <property name="strictScopeValidationPredicate">
+ <bean class="net.shibboleth.shared.logic.PredicateSupport" factory-method="fromFunction">
+ <constructor-arg>
+ <bean parent="shibboleth.MDDrivenBoolProperty" p:propertyName="strictScopeValidation" />
+ </constructor-arg>
+ <constructor-arg value="%{idp.oidc.strictScopeValidation:false}" />
+ </bean>
+ </property>
</bean>
<bean id="OIDC.SSO.MDDriven" parent="AbstractMDDrivenOIDCSSOProfile" lazy-init="true"
@@ -746,6 +755,14 @@
<bean parent="shibboleth.MDDrivenDurationProperty" p:propertyName="requestUriLifetime"
p:defaultValue="%{idp.oauth2.par.requestUriLifetime:PT1M}" />
</property>
+ <property name="strictScopeValidationPredicate">
+ <bean class="net.shibboleth.shared.logic.PredicateSupport" factory-method="fromFunction">
+ <constructor-arg>
+ <bean parent="shibboleth.MDDrivenBoolProperty" p:propertyName="strictScopeValidation" />
+ </constructor-arg>
+ <constructor-arg value="true" />
+ </bean>
+ </property>
</bean>
<!-- Default client-auth JWT validation wiring. -->
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 7a4e15b1..7d602743 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
@@ -204,4 +204,8 @@ idp.oidc.subject.salt = this_too_should_be_ch4ng3d
#idp.oauth2.jwtAuth.audienceValidator.endpointTargets = /profile/oauth2/introspection,/profile/oauth2/revocation,/profile/oauth2/pushed-authorization
# Bean to determine whether refresh token is issuance is activated
-#idp.oauth2.refreshToken.activation = DefaultRefreshTokenActivationCondition
\ No newline at end of file
+#idp.oauth2.refreshToken.activation = DefaultRefreshTokenActivationCondition
+
+# Set to true to enable strict scope validation, i.e. unallowed requested scope is considered as an error (defaults to false)
+# Affects OIDC.SSO and OAUTH2.Token profile configurations
+#idp.oidc.strictScopeValidation = 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 f46445dc..3a54eff1 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
@@ -171,6 +171,80 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithAuthorizationCodeFlowUnallowedScopeNotStrict() throws IOException, SessionException {
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("response_type", "code"),
+ new Pair<>("scope", "notAllowed"),
+ new Pair<>("redirect_uri", redirectUri)));
+ request.setMethod("GET");
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ final AuthorizationResponse responseMessage = parseSuccessResponse(result, AuthorizationResponse.class);
+ final AuthorizationSuccessResponse successResponse = responseMessage.toSuccessResponse();
+ Assert.assertEquals(successResponse.getRedirectionURI().toString(), redirectUri);
+ Assert.assertNull(successResponse.getAccessToken());
+ Assert.assertNotNull(successResponse.getAuthorizationCode());
+ Assert.assertNull(successResponse.getIssuer());
+ }
+
+ @Test
+ public void testWithAuthorizationCodeFlowOpenidUnallowedScopeNotStrict() throws IOException, SessionException {
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("response_type", "code"),
+ new Pair<>("scope", "openid notAllowed"),
+ new Pair<>("redirect_uri", redirectUri)));
+ request.setMethod("GET");
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ final AuthorizationResponse responseMessage = parseSuccessResponse(result, AuthorizationResponse.class);
+ final AuthorizationSuccessResponse successResponse = responseMessage.toSuccessResponse();
+ Assert.assertEquals(successResponse.getRedirectionURI().toString(), redirectUri);
+ Assert.assertNull(successResponse.getAccessToken());
+ Assert.assertNotNull(successResponse.getAuthorizationCode());
+ Assert.assertNull(successResponse.getIssuer());
+ }
+
+ @Test
+ public void testWithAuthorizationCodeFlowUnallowedScopeStrict() throws IOException, SessionException {
+ final String clientId = "mockClientIdStrictScope";
+ setRequestParameters(List.of(new Pair<>("client_id", clientId),
+ new Pair<>("response_type", "code"),
+ new Pair<>("scope", "notAllowed"),
+ new Pair<>("redirect_uri", redirectUri)));
+ request.setMethod("GET");
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, "invalid_scope");
+ removeMetadata(storageService, clientId);
+ }
+
+ @Test
+ public void testWithAuthorizationCodeFlowOpenidUnallowedScopeStrict() throws IOException, SessionException {
+ final String clientId = "mockClientIdStrictScope";
+ setRequestParameters(List.of(new Pair<>("client_id", clientId),
+ new Pair<>("response_type", "code"),
+ new Pair<>("scope", "openid notAllowed"),
+ new Pair<>("redirect_uri", redirectUri)));
+ request.setMethod("GET");
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, "invalid_scope");
+ removeMetadata(storageService, clientId);
+ }
+
@Test
public void testWithAuthorizationCodeFlow_parRequiredByProfileNotProvided() throws IOException, SessionException {
setRequestParameters(List.of(new Pair<>("client_id", clientIdRequirePAR),
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 cb32a14e..058729b8 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,17 @@ public class ClientCredentialsTokenFlowTest extends AbstractOidcClientAuthentica
Collections.singletonList(resource), "eduPersonScopedAffiliation");
}
+ @Test
+ public void testNoScopeRegisteredStrictValidation() throws Exception {
+ final String clientId = "mockClientIdStrictScope";
+ setHttpFormRequest("POST", createRequestParameters(clientId, scope, resource));
+ storeMetadata(storageService, clientId, clientSecret, null);
+ setBasicAuth(clientId, clientSecret);
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_SCOPE_CODE);
+ removeMetadata(storageService, clientId);
+ }
+
@Test
public void testNoScopeRequestedNorRegistered() throws Exception {
setHttpFormRequest("POST", createRequestParameters(clientId, null, resource));
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 82a43ee5..fa9af095 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
@@ -206,6 +206,20 @@ public class TokenFlowTest extends AbstractOidcClientAuthenticationFlowTest {
Assert.assertNotNull(response.getTokens().getAccessToken());
}
+ @Test
+ public void testNoScopesRegisteredWithStrictScopeValidation() throws Exception {
+ final String clientId = "mockClientIdStrictScope";
+ setHttpFormRequest("POST",
+ createRequestParameters(redirectUri, "authorization_code",
+ buildAuthorizationCode(clientId, null, "profile"), clientId));
+ storeMetadata(storageService, clientId, clientSecret, null);
+ setBasicAuth(clientId, clientSecret);
+ storeConsent(storageService, "jdoe", clientId, "mail");
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, "invalid_scope");
+ removeMetadata(storageService, clientId);
+ }
+
@Test
public void testNoScopesRegisteredWithoutOpenidScopeRequestedNoResourcesInMetadata() throws Exception {
setHttpFormRequest("POST",
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 acbced56..0d34abd0 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
@@ -117,6 +117,15 @@
</list>
</property>
</bean>
+ <bean parent="RelyingPartyByName" c:relyingPartyIds="mockClientIdStrictScope">
+ <property name="profileConfigurations">
+ <list>
+ <bean parent="OIDC.SSO" p:strictScopeValidation="true" />
+ <bean parent="OAUTH2.Token" p:strictScopeValidation="true" />
+ <bean parent="OAUTH2.PAR" p:strictScopeValidation="true" />
+ </list>
+ </property>
+ </bean>
<bean parent="RelyingPartyByName" c:relyingPartyIds="mockClientIdNotMDDrivenRefreshTokenJwt">
<property name="profileConfigurations">
<list>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list