[java-idp-oidc] 02/02: JOIDC-298 - Response mode query should not be allowed for implicit and hybrid flows
Codeberg
noreply at shibboleth.net
Thu Nov 5 16:58:34 UTC 2026
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch main
in repository java-idp-oidc.
View the commit online:
https://codeberg.org/Shibboleth/java-idp-oidc/commit/4e8ca9a15e9b107d45376fae5435d0762c3716d5
commit 4e8ca9a15e9b107d45376fae5435d0762c3716d5
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Thu Nov 5 15:57:23 2026 +0200
JOIDC-298 - Response mode query should not be allowed for implicit and hybrid flows
https://shibboleth.atlassian.net/browse/JOIDC-298
- Refactored ValidateResponseMode to prevent 'query' mode for implicit and hybrid flows
- Response type is fetched via lookup strategy that is able to fetch it also via request object
- Cover in the flow tests for both authorize and PAR
---
.../oauth2/profile/impl/ValidateResponseMode.java | 48 ++++++--
.../pushed-authorization-beans.xml | 3 +-
.../idp/flows/oidc/authorize/authorize-beans.xml | 3 +-
.../oidc/op/profile/flow/AuthorizeFlowTest.java | 36 ++++++
.../op/profile/flow/PushedAuthorizeFlowTest.java | 127 +++++++++++++++++++++
5 files changed, 208 insertions(+), 9 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseMode.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseMode.java
index 5f235e6e..66dae514 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseMode.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseMode.java
@@ -26,6 +26,7 @@ import org.slf4j.Logger;
import com.nimbusds.oauth2.sdk.AuthorizationRequest;
import com.nimbusds.oauth2.sdk.ResponseMode;
+import com.nimbusds.oauth2.sdk.ResponseType;
import net.shibboleth.oidc.profile.config.navigate.ResponseModesLookupFunction;
import net.shibboleth.oidc.profile.core.OidcEventIds;
@@ -48,6 +49,9 @@ public class ValidateResponseMode extends AbstractOAuthAuthorizationResponseActi
/** Lookup strategy for fetching the valid response modes. */
@Nonnull private Function<ProfileRequestContext, Set<String>> validResponseModesLookupStrategy;
+ /** Lookup strategy for fetching the requested response type. */
+ @NonnullAfterInit private Function<ProfileRequestContext, ResponseType> requestedResponseTypeLookupStrategy;
+
/**
* Constructor.
*/
@@ -77,6 +81,17 @@ public class ValidateResponseMode extends AbstractOAuthAuthorizationResponseActi
validResponseModesLookupStrategy = Constraint.isNotNull(strategy, "Lookup strategy cannot be null");
}
+ /**
+ * Set the lookup strategy for fetching the requested response type.
+ *
+ * @param strategy What to set.
+ */
+ public void setRequestedResponseTypeLookupStrategy(
+ @Nonnull final Function<ProfileRequestContext, ResponseType> strategy) {
+ checkSetterPreconditions();
+ requestedResponseTypeLookupStrategy = Constraint.isNotNull(strategy, "Lookup strategy cannot be null");
+ }
+
/** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
@@ -85,17 +100,14 @@ public class ValidateResponseMode extends AbstractOAuthAuthorizationResponseActi
if (requestedResponseModeLookupStrategy == null) {
throw new ComponentInitializationException("Requested response mode lookup strategy cannot be null");
}
+ if (requestedResponseTypeLookupStrategy == null) {
+ throw new ComponentInitializationException("Requested response type lookup strategy cannot be null");
+ }
}
/** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
- final Set<String> validResponseModes = validResponseModesLookupStrategy.apply(profileRequestContext);
- if (validResponseModes == null || validResponseModes.isEmpty()) {
- log.debug("{} No restrictions for the response mode", getLogPrefix());
- return;
- }
-
final AuthorizationRequest authorizationRequest = getAuthorizationRequest();
if (authorizationRequest == null) {
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
@@ -103,10 +115,31 @@ public class ValidateResponseMode extends AbstractOAuthAuthorizationResponseActi
return;
}
+ //TODO: this could be stored in the context by the ValidateResponseType action
+ final ResponseType responseType = requestedResponseTypeLookupStrategy.apply(profileRequestContext);
+ if (responseType == null || responseType.isEmpty()) {
+ log.error("{} Could not resolve requested response type", getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_RESPONSE_TYPE);
+ return;
+ }
final ResponseMode requestedMode = requestedResponseModeLookupStrategy.apply(profileRequestContext);
+ if (ResponseMode.QUERY.equals(requestedMode)) {
+ if (responseType.impliesImplicitFlow() || responseType.impliesHybridFlow()) {
+ log.warn("{} Response mode query is not allowed for response type {}", getLogPrefix(), responseType);
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_RESPONSE_MODE);
+ return;
+ }
+ }
+
+ final Set<String> validResponseModes = validResponseModesLookupStrategy.apply(profileRequestContext);
+ if (validResponseModes == null || validResponseModes.isEmpty()) {
+ log.debug("{} No restrictions for the response mode", getLogPrefix());
+ return;
+ }
+
final ResponseMode responseMode;
if (requestedMode == null) {
- responseMode = authorizationRequest.impliedResponseMode();
+ responseMode = ResponseMode.resolve(null, responseType);
log.debug("{} No response mode set in the request, using the default: {}", getLogPrefix(), responseMode);
} else {
responseMode = requestedMode;
@@ -115,6 +148,7 @@ public class ValidateResponseMode extends AbstractOAuthAuthorizationResponseActi
if (!validResponseModes.stream().map(string -> new ResponseMode(string)).toList().contains(responseMode)) {
log.warn("{} Response mode {} is not allowed", getLogPrefix(), responseMode);
ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_RESPONSE_MODE);
+ return;
}
}
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/pushed-authorization/pushed-authorization-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/pushed-authorization/pushed-authorization-beans.xml
index 12e5ee7f..b31d8383 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/pushed-authorization/pushed-authorization-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/pushed-authorization/pushed-authorization-beans.xml
@@ -299,7 +299,8 @@
</bean>
<bean id="ValidateResponseMode" class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.ValidateResponseMode"
- scope="prototype">
+ scope="prototype"
+ p:requestedResponseTypeLookupStrategy-ref="RequestResponseTypeLookupFunction">
<property name="requestedResponseModeLookupStrategy">
<bean class="net.shibboleth.idp.plugin.oidc.op.messaging.context.navigate.DefaultRequestResponseModeLookupFunction"
p:useOnlyRequestObjectPredicate-ref="#{'%{idp.oauth2.par.useOnlyRequestObject:DefaultParUseOnlyRequestObjectCondition}'.trim()}"/>
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml
index f47199a0..3d9d3b54 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml
@@ -451,7 +451,8 @@
<bean id="ValidateResponseMode" class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.ValidateResponseMode"
scope="prototype"
- p:requestedResponseModeLookupStrategy-ref="DefaultRequestResponseModeLookupFunction"/>
+ p:requestedResponseModeLookupStrategy-ref="DefaultRequestResponseModeLookupFunction"
+ p:requestedResponseTypeLookupStrategy-ref="RequestResponseTypeLookupFunction" />
<bean id="DefaultRequestCodeChallengeLookupFunction"
class="net.shibboleth.idp.plugin.oidc.op.messaging.context.navigate.DefaultRequestCodeChallengeLookupFunction"
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 2bcd0476..0b0f6ac4 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
@@ -823,6 +823,24 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
assertErrorDescriptionContains(result, "InvalidResponseMode");
}
+ @Test
+ public void testWithImplicitFlow_queryResponseModeNotAllowed() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", clientIdQueryResponseMode),
+ new Pair<>("response_type", "id_token"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("response_mode", "query"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("nonce", "idhas3h23hi13h1o2i32")));
+ storeMetadata(storageService, clientIdQueryResponseMode, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, "invalid_request");
+ assertErrorDescriptionContains(result, "InvalidResponseMode");
+ }
+
@Test
public void testWithImplicitFlow_formPostResponseModeNotAllowed() throws IOException, SessionException {
request.setMethod("GET");
@@ -1359,6 +1377,24 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithHybridTokenFlow_queryResponseModeNotAllowed() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", clientId),
+ new Pair<>("response_type", "code token"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("response_mode", "query"),
+ new Pair<>("nonce", "idhas3h23hi13h1o2i32")));
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, "invalid_request");
+ assertErrorDescriptionContains(result, "InvalidResponseMode");
+ }
+
@Test
public void testWithHybridTokenFlowRequestObjectEnforcedNoRO() throws IOException, SessionException {
request.setMethod("GET");
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/PushedAuthorizeFlowTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/PushedAuthorizeFlowTest.java
index 84dc0582..bb4b50bc 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/PushedAuthorizeFlowTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/PushedAuthorizeFlowTest.java
@@ -199,6 +199,133 @@ public class PushedAuthorizeFlowTest extends AbstractOidcClientAuthenticationFlo
}
}
+ @Test
+ public void testQueryResponseModeWithCodeSuccess() throws IOException, NoSuchAlgorithmException,
+ URISyntaxException, DataSealerException, ComponentInitializationException {
+ for (final String clientId : clientIds) {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final Map<String, String> requestParams = createRequestParameters(clientId);
+ requestParams.put("response_type", "code");
+ requestParams.put("response_mode", "query");
+ setHttpFormRequest("POST", requestParams);
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertSuccessResponse(result, clientId);
+ final PushedAuthorizationSuccessResponse response =
+ parseSuccessResponse(result, PushedAuthorizationSuccessResponse.class);
+ verifyAuthorizeEndpoint(clientId, response.getRequestURI().toString());
+ }
+ }
+
+ @Test
+ public void testQueryResponseModeWithCodeSuccessRO() throws IOException, NoSuchAlgorithmException,
+ URISyntaxException, DataSealerException, ComponentInitializationException, JOSEException {
+ for (final String clientId : clientIds) {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final String nonce = idGenerator.generateIdentifier();
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("iss", clientId)
+ .claim("client_id", clientId)
+ .claim("aud", issuer)
+ .claim("response_type", "code")
+ .claim("response_mode", "query")
+ .claim("redirect_uri", "https://example.org/cb")
+ .claim("scope", "openid profile")
+ .claim("nonce", nonce)
+ .build();
+ final SignedJWT requestObject = createSecretJWT(ro, clientSecret);
+ setHttpFormRequest("POST", createRequestParameters(clientId, null, null,
+ requestObject.serialize()));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertSuccessResponse(result, clientId);
+ final PushedAuthorizationSuccessResponse response =
+ parseSuccessResponse(result, PushedAuthorizationSuccessResponse.class);
+ verifyAuthorizeEndpoint(clientId, response.getRequestURI().toString(), null, nonce);
+ }
+ }
+
+ @Test
+ public void testQueryResponseModePreventedForHybrid() throws IOException, NoSuchAlgorithmException,
+ URISyntaxException, DataSealerException, ComponentInitializationException {
+ for (final String clientId : clientIds) {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final Map<String, String> requestParams = createRequestParameters(clientId);
+ requestParams.put("response_type", "code token");
+ requestParams.put("response_mode", "query");
+ setHttpFormRequest("POST", requestParams);
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ assertErrorDescriptionContains(result, "InvalidResponseMode");
+ }
+ }
+
+ @Test
+ public void testQueryResponseModePreventedForImplicit() throws IOException, NoSuchAlgorithmException,
+ URISyntaxException, DataSealerException, ComponentInitializationException {
+ for (final String clientId : clientIds) {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final Map<String, String> requestParams = createRequestParameters(clientId);
+ requestParams.put("response_type", "id_token");
+ requestParams.put("response_mode", "query");
+ requestParams.put("nonce", idGenerator.generateIdentifier());
+ setHttpFormRequest("POST", requestParams);
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ assertErrorDescriptionContains(result, "InvalidResponseMode");
+ }
+ }
+
+ @Test
+ public void testQueryResponseModePreventedForHybridRO() throws IOException, NoSuchAlgorithmException,
+ URISyntaxException, DataSealerException, ComponentInitializationException, JOSEException {
+ for (final String clientId : clientIds) {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("iss", clientId)
+ .claim("client_id", clientId)
+ .claim("aud", issuer)
+ .claim("response_type", "code token")
+ .claim("response_mode", "query")
+ .claim("redirect_uri", "https://example.org/cb")
+ .claim("scope", "openid profile")
+ .claim("nonce", idGenerator.generateIdentifier())
+ .build();
+ final SignedJWT requestObject = createSecretJWT(ro, clientSecret);
+ setHttpFormRequest("POST", createRequestParameters(clientId, null, null,
+ requestObject.serialize()));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ }
+ }
+
+ @Test
+ public void testQueryResponseModePreventedForImplicitRO() throws IOException, NoSuchAlgorithmException,
+ URISyntaxException, DataSealerException, ComponentInitializationException, JOSEException {
+ for (final String clientId : clientIds) {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("iss", clientId)
+ .claim("client_id", clientId)
+ .claim("aud", issuer)
+ .claim("response_type", "id_token")
+ .claim("response_mode", "query")
+ .claim("redirect_uri", "https://example.org/cb")
+ .claim("scope", "openid profile")
+ .claim("nonce", idGenerator.generateIdentifier())
+ .build();
+ final SignedJWT requestObject = createSecretJWT(ro, clientSecret);
+ setHttpFormRequest("POST", createRequestParameters(clientId, null, null,
+ requestObject.serialize()));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ }
+ }
+
@Test
public void testSuccessWithOpenIdParams() throws IOException, NoSuchAlgorithmException, URISyntaxException,
DataSealerException, ComponentInitializationException {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list