[java-idp-oidc] branch main updated: JOIDC-214 - Response type parameter handling in authorization endpoint
Henri Mikkonen
henri.mikkonen at iki.fi
Mon Jun 17 12:27:08 UTC 2024
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=ff163fb83ee867b3053d8509ee6ea81bfbe3f146
The following commit(s) were added to refs/heads/main by this push:
new ff163fb8 JOIDC-214 - Response type parameter handling in authorization endpoint
ff163fb8 is described below
commit ff163fb83ee867b3053d8509ee6ea81bfbe3f146
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Mon Jun 17 15:26:42 2024 +0300
JOIDC-214 - Response type parameter handling in authorization endpoint
https://shibboleth.atlassian.net/browse/JOIDC-214
Modified the response_type handling to allow it solely exist in request object
if the request is plain OAuth2 request (i.e. not containing openid scope).
In OIDC case, the value must always be set in request parameters. If also set in
request object, the value must match with the one in request parameters.
The same logic is applied in the PAR endpoint.
---
.../DefaultRequestResponseTypeLookupFunction.java | 9 +-
.../oauth2/profile/impl/ValidateRequestObject.java | 22 +++--
.../oauth2/profile/impl/ValidateResponseType.java | 5 +
.../idp/flows/oidc/authorize/authorize-beans.xml | 15 +--
.../oidc/op/profile/flow/AuthorizeFlowTest.java | 83 +++++++++++++++++
.../op/profile/flow/PushedAuthorizeFlowTest.java | 101 ++++++++++++++++++++-
6 files changed, 216 insertions(+), 19 deletions(-)
diff --git a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestResponseTypeLookupFunction.java b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestResponseTypeLookupFunction.java
index 5b05ab29..c086ab97 100644
--- a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestResponseTypeLookupFunction.java
+++ b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestResponseTypeLookupFunction.java
@@ -48,8 +48,11 @@ public class DefaultRequestResponseTypeLookupFunction
log.error("Unable to parse response type from request object response_type value {}", e.getMessage());
return null;
}
- final ResponseType requestParameterScope = new ResponseType();
- requestParameterScope.addAll(req.getResponseType());
- return requestParameterScope;
+ final ResponseType requestedType = req.getResponseType();
+ final ResponseType result = new ResponseType();
+ if (requestedType != null) {
+ result.addAll(requestedType);
+ }
+ return result;
}
}
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRequestObject.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRequestObject.java
index b1ecb011..dac908de 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRequestObject.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRequestObject.java
@@ -32,6 +32,7 @@ import com.nimbusds.jwt.SignedJWT;
import com.nimbusds.oauth2.sdk.AuthorizationRequest;
import com.nimbusds.oauth2.sdk.ResponseType;
import com.nimbusds.oauth2.sdk.id.ClientID;
+import com.nimbusds.openid.connect.sdk.AuthenticationRequest;
import com.nimbusds.openid.connect.sdk.rp.OIDCClientInformation;
import net.shibboleth.idp.plugin.oidc.op.messaging.context.OIDCAuthenticationResponseContext;
@@ -167,13 +168,20 @@ public class ValidateRequestObject extends AbstractOAuthAuthorizationResponseAct
ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_REQUEST_OBJECT);
return;
}
- if (claimsSet.getClaims().containsKey("response_type")
- && !authorizationRequest.getResponseType().equals(new ResponseType(
- ((String) claimsSet.getClaim("response_type")).split(" ")))) {
- log.error("{} response_type in request object not matching response_type request parameter",
- getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_REQUEST_OBJECT);
- return;
+ if (authorizationRequest instanceof AuthenticationRequest authenticationRequest) {
+ final ResponseType requestedType = authenticationRequest.getResponseType();
+ if (requestedType == null) {
+ log.error("{} mandatory response_type is missing from the request", getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_RESPONSE_TYPE);
+ return;
+ }
+ if (claimsSet.getClaims().containsKey("response_type")
+ && !requestedType.equals(new ResponseType(claimsSet.getStringClaim("response_type").split(" ")))) {
+ log.error("{} response_type in request object not matching response_type request parameter",
+ getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_REQUEST_OBJECT);
+ return;
+ }
}
} catch (final ParseException e) {
log.error("{} Unable to parse request object {}", getLogPrefix(), e.getMessage());
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseType.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseType.java
index b2f5aa10..42a2326a 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseType.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateResponseType.java
@@ -125,6 +125,11 @@ public class ValidateResponseType extends AbstractOAuthAuthorizationResponseActi
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
final ResponseType requestedType = requestedResponseTypeLookupStrategy.apply(profileRequestContext);
+ if (requestedType == null || requestedType.isEmpty()) {
+ log.warn("{} The response type {} is missing from the request", getLogPrefix(), requestedType);
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_RESPONSE_TYPE);
+ return;
+ }
final OIDCMetadataContext metadataContext = getMetadataContext();
if (metadataContext != null) {
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 66bf520b..2c359a4d 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
@@ -991,26 +991,29 @@
<bean id="MapEventToView" class="net.shibboleth.idp.profile.context.navigate.SpringEventToViewLookupFunction"
p:defaultView-ref="shibboleth.DefaultErrorView" p:eventMap="#{getObject('shibboleth.EventViewMap')}" />
- <bean id="IDTokenRequested" parent="shibboleth.Conditions.Expression">
+ <bean id="RequestResponseTypeLookupFunction"
+ class="net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultRequestResponseTypeLookupFunction"/>
+
+ <bean id="IDTokenRequested" parent="shibboleth.Conditions.Expression" p:customObject-ref="RequestResponseTypeLookupFunction">
<constructor-arg>
<value>
- #profileContext.getInboundMessageContext().getMessage().getResponseType().contains(T(com.nimbusds.openid.connect.sdk.OIDCResponseTypeValue).ID_TOKEN)
+ #custom.apply(#profileContext).contains(T(com.nimbusds.openid.connect.sdk.OIDCResponseTypeValue).ID_TOKEN)
</value>
</constructor-arg>
</bean>
- <bean id="AccessTokenRequested" parent="shibboleth.Conditions.Expression">
+ <bean id="AccessTokenRequested" parent="shibboleth.Conditions.Expression" p:customObject-ref="RequestResponseTypeLookupFunction">
<constructor-arg>
<value>
- #profileContext.getInboundMessageContext().getMessage().getResponseType().contains(T(com.nimbusds.oauth2.sdk.ResponseType.Value).TOKEN)
+ #custom.apply(#profileContext).contains(T(com.nimbusds.oauth2.sdk.ResponseType.Value).TOKEN)
</value>
</constructor-arg>
</bean>
- <bean id="AuthorizeCodeRequested" parent="shibboleth.Conditions.Expression">
+ <bean id="AuthorizeCodeRequested" parent="shibboleth.Conditions.Expression" p:customObject-ref="RequestResponseTypeLookupFunction">
<constructor-arg>
<value>
- #profileContext.getInboundMessageContext().getMessage().getResponseType().contains(T(com.nimbusds.oauth2.sdk.ResponseType.Value).CODE)
+ #custom.apply(#profileContext).contains(T(com.nimbusds.oauth2.sdk.ResponseType.Value).CODE)
</value>
</constructor-arg>
</bean>
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 aff3da2d..86aa61d2 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
@@ -1811,6 +1811,89 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
assertRequestObjectError(new PlainJWT(ro));
}
+ @Test
+ public void testWitnOpenIDReqObjectResponseTypeParameter() throws IOException, SessionException {
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("response_type", "code")
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("request", requestObject.serialize())));
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertFlowExecutionResult(result, FLOW_ID);
+ Assert.assertEquals(result.getOutcome().getId(), "ErrorView");
+ }
+
+ @Test
+ public void testWithOpenIDReqObjectNoResponseType() throws IOException, SessionException {
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("redirect_uri", redirectUri)
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("request", requestObject.serialize())));
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertFlowExecutionResult(result, FLOW_ID);
+ Assert.assertEquals(result.getOutcome().getId(), "ErrorView");
+ }
+
+ @Test
+ public void testWithOAuth2ReqObjectResponseTypeParameter() throws IOException, SessionException {
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("response_type", "code")
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("scope", "profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("request", requestObject.serialize())));
+ 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 testWithOAuth2ReqObjectNoResponseType() throws IOException, SessionException {
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("redirect_uri", redirectUri)
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("scope", "profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("request", requestObject.serialize())));
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, "unsupported_response_type");
+ }
+
@Test
public void testWithPlainReqObjectOverwriteRedirectUri() throws IOException, SessionException {
final JWTClaimsSet ro = new JWTClaimsSet.Builder()
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 d5836eba..048daadd 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
@@ -34,6 +34,8 @@ import org.testng.annotations.Test;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jwt.JWT;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.PlainJWT;
import com.nimbusds.oauth2.sdk.OAuth2Error;
import com.nimbusds.oauth2.sdk.PushedAuthorizationSuccessResponse;
import com.nimbusds.oauth2.sdk.Scope;
@@ -268,7 +270,89 @@ public class PushedAuthorizeFlowTest extends AbstractOidcClientAuthenticationFlo
parseSuccessResponse(result, PushedAuthorizationSuccessResponse.class);
verifyAuthorizeEndpoint(clientId, response.getRequestURI().toString());
}
-
+
+ @Test
+ public void testOIDCNoResponseType() throws IOException, NoSuchAlgorithmException, URISyntaxException,
+ DataSealerException, ComponentInitializationException {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ setHttpFormRequest("POST", createRequestParameters(clientId, "openid profile", null, null));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ }
+
+ @Test
+ public void testOIDCNoResponseTypeRO() throws IOException, NoSuchAlgorithmException, URISyntaxException,
+ DataSealerException, ComponentInitializationException {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("redirect_uri", "https://example.org/cb")
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ setHttpFormRequest("POST", createRequestParameters(clientId, "openid profile", null,
+ requestObject.serialize()));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ }
+
+ @Test
+ public void testOIDCResponseTypeInRO() throws IOException, NoSuchAlgorithmException, URISyntaxException,
+ DataSealerException, ComponentInitializationException {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("response_type", "code")
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ setHttpFormRequest("POST", createRequestParameters(clientId, "openid profile", null,
+ requestObject.serialize()));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ }
+
+ @Test
+ public void testOAuth2NoResponseType() throws IOException, NoSuchAlgorithmException, URISyntaxException,
+ DataSealerException, ComponentInitializationException {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ setHttpFormRequest("POST", createRequestParameters(clientId, "profile", null, null));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ }
+
+ @Test
+ public void testOAuth2NoResponseTypeRO() throws IOException, NoSuchAlgorithmException, URISyntaxException,
+ DataSealerException, ComponentInitializationException {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("redirect_uri", "https://example.org/cb")
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ setHttpFormRequest("POST", createRequestParameters(clientId, "profile", null, requestObject.serialize()));
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.INVALID_REQUEST_CODE);
+ }
+
+ @Test
+ public void testOAuth2ResponseTypeInRO() throws IOException, NoSuchAlgorithmException, URISyntaxException,
+ DataSealerException, ComponentInitializationException {
+ storeMetadata(storageService, clientId, clientSecret, scope, "https://example.org/cb");
+ setBasicAuth(clientId, clientSecret);
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("response_type", "code")
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ setHttpFormRequest("POST", createRequestParameters(clientId, "profile", 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());
+ }
+
@Factory
public Object[] createRequestObjectSecurityTests() {
return new Object[] {
@@ -324,12 +408,23 @@ public class PushedAuthorizeFlowTest extends AbstractOidcClientAuthenticationFlo
request.removeHeader("Authorization");
}
+
protected static Map<String,String> createRequestParameters(final String id) {
+ return createRequestParameters(id, "openid profile", "code", null);
+ }
+
+ protected static Map<String,String> createRequestParameters(final String id, final String scope,
+ final String responseType, final String requestObject) {
final Map<String,String> result = new HashMap<>();
result.put("client_id", id);
- result.put("response_type", "code");
- result.put("scope", "openid profile");
+ if (responseType != null) {
+ result.put("response_type", responseType);
+ }
+ result.put("scope", scope);
result.put("redirect_uri", "https://example.org/cb");
+ if (requestObject != null) {
+ result.put("request", requestObject);
+ }
return result;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list