[java-idp-oidc] branch main updated: JOIDC-78 - Wrong JSONObject type when decoding claims from Signed JAR Authentication request
Henri Mikkonen
henri.mikkonen at iki.fi
Fri Mar 11 07:16:37 UTC 2022
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=83b58dad58a890f6dbec652780a7531f3cb58d03
The following commit(s) were added to refs/heads/main by this push:
new 83b58dad JOIDC-78 - Wrong JSONObject type when decoding claims from Signed JAR Authentication request
83b58dad is described below
commit 83b58dad58a890f6dbec652780a7531f3cb58d03
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Mar 11 09:13:47 2022 +0200
JOIDC-78 - Wrong JSONObject type when decoding claims from Signed JAR Authentication request
https://shibboleth.atlassian.net/browse/JOIDC-78
Changed DefaultRequestedClaimsLookupFunction to check if the incoming request object
contains 'claims' as Map instead of JSONObject: Nimbus was returning a shaded JSONObject,
which didn't match to the unshaded version. Both (shaded or not) extends a String-keyed
Map.
Also improved unit testing.
---
.../DefaultRequestedClaimsLookupFunction.java | 10 +-
.../DefaultRequestedClaimsLookupFunctionTest.java | 8 +-
.../oidc/op/profile/flow/AuthorizeFlowTest.java | 158 ++++++++++++++++++++-
3 files changed, 167 insertions(+), 9 deletions(-)
diff --git a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunction.java b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunction.java
index c0330a32..6b9f5bb8 100644
--- a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunction.java
+++ b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunction.java
@@ -18,6 +18,7 @@
package net.shibboleth.idp.plugin.oidc.op.profile.context.navigate;
import java.text.ParseException;
+import java.util.Map;
import javax.annotation.Nonnull;
import org.slf4j.Logger;
@@ -40,15 +41,18 @@ public class DefaultRequestedClaimsLookupFunction
private Logger log = LoggerFactory.getLogger(DefaultRequestedClaimsLookupFunction.class);
/** {@inheritDoc} */
+ @SuppressWarnings("unchecked")
@Override
protected OIDCClaimsRequest doLookup(@Nonnull final AuthenticationRequest req) {
try {
if (getRequestObject() != null && getRequestObject().getJWTClaimsSet().getClaim("claims") != null) {
final Object claims = getRequestObject().getJWTClaimsSet().getClaim("claims");
- if (claims instanceof JSONObject) {
- return OIDCClaimsRequest.parse((JSONObject) claims);
+ if (claims instanceof Map) {
+ log.debug("claims claim is a map, converting it into a JSONObject");
+ // the casting is safe as Nimbus shouldn't allow other than String-keyed maps to exist here
+ return OIDCClaimsRequest.parse(new JSONObject((Map<String, ?>) claims));
} else {
- log.error("claims claim is not of expected type");
+ log.error("claims claim is not of expected type (java.util.Map), it's: {}", claims.getClass());
return null;
}
}
diff --git a/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunctionTest.java b/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunctionTest.java
index cb4339d7..612875ba 100644
--- a/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunctionTest.java
+++ b/idp-oidc-extension-api/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/context/navigate/DefaultRequestedClaimsLookupFunctionTest.java
@@ -18,6 +18,7 @@
package net.shibboleth.idp.plugin.oidc.op.profile.context.navigate;
import java.net.URI;
+import java.text.ParseException;
import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
@@ -35,8 +36,6 @@ import com.nimbusds.openid.connect.sdk.claims.ClaimsSetRequest;
import com.nimbusds.openid.connect.sdk.claims.IDTokenClaimsSet;
import com.nimbusds.openid.connect.sdk.claims.UserInfo;
-import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultRequestedClaimsLookupFunction;
-
public class DefaultRequestedClaimsLookupFunctionTest extends BaseDefaultRequestLookupFunctionTest {
private DefaultRequestedClaimsLookupFunction lookup;
@@ -65,7 +64,7 @@ public class DefaultRequestedClaimsLookupFunctionTest extends BaseDefaultRequest
}
@Test
- public void testSuccessReqObject() {
+ public void testSuccessReqObject() throws ParseException {
final ClaimsSetRequest idTokenClaims = new ClaimsSetRequest()
.add(new ClaimsSetRequest.Entry(IDTokenClaimsSet.SUB_CLAIM_NAME)
.withClaimRequirement(ClaimRequirement.ESSENTIAL));
@@ -83,7 +82,8 @@ public class DefaultRequestedClaimsLookupFunctionTest extends BaseDefaultRequest
final AuthenticationRequest req = new AuthenticationRequest.Builder(
new ResponseType("code"), new Scope("openid"), new ClientID("000123"),
URI.create("https://example.com/callback")).claims(crParameter)
- .requestObject(new PlainJWT(ro)).state(new State()).build();
+ .requestObject(PlainJWT.parse(new PlainJWT(ro).serialize())).state(new State()).build();
+ // request object JWT is serialized and parsed in order to simulate incoming authentication request better
msgCtx.setMessage(req);
oidcCtx.setRequestObject(req.getRequestObject());
Assert.assertEquals(crRequestObject.toJSONObject(), lookup.apply(prc).toJSONObject());
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 36b76eeb..62bb2070 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
@@ -40,9 +40,13 @@ import com.nimbusds.oauth2.sdk.ParseException;
import com.nimbusds.oauth2.sdk.Scope;
import com.nimbusds.openid.connect.sdk.AuthenticationResponse;
import com.nimbusds.openid.connect.sdk.AuthenticationSuccessResponse;
+import com.nimbusds.openid.connect.sdk.claims.ClaimRequirement;
+import com.nimbusds.openid.connect.sdk.claims.ClaimsSetRequest;
+import net.shibboleth.idp.plugin.oidc.op.token.support.AuthorizeCodeClaimsSet;
import net.shibboleth.idp.session.SessionException;
import net.shibboleth.oidc.profile.core.OidcError;
+import net.shibboleth.utilities.java.support.security.DataSealerException;
/**
* Tests for the authorize-flow.
@@ -383,7 +387,7 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
}
@Test
- public void testWithAuthorizationCodeFlowWithIDTokenClaims() throws IOException, ParseException, SessionException {
+ public void testWithAuthorizationCodeFlowWithIDTokenClaims() throws IOException, ParseException, SessionException, java.text.ParseException, DataSealerException {
request.setMethod("GET");
request.setQueryString("client_id=mockClientId&response_type=code&scope=openid%20profile"
+ "&claims=%7B%22id_token%22%3A%7B%22email%22%3A%7B%22essential%22%3Atrue%7D%7D%7D"
@@ -399,10 +403,20 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIDToken());
Assert.assertNull(successResponse.getAccessToken());
Assert.assertNotNull(successResponse.getAuthorizationCode());
+
+ final AuthorizeCodeClaimsSet code =
+ AuthorizeCodeClaimsSet.parse(successResponse.getAuthorizationCode().getValue(), getDataSealer());
+ Assert.assertNotNull(code.getClaimsRequest());
+ Assert.assertNotNull(code.getClaimsRequest().getIDTokenClaimsRequest());
+ Assert.assertNull(code.getClaimsRequest().getUserInfoClaimsRequest());
+ Assert.assertTrue(code.getClaimsRequest().getIDTokenClaimsRequest().getClaimNames(false).contains("email"));
+ final ClaimsSetRequest.Entry email = code.getClaimsRequest().getIDTokenClaimsRequest().get("email", null);
+ Assert.assertEquals(email.getClaimName(), "email");
+ Assert.assertEquals(email.getClaimRequirement(), ClaimRequirement.ESSENTIAL);
}
@Test
- public void testWithAuthorizationCodeFlowWithUIClaims() throws IOException, ParseException, SessionException {
+ public void testWithAuthorizationCodeFlowWithUIClaims() throws IOException, ParseException, SessionException, java.text.ParseException, DataSealerException {
request.setMethod("GET");
request.setQueryString("client_id=mockClientId&response_type=code&scope=openid%20profile"
+ "&claims=%7B%22userinfo%22%3A%7B%22email%22%3A%7B%22essential%22%3Atrue%7D%7D%7D"
@@ -418,6 +432,16 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIDToken());
Assert.assertNull(successResponse.getAccessToken());
Assert.assertNotNull(successResponse.getAuthorizationCode());
+
+ final AuthorizeCodeClaimsSet code =
+ AuthorizeCodeClaimsSet.parse(successResponse.getAuthorizationCode().getValue(), getDataSealer());
+ Assert.assertNotNull(code.getClaimsRequest());
+ Assert.assertNull(code.getClaimsRequest().getIDTokenClaimsRequest());
+ Assert.assertNotNull(code.getClaimsRequest().getUserInfoClaimsRequest());
+ Assert.assertTrue(code.getClaimsRequest().getUserInfoClaimsRequest().getClaimNames(false).contains("email"));
+ final ClaimsSetRequest.Entry email = code.getClaimsRequest().getUserInfoClaimsRequest().get("email", null);
+ Assert.assertEquals(email.getClaimName(), "email");
+ Assert.assertEquals(email.getClaimRequirement(), ClaimRequirement.ESSENTIAL);
}
@Test
@@ -488,6 +512,71 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNotNull(successResponse.getAuthorizationCode());
}
+ @Test
+ public void testWithPlainReqObjectClaimsRequest() throws IOException, ParseException, SessionException,
+ java.text.ParseException, DataSealerException {
+ final String payload = "{\n"
+ + " \"iss\": \"" + clientId + "\",\n"
+ + " \"response_type\": \"code\",\n"
+ + " \"code_challenge_method\": \"S256\",\n"
+ + " \"nonce\": \"k5r-Uwjw0KKr18XiKD2VbiLtD2adwt85_HiSvzBi8FI\",\n"
+ + " \"client_id\": \"" + clientId + "\",\n"
+ + " \"aud\": \"https://op.example.org\",\n"
+ + " \"scope\": \"openid profile offline_access\",\n"
+ + " \"claims\": {\n"
+ + " \"id_token\": {\n"
+ + " \"given_name\": {\n"
+ + " \"essential\": true\n"
+ + " }\n"
+ + " },\n"
+ + " \"userinfo\": {\n"
+ + " \"family_name\": {\n"
+ + " \"essential\": true\n"
+ + " }\n"
+ + " }\n"
+ + " },\n"
+ + " \"redirect_uri\": \"" + redirectUri + "\",\n"
+ + " \"state\": \"81c33d57-59c7-4b41-9a15-80e2ed1482e21646857349537\",\n"
+ + " \"code_challenge\": \"MiAR-UxCj6oVyPatcUnrb3MGEZbwLKBmIRSoOKLLTl0\"\n"
+ + "}";
+
+ final JWTClaimsSet ro = JWTClaimsSet.parse(payload);
+ final PlainJWT requestObject = new PlainJWT(ro);
+
+ request.setMethod("GET");
+ request.setQueryString("client_id=mockClientId&response_type=code&scope=openid%20profile&redirect_uri="
+ + "https://invalid.org/cb&request=" + requestObject.serialize());
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ final AuthenticationResponse responseMessage = parseSuccessResponse(result, AuthenticationResponse.class);
+ final AuthenticationSuccessResponse successResponse = responseMessage.toSuccessResponse();
+ Assert.assertEquals(successResponse.getRedirectionURI().toString(), redirectUri);
+ Assert.assertNull(successResponse.getIDToken());
+ Assert.assertNull(successResponse.getAccessToken());
+ Assert.assertNotNull(successResponse.getAuthorizationCode());
+
+ final AuthorizeCodeClaimsSet code =
+ AuthorizeCodeClaimsSet.parse(successResponse.getAuthorizationCode().getValue(), getDataSealer());
+ Assert.assertNotNull(code.getClaimsRequest());
+ Assert.assertNotNull(code.getClaimsRequest().getIDTokenClaimsRequest());
+ Assert.assertNotNull(code.getClaimsRequest().getUserInfoClaimsRequest());
+ Assert.assertTrue(code.getClaimsRequest().getUserInfoClaimsRequest().getClaimNames(false)
+ .contains("family_name"));
+ Assert.assertTrue(code.getClaimsRequest().getIDTokenClaimsRequest().getClaimNames(false)
+ .contains("given_name"));
+ final ClaimsSetRequest.Entry familyName = code.getClaimsRequest().getUserInfoClaimsRequest().get("family_name",
+ null);
+ Assert.assertEquals(familyName.getClaimName(), "family_name");
+ Assert.assertEquals(familyName.getClaimRequirement(), ClaimRequirement.ESSENTIAL);
+ final ClaimsSetRequest.Entry givenName = code.getClaimsRequest().getIDTokenClaimsRequest().get("given_name",
+ null);
+ Assert.assertEquals(givenName.getClaimName(), "given_name");
+ Assert.assertEquals(givenName.getClaimRequirement(), ClaimRequirement.ESSENTIAL);
+ }
+
@Test
public void testWithSignedReqObjectNoIssuer() throws IOException, ParseException, SessionException,
JOSEException {
@@ -551,6 +640,71 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNotNull(successResponse.getAuthorizationCode());
}
+ @Test
+ public void testWithSignedReqObjectClaimsRequest() throws IOException, ParseException,
+ SessionException, JOSEException, java.text.ParseException, DataSealerException {
+ final String payload = "{\n"
+ + " \"iss\": \"" + clientId + "\",\n"
+ + " \"response_type\": \"code\",\n"
+ + " \"code_challenge_method\": \"S256\",\n"
+ + " \"nonce\": \"k5r-Uwjw0KKr18XiKD2VbiLtD2adwt85_HiSvzBi8FI\",\n"
+ + " \"client_id\": \"" + clientId + "\",\n"
+ + " \"aud\": \"https://op.example.org\",\n"
+ + " \"scope\": \"openid profile offline_access\",\n"
+ + " \"claims\": {\n"
+ + " \"id_token\": {\n"
+ + " \"given_name\": {\n"
+ + " \"essential\": true\n"
+ + " }\n"
+ + " },\n"
+ + " \"userinfo\": {\n"
+ + " \"family_name\": {\n"
+ + " \"essential\": true\n"
+ + " }\n"
+ + " }\n"
+ + " },\n"
+ + " \"redirect_uri\": \"" + redirectUri + "\",\n"
+ + " \"state\": \"81c33d57-59c7-4b41-9a15-80e2ed1482e21646857349537\",\n"
+ + " \"code_challenge\": \"MiAR-UxCj6oVyPatcUnrb3MGEZbwLKBmIRSoOKLLTl0\"\n"
+ + "}";
+
+ final JWTClaimsSet ro = JWTClaimsSet.parse(payload);
+ final SignedJWT requestObject = createSecretJWT(ro, clientSecret);
+ request.setMethod("GET");
+ request.setQueryString("client_id=mockClientId&response_type=code&scope=openid%20profile&redirect_uri="
+ + redirectUri + "&request=" + requestObject.serialize());
+ storeMetadata(storageService, clientId, clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ final AuthenticationResponse responseMessage = parseSuccessResponse(result, AuthenticationResponse.class);
+ final AuthenticationSuccessResponse successResponse = responseMessage.toSuccessResponse();
+ Assert.assertEquals(successResponse.getRedirectionURI().toString(), redirectUri);
+ Assert.assertNull(successResponse.getIDToken());
+ Assert.assertNull(successResponse.getAccessToken());
+ Assert.assertNotNull(successResponse.getAuthorizationCode());
+
+ final AuthorizeCodeClaimsSet code =
+ AuthorizeCodeClaimsSet.parse(successResponse.getAuthorizationCode().getValue(), getDataSealer());
+ Assert.assertNotNull(code.getClaimsRequest());
+ Assert.assertNotNull(code.getClaimsRequest().getIDTokenClaimsRequest());
+ Assert.assertNotNull(code.getClaimsRequest().getUserInfoClaimsRequest());
+ Assert.assertTrue(code.getClaimsRequest().getUserInfoClaimsRequest().getClaimNames(false)
+ .contains("family_name"));
+ Assert.assertTrue(code.getClaimsRequest().getIDTokenClaimsRequest().getClaimNames(false)
+ .contains("given_name"));
+ final ClaimsSetRequest.Entry familyName = code.getClaimsRequest().getUserInfoClaimsRequest().get("family_name",
+ null);
+ Assert.assertEquals(familyName.getClaimName(), "family_name");
+ Assert.assertEquals(familyName.getClaimRequirement(), ClaimRequirement.ESSENTIAL);
+ final ClaimsSetRequest.Entry givenName = code.getClaimsRequest().getIDTokenClaimsRequest().get("given_name",
+ null);
+ Assert.assertEquals(givenName.getClaimName(), "given_name");
+ Assert.assertEquals(givenName.getClaimRequirement(), ClaimRequirement.ESSENTIAL);
+
+ }
+
protected void assertRequestObjectError(final JWT requestObject) throws IOException {
request.setMethod("GET");
request.setQueryString("client_id=mockClientId&response_type=code&scope=openid%20profile&redirect_uri="
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list