[java-idp-oidc] branch main updated: JOIDC-212 - Empty/missing scope in authorization request produces uncaught exception
Henri Mikkonen
henri.mikkonen at iki.fi
Mon May 27 16:57:49 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=c8d0b548e5293420bae5fc9d17cada671bc465bc
The following commit(s) were added to refs/heads/main by this push:
new c8d0b548 JOIDC-212 - Empty/missing scope in authorization request produces uncaught exception
c8d0b548 is described below
commit c8d0b548e5293420bae5fc9d17cada671bc465bc
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Mon May 27 19:57:32 2024 +0300
JOIDC-212 - Empty/missing scope in authorization request produces uncaught exception
https://shibboleth.atlassian.net/browse/JOIDC-212
Fixed the function and improved tests.
---
.../DefaultRequestedScopeLookupFunction.java | 9 ++--
.../DefaultRequestedScopeLookupFunctionTest.java | 49 +++++++++++++++++-
.../oidc/op/profile/flow/AuthorizeFlowTest.java | 59 ++++++++++++++++++++++
3 files changed, 112 insertions(+), 5 deletions(-)
diff --git a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunction.java b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunction.java
index 57d2d298..d6acbef5 100644
--- a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunction.java
+++ b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunction.java
@@ -47,8 +47,11 @@ public class DefaultRequestedScopeLookupFunction extends AbstractAuthorizationRe
log.error("Unable to parse scope from request object scope value");
return null;
}
- final Scope requestParameterScope = new Scope();
- requestParameterScope.addAll(req.getScope());
- return requestParameterScope;
+ final Scope result = new Scope();
+ final Scope requestParameterScope = req.getScope();
+ if (requestParameterScope != null) {
+ result.addAll(requestParameterScope);
+ }
+ return result;
}
}
\ No newline at end of file
diff --git a/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunctionTest.java b/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunctionTest.java
index aceb2b22..bbc3a49e 100644
--- a/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunctionTest.java
+++ b/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedScopeLookupFunctionTest.java
@@ -19,6 +19,7 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.PlainJWT;
+import com.nimbusds.oauth2.sdk.AuthorizationRequest;
import com.nimbusds.oauth2.sdk.ResponseType;
import com.nimbusds.oauth2.sdk.Scope;
import com.nimbusds.oauth2.sdk.id.ClientID;
@@ -46,7 +47,7 @@ public class DefaultRequestedScopeLookupFunctionTest extends BaseDefaultRequestL
assert result != null;
Assert.assertTrue(result.contains("openid"));
Assert.assertTrue(result.contains("email"));
- Assert.assertEquals(2, result.size());
+ Assert.assertEquals(result.size(), 2);
}
@Test
@@ -62,7 +63,51 @@ public class DefaultRequestedScopeLookupFunctionTest extends BaseDefaultRequestL
assert result != null;
Assert.assertTrue(result.contains("openid"));
Assert.assertTrue(result.contains("email"));
- Assert.assertEquals(2, result.size());
+ Assert.assertEquals(result.size(), 2);
+ }
+
+ @Test
+ public void testSuccessReqObjectNullScope() {
+ JWTClaimsSet ro = new JWTClaimsSet.Builder().claim("scope", null).build();
+ AuthorizationRequest req = new AuthorizationRequest.Builder(new PlainJWT(ro),
+ new ClientID("000123")).state(new State()).build();
+ msgCtx.setMessage(req);
+ oidcCtx.setRequestObject(req.getRequestObject());
+ final Scope result = lookup.apply(prc);
+ assert result != null;
+ Assert.assertEquals(result.size(), 0);
+ }
+
+ @Test
+ public void testSuccessReqObjectEmptyScope() {
+ JWTClaimsSet ro = new JWTClaimsSet.Builder().claim("scope", "").build();
+ AuthorizationRequest req = new AuthorizationRequest.Builder(new PlainJWT(ro),
+ new ClientID("000123")).state(new State()).build();
+ msgCtx.setMessage(req);
+ oidcCtx.setRequestObject(req.getRequestObject());
+ final Scope result = lookup.apply(prc);
+ assert result != null;
+ Assert.assertEquals(result.size(), 0);
+ }
+
+ @Test
+ public void testWithNullScopeInRequest() {
+ AuthorizationRequest req = new AuthorizationRequest.Builder(URI.create("https://example.com/callback"),
+ new ClientID("000123")).state(new State()).build();
+ msgCtx.setMessage(req);
+ final Scope result = lookup.apply(prc);
+ assert result != null;
+ Assert.assertEquals(result.size(), 0);
+ }
+
+ @Test
+ public void testWithEmptyScopeInRequest() {
+ AuthorizationRequest req = new AuthorizationRequest.Builder(URI.create("https://example.com/callback"),
+ new ClientID("000123")).state(new State()).scope(new Scope()).build();
+ msgCtx.setMessage(req);
+ final Scope result = lookup.apply(prc);
+ assert result != null;
+ Assert.assertEquals(result.size(), 0);
}
}
\ 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 18209036..120aa96f 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
@@ -99,6 +99,7 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
public void setup() {
setBasicAuth("jdoe", "changeit");
}
+
@Test
public void testWithAuthorizationCodeFlow() throws IOException, SessionException {
setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
@@ -121,6 +122,45 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithAuthorizationCodeFlowNoScope() throws IOException, SessionException {
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("response_type", "code"),
+ 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 testWithAuthorizationCodeFlowEmptyScope() throws IOException, SessionException {
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("response_type", "code"),
+ new Pair<>("scope", ""),
+ 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 testWithAuthorizationCodeFlow_defaultResponseModeNotAllowed() throws IOException, SessionException {
setRequestParameters(List.of(new Pair<>("client_id", clientIdFragmentResponseMode),
@@ -314,6 +354,25 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithAuthorizationCodeFlowNoScopeMetadataContainsResource() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientId"),
+ new Pair<>("response_type", "code"),
+ new Pair<>("redirect_uri", redirectUri)));
+ 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 testWithAuthorizationCodeFlowNoOpenidMetadataNotContainingResource() throws IOException, SessionException {
request.setMethod("GET");
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list