From d91820e461c0a50a2a62fc28d442075d3da695ef Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 12 Jan 2024 11:54:55 +0100 Subject: [PATCH 1/7] Post jwt token library refactorings - remove and simplify token creation. Only from map instead of a string - reduce complexity in token creation --- .../identity/uaa/oauth/token/Claims.java | 8 ++ .../identity/uaa/util/UaaStringUtils.java | 7 +- .../identity/uaa/util/UaaStringUtilsTest.java | 8 +- .../identity/uaa/oauth/UaaTokenServices.java | 19 ++- .../oauth/jwt/JwtClientAuthentication.java | 3 +- .../identity/uaa/oauth/jwt/JwtHelper.java | 31 +---- .../identity/uaa/oauth/openid/IdToken.java | 6 + .../oauth/refresh/RefreshTokenCreator.java | 109 ++++++++---------- .../jwt/ChainedSignatureVerifierTests.java | 2 +- .../identity/uaa/oauth/jwt/JwtHelperTest.java | 13 +-- .../identity/uaa/util/UaaTokenUtilsTest.java | 9 -- 11 files changed, 91 insertions(+), 124 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java index a1d7f5f84f3..8bc6a91e2d6 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java @@ -14,10 +14,13 @@ package org.cloudfoundry.identity.uaa.oauth.token; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; +import org.cloudfoundry.identity.uaa.util.JsonUtils; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -363,4 +366,9 @@ public String getClientAuth() { public void setClientAuth(final String clientAuth) { this.clientAuth = clientAuth; } + + @JsonIgnore + public HashMap getClaimObject() { + return JsonUtils.convertValue(this, HashMap.class); + } } diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java b/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java index ed3d663a704..5d0d3b48e2a 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java @@ -21,6 +21,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -321,11 +322,11 @@ public static String getSafeParameterValue(String[] value) { return StringUtils.hasText(value[0]) ? value[0] : EMPTY_STRING; } - public static Set getValuesOrDefaultValue(Set values, String defaultValue) { + public static List getValuesOrDefaultValue(Set values, String defaultValue) { if (ObjectUtils.isEmpty(values)) { - return Set.of(defaultValue); + return List.of(defaultValue); } else { - return values; + return new ArrayList<>(values); } } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java index 9b0b023cd23..902d249a617 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java @@ -19,6 +19,7 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -434,9 +435,10 @@ void getSafeParameterValue() { @Test void getArrayDefaultValue() { - assertEquals(Set.of("1", "2"), UaaStringUtils.getValuesOrDefaultValue(Set.of("1", "2"), "1")); - assertEquals(Set.of("1"), UaaStringUtils.getValuesOrDefaultValue(Set.of(), "1")); - assertEquals(Set.of("1"), UaaStringUtils.getValuesOrDefaultValue(null, "1")); + assertEquals(List.of("1", "2").stream().sorted().collect(Collectors.toList()), + UaaStringUtils.getValuesOrDefaultValue(Set.of("1", "2"), "1").stream().sorted().collect(Collectors.toList())); + assertEquals(List.of("1"), UaaStringUtils.getValuesOrDefaultValue(Set.of(), "1")); + assertEquals(List.of("1"), UaaStringUtils.getValuesOrDefaultValue(null, "1")); } private static void replaceZoneVariables(IdentityZone zone) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java index fc48ea819a5..0ed19812581 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java @@ -20,6 +20,7 @@ import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication; import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; import org.cloudfoundry.identity.uaa.oauth.jwt.JwtHelper; +import org.cloudfoundry.identity.uaa.oauth.openid.IdToken; import org.cloudfoundry.identity.uaa.oauth.openid.IdTokenCreationException; import org.cloudfoundry.identity.uaa.oauth.openid.IdTokenCreator; import org.cloudfoundry.identity.uaa.oauth.openid.IdTokenGranter; @@ -451,8 +452,7 @@ private CompositeToken createCompositeToken(String tokenId, compositeToken.setAdditionalInformation(info); - String content; - Map jwtAccessToken = createJWTAccessToken( + Map jwtAccessToken = createJWTAccessToken( compositeToken, user, userAuthenticationTime, @@ -464,23 +464,18 @@ private CompositeToken createCompositeToken(String tokenId, revocableHashSignature, isRevocable, additionalRootClaims); - try { - content = JsonUtils.writeValueAsString(jwtAccessToken); - } catch (JsonUtils.JsonUtilException e) { - throw new IllegalStateException("Cannot convert access token to JSON", e); - } - String token = JwtHelper.encode(content, getActiveKeyInfo()).getEncoded(); + String token = JwtHelper.encode(jwtAccessToken, getActiveKeyInfo()).getEncoded(); compositeToken.setValue(token); BaseClientDetails clientDetails = (BaseClientDetails) clientDetailsService.loadClientByClientId(clientId); if (idTokenGranter.shouldSendIdToken(user, clientDetails, requestedScopes, grantType)) { - String idTokenContent; + IdToken idTokenContent; try { - idTokenContent = JsonUtils.writeValueAsString(idTokenCreator.create(clientDetails, user, userAuthenticationData)); + idTokenContent = idTokenCreator.create(clientDetails, user, userAuthenticationData); } catch (RuntimeException | IdTokenCreationException ignored) { throw new IllegalStateException("Cannot convert id token to JSON"); } - String encodedIdTokenContent = JwtHelper.encode(idTokenContent, keyInfoService.getActiveKey()).getEncoded(); + String encodedIdTokenContent = JwtHelper.encode(idTokenContent.getClaimObject(), keyInfoService.getActiveKey()).getEncoded(); compositeToken.setIdTokenValue(encodedIdTokenContent); } @@ -501,7 +496,7 @@ private KeyInfo getActiveKeyInfo() { .orElseThrow(() -> new InternalAuthenticationServiceException("Unable to sign token, misconfigured JWT signing keys")); } - private Map createJWTAccessToken(OAuth2AccessToken token, + private Map createJWTAccessToken(OAuth2AccessToken token, UaaUser user, Date userAuthenticationTime, Collection clientScopes, diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java index 606e0144dca..d47b42301eb 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java @@ -26,7 +26,6 @@ import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.oauth.OidcMetadataFetcher; import org.cloudfoundry.identity.uaa.provider.oauth.OidcMetadataFetchingException; -import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.UaaStringUtils; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.util.MultiValueMap; @@ -83,7 +82,7 @@ public String getClientAssertion(OIDCIdentityProviderDefinition config) { KeyInfo signingKeyInfo = Optional.ofNullable(keyInfoService.getKey(kid)).orElseThrow(() -> new BadCredentialsException("Missing requested signing key")); return signingKeyInfo.verifierCertificate().isPresent() ? JwtHelper.encodePlusX5t(getMapFromClaims(claims), signingKeyInfo, signingKeyInfo.verifierCertificate().orElseThrow()).getEncoded() : - JwtHelper.encode(JsonUtils.writeValueAsString(claims), signingKeyInfo).getEncoded(); + JwtHelper.encode(claims.getClaimObject(), signingKeyInfo).getEncoded(); } public MultiValueMap getClientAuthenticationParameters(MultiValueMap params, OIDCIdentityProviderDefinition config) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java index 5bb97ef38d9..95cd50f25f9 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java @@ -66,14 +66,10 @@ public static Jwt encodePlusX5t(Map payLoad, KeyInfo keyInfo, X5 return createJwt(header, payLoad, keyInfo); } - public static Jwt encode(CharSequence content, KeyInfo keyInfo) { + public static Jwt encode(Map payLoad, KeyInfo keyInfo) { JwtHeader header; header = JwtHeaderHelper.create(keyInfo.algorithm(), keyInfo.keyId(), keyInfo.keyURL()); - return createJwt(content, keyInfo, header); - } - - private static JwtImpl createJwt(CharSequence content, KeyInfo keyInfo, JwtHeader header) { - return new JwtImpl(header, content, keyInfo.getSigner()); + return new JwtImpl(header, payLoad, keyInfo.getSigner()); } private static JwtImpl createJwt(JwtHeader header, Map payLoad, KeyInfo keyInfo) { @@ -160,31 +156,10 @@ class JwtImpl implements Jwt { /** * @param header the header, containing the JWS/JWE algorithm information. - * @param content the base64-decoded "claims" segment (may be encrypted, depending on + * @param payLoad the "claims" segment (may be encrypted, depending on * header information). * @param signature the base64-decoded "signature" segment. */ - JwtImpl(JwtHeader header, CharSequence content, JWSSigner signature) { - this.header = header; - this.content = content; - this.signature = signature; - try { - this.claimsSet = JWTClaimsSet.parse(String.valueOf(content)); - JWSHeader jwsHeader = JWSHeader.parse(JsonUtils.convertValue(header.parameters, HashMap.class)); - if (signature != null) { - SignedJWT signedJWT = new SignedJWT(jwsHeader, claimsSet); - signedJWT.sign(signature); - signedJwtObject = signedJWT; - parsedJwtObject = null; - } else { - signedJwtObject = null; - parsedJwtObject = null; - } - } catch (ParseException | JOSEException e) { - throw new InvalidTokenException(INVALID_TOKEN, e); - } - } - JwtImpl(JwtHeader header, Map payLoad, JWSSigner signature) { this.header = header; this.signature = signature; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java index 8ff3aec7916..5915de8f464 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableList; import org.cloudfoundry.identity.uaa.oauth.AuthTimeDateConverter; +import org.cloudfoundry.identity.uaa.util.JsonUtils; import java.util.Date; import java.util.HashMap; @@ -156,4 +157,9 @@ public Long getAuthTimeInSeconds() { public String userId() { return sub; } + + @JsonIgnore + public HashMap getClaimObject() { + return JsonUtils.convertValue(this, HashMap.class); + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java index 1812476a595..3c7c9994060 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java @@ -3,10 +3,8 @@ import com.google.common.collect.Maps; import org.cloudfoundry.identity.uaa.oauth.*; import org.cloudfoundry.identity.uaa.oauth.jwt.JwtHelper; -import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; import org.cloudfoundry.identity.uaa.oauth.token.Claims; import org.cloudfoundry.identity.uaa.user.UaaUser; -import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.TimeService; import org.cloudfoundry.identity.uaa.util.JwtTokenSignedByThisUAA; import org.cloudfoundry.identity.uaa.util.UaaStringUtils; @@ -77,60 +75,54 @@ private String buildJwtToken(UaaUser user, Map additionalAuthorizationAttributes, Date expirationDate, String tokenId) { - String content; - try { - Map claims = new LinkedHashMap<>(); - - claims.put(JTI, tokenId); - claims.put(SUB, user.getId()); - claims.put(IAT, timeService.getCurrentTimeMillis() / 1000); - claims.put(EXPIRY_IN_SECONDS, expirationDate.getTime() / 1000); - claims.put(CID, tokenRequestData.clientId); - claims.put(CLIENT_ID, tokenRequestData.clientId); - claims.put(ISS, tokenEndpointBuilder.getTokenEndpoint(IdentityZoneHolder.get())); - claims.put(ZONE_ID, IdentityZoneHolder.get().getId()); - claims.put(AUD, UaaStringUtils.getValuesOrDefaultValue(tokenRequestData.resourceIds, tokenRequestData.clientId)); - claims.put(GRANTED_SCOPES, tokenRequestData.scopes); - - if (null != tokenRequestData.authenticationMethods && !tokenRequestData.authenticationMethods.isEmpty()) { - claims.put(AMR, tokenRequestData.authenticationMethods); - } - if (null != tokenRequestData.authTime) { - claims.put(AUTH_TIME, AuthTimeDateConverter.dateToAuthTime(tokenRequestData.authTime)); - } - if (null != tokenRequestData.acr && !tokenRequestData.acr.isEmpty()) { - HashMap acrMap = Maps.newHashMap(); - acrMap.put(ACR_VALUES_KEY, tokenRequestData.acr); - claims.put(ACR, acrMap); - } - if (null != additionalAuthorizationAttributes) { - claims.put(ADDITIONAL_AZ_ATTR, additionalAuthorizationAttributes); - } - if (null != tokenRequestData.externalAttributes) { - claims.putAll(tokenRequestData.externalAttributes); - } - if (null != grantType) { - claims.put(GRANT_TYPE, grantType); - } - if (null != user) { - claims.put(USER_NAME, user.getUsername()); - claims.put(ORIGIN, user.getOrigin()); - claims.put(USER_ID, user.getId()); - } - - if (tokenRequestData.revocable) { - claims.put(ClaimConstants.REVOCABLE, true); - } - - if (hasText(revocableHashSignature)) { - claims.put(REVOCATION_SIGNATURE, revocableHashSignature); - } - - content = JsonUtils.writeValueAsString(claims); - } catch (JsonUtils.JsonUtilException e) { - throw new IllegalStateException("Cannot convert access token to JSON", e); + + Map claims = new LinkedHashMap<>(); + claims.put(JTI, tokenId); + claims.put(SUB, user.getId()); + claims.put(IAT, timeService.getCurrentTimeMillis() / 1000); + claims.put(EXPIRY_IN_SECONDS, expirationDate.getTime() / 1000); + claims.put(CID, tokenRequestData.clientId); + claims.put(CLIENT_ID, tokenRequestData.clientId); + claims.put(ISS, tokenEndpointBuilder.getTokenEndpoint(IdentityZoneHolder.get())); + claims.put(ZONE_ID, IdentityZoneHolder.get().getId()); + claims.put(AUD, UaaStringUtils.getValuesOrDefaultValue(tokenRequestData.resourceIds, tokenRequestData.clientId)); + claims.put(GRANTED_SCOPES, tokenRequestData.scopes); + + if (null != tokenRequestData.authenticationMethods && !tokenRequestData.authenticationMethods.isEmpty()) { + claims.put(AMR, tokenRequestData.authenticationMethods); + } + if (null != tokenRequestData.authTime) { + claims.put(AUTH_TIME, AuthTimeDateConverter.dateToAuthTime(tokenRequestData.authTime)); + } + if (null != tokenRequestData.acr && !tokenRequestData.acr.isEmpty()) { + HashMap acrMap = Maps.newHashMap(); + acrMap.put(ACR_VALUES_KEY, tokenRequestData.acr); + claims.put(ACR, acrMap); + } + if (null != additionalAuthorizationAttributes) { + claims.put(ADDITIONAL_AZ_ATTR, additionalAuthorizationAttributes); + } + if (null != tokenRequestData.externalAttributes) { + claims.putAll(tokenRequestData.externalAttributes); + } + if (null != grantType) { + claims.put(GRANT_TYPE, grantType); } - return JwtHelper.encode(content, getActiveKeyInfo()).getEncoded(); + if (null != user) { + claims.put(USER_NAME, user.getUsername()); + claims.put(ORIGIN, user.getOrigin()); + claims.put(USER_ID, user.getId()); + } + + if (tokenRequestData.revocable) { + claims.put(REVOCABLE, true); + } + + if (hasText(revocableHashSignature)) { + claims.put(REVOCATION_SIGNATURE, revocableHashSignature); + } + + return JwtHelper.encode(claims, getActiveKeyInfo()).getEncoded(); } private KeyInfo getActiveKeyInfo() { @@ -177,16 +169,15 @@ public boolean shouldRotateRefreshTokens() { return getActiveTokenPolicy().isRefreshTokenRotate(); } - private String getRefreshedTokenString(Claims claims) { + private HashMap getRefreshedTokenMap(Claims claims) { claims.setJti(UUID.randomUUID().toString().replace("-", "") + REFRESH_TOKEN_SUFFIX); - return JsonUtils.writeValueAsString(claims); + return claims.getClaimObject(); } public String createRefreshTokenValue(JwtTokenSignedByThisUAA jwtToken, Claims claims) { String refreshTokenValue; if (shouldRotateRefreshTokens()) { - String refreshTokenString = getRefreshedTokenString(claims); - refreshTokenValue = JwtHelper.encode(refreshTokenString, getActiveKeyInfo()).getEncoded(); + refreshTokenValue = JwtHelper.encode(getRefreshedTokenMap(claims), getActiveKeyInfo()).getEncoded(); } else { refreshTokenValue = jwtToken.getJwt().getEncoded(); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/ChainedSignatureVerifierTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/ChainedSignatureVerifierTests.java index c6fbe813d27..f6e417de998 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/ChainedSignatureVerifierTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/ChainedSignatureVerifierTests.java @@ -79,7 +79,7 @@ public void setup() { content = "{\"sub\": \"" + new RandomValueStringGenerator(1024 * 4).generate() + "\"}"; keyInfo = KeyInfoBuilder.build("valid", rsaSigningKey, "http://localhost/uaa"); - signedValidContent = JwtHelper.encode(content, keyInfo); + signedValidContent = JwtHelper.encode(JsonUtils.readValue(content, HashMap.class), keyInfo); validKey = new JsonWebKey(KeyInfoBuilder.build(null, rsaSigningKey, "http://localhost/uaa").getJwkMap()); invalidKey = new JsonWebKey(KeyInfoBuilder.build(null, invalidRsaSigningKey, "http://localhost/uaa").getJwkMap()); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java index cd7ea0c4fd3..8f7163249f7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java @@ -6,7 +6,6 @@ import org.cloudfoundry.identity.uaa.oauth.KeyInfoBuilder; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; import org.cloudfoundry.identity.uaa.oauth.token.Claims; -import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.UaaTokenUtils; import org.junit.Before; import org.junit.Test; @@ -35,7 +34,7 @@ public void setUp() { @Test public void testKidInHeader() { - Jwt jwt = JwtHelper.encode(JsonUtils.writeValueAsString(Map.of("sub", "testJwtContent")), keyInfo); + Jwt jwt = JwtHelper.encode(Map.of("sub", "testJwtContent"), keyInfo); assertEquals("testKid", jwt.getHeader().getKid()); jwt = JwtHelper.decode(jwt.getEncoded()); @@ -44,7 +43,7 @@ public void testKidInHeader() { @Test public void jwtHeaderShouldContainJkuInTheHeader() { - Jwt jwt = JwtHelper.encode(JsonUtils.writeValueAsString(Map.of("sub", "testJwtContent")), keyInfo); + Jwt jwt = JwtHelper.encode(Map.of("sub", "testJwtContent"), keyInfo); assertEquals("https://localhost/uaa/token_keys", jwt.getHeader().getJku()); } @@ -58,9 +57,9 @@ public void jwtHeaderShouldNotContainJkuInTheHeaderIfCertificateDefined() { @Test public void testAudClaimTypes() { - Jwt audSingle = JwtHelper.encode(JsonUtils.writeValueAsString(Map.of("sub", "subject", "aud", "single")), keyInfo); - Jwt audArray = JwtHelper.encode(JsonUtils.writeValueAsString(Map.of("sub", "subject", "aud", Arrays.asList("one"))), keyInfo); - Jwt audArrayThree = JwtHelper.encode(JsonUtils.writeValueAsString(Map.of("sub", "subject", "aud", Arrays.asList("one", "two", "three"))), keyInfo); + Jwt audSingle = JwtHelper.encode(Map.of("sub", "subject", "aud", "single"), keyInfo); + Jwt audArray = JwtHelper.encode(Map.of("sub", "subject", "aud", Arrays.asList("one")), keyInfo); + Jwt audArrayThree = JwtHelper.encode(Map.of("sub", "subject", "aud", Arrays.asList("one", "two", "three")), keyInfo); Claims claimSingle = UaaTokenUtils.getClaimsFromTokenString(audSingle.getEncoded()); assertNotNull(claimSingle); @@ -86,7 +85,7 @@ public void testLegacyHmacVerify() { JsonWebKey jsonWebKey = new JsonWebKey(key); SignatureVerifier cs = new SignatureVerifier(jsonWebKey); KeyInfo hmacKeyInfo = new KeyInfo(kid, keyValue, DEFAULT_UAA_URL); - Jwt legacySignature = JwtHelper.encode(JsonUtils.writeValueAsString(Map.of("sub", "subject", "aud", "single")), hmacKeyInfo); + Jwt legacySignature = JwtHelper.encode(Map.of("sub", "subject", "aud", "single"), hmacKeyInfo); assertNotNull(legacySignature); Jwt legacyVerify = JwtHelper.decode(legacySignature.getEncoded()); assertNotNull(legacyVerify); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java index b906cd18d1e..4ae23df29eb 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java @@ -1,9 +1,6 @@ package org.cloudfoundry.identity.uaa.util; import com.nimbusds.jose.KeyLengthException; -import org.cloudfoundry.identity.uaa.oauth.KeyInfoBuilder; -import org.cloudfoundry.identity.uaa.oauth.jwt.Jwt; -import org.cloudfoundry.identity.uaa.oauth.jwt.JwtHelper; import org.cloudfoundry.identity.uaa.login.util.RandomValueStringGenerator; import org.cloudfoundry.identity.uaa.oauth.jwt.UaaMacSigner; import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants; @@ -163,12 +160,6 @@ public void getClaims_throwsExceptionWhenJwtIsMalformed() { UaaTokenUtils.getClaims("not.a.jwt"); } - @Test(expected = InvalidTokenException.class) - public void getClaims_throwsExceptionWhenClaimsCannotBeRead() { - Jwt encoded = JwtHelper.encode("great content", KeyInfoBuilder.build("foo", "bar", "https://localhost/uaa")); - UaaTokenUtils.getClaims(encoded.getEncoded()); - } - @Test public void getClaims_WhenClaimsAreMissing_returnsEmptyMap() { Map headers = new HashMap<>(); From 8dc5b526e3adb69aac82fe08812967c89772dd13 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 13 Jan 2024 10:10:48 +0100 Subject: [PATCH 2/7] return map as sonar suggested --- .../java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java | 2 +- .../identity/uaa/oauth/refresh/RefreshTokenCreator.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java index 8bc6a91e2d6..e47edc3ef19 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java @@ -368,7 +368,7 @@ public void setClientAuth(final String clientAuth) { } @JsonIgnore - public HashMap getClaimObject() { + public Map getClaimObject() { return JsonUtils.convertValue(this, HashMap.class); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java index 3c7c9994060..65ec62f5f0d 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java @@ -169,7 +169,7 @@ public boolean shouldRotateRefreshTokens() { return getActiveTokenPolicy().isRefreshTokenRotate(); } - private HashMap getRefreshedTokenMap(Claims claims) { + private Map getRefreshedTokenMap(Claims claims) { claims.setJti(UUID.randomUUID().toString().replace("-", "") + REFRESH_TOKEN_SUFFIX); return claims.getClaimObject(); } From 1439d07cad8516be0655cedf74dc8fe20c876a69 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 13 Jan 2024 10:11:11 +0100 Subject: [PATCH 3/7] more tests and therefore simplify code --- .../uaa/oauth/token/IntrospectionClaimsTest.java | 2 ++ .../identity/uaa/oauth/jwt/JwtHelper.java | 13 +++++-------- .../identity/uaa/oauth/jwt/JwtHeaderHelperTest.java | 6 ++++++ .../identity/uaa/oauth/jwt/JwtHelperTest.java | 13 +++++++++++++ 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java index 43f6284fe23..b3b00ae44c0 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java @@ -8,6 +8,7 @@ import java.text.ParseException; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; class IntrospectionClaimsTest { @@ -36,5 +37,6 @@ void isActive() { @Test void testSerialize() { assertTrue(JsonUtils.writeValueAsString(INTROSPECTION_PAYLOAD).contains(TokenConstants.CLIENT_AUTH_NONE)); + assertNotNull(INTROSPECTION_PAYLOAD.getClaimObject()); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java index 95cd50f25f9..98fa3b8f5c5 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java @@ -38,6 +38,7 @@ import java.text.ParseException; import java.util.HashMap; import java.util.Map; +import java.util.Optional; /** * @author Luke Taylor @@ -166,15 +167,11 @@ class JwtImpl implements Jwt { this.parsedJwtObject = null; this.content = null; try { - this.claimsSet = JWTClaimsSet.parse(payLoad); + this.claimsSet = JWTClaimsSet.parse(Optional.ofNullable(payLoad).orElseThrow(() -> new JOSEException("Payload null"))); JWSHeader joseHeader = JWSHeader.parse(JsonUtils.convertValue(header.parameters, HashMap.class)); - if (signature != null) { - SignedJWT signedJWT = new SignedJWT(joseHeader, claimsSet); - signedJWT.sign(signature); - signedJwtObject = signedJWT; - } else { - signedJwtObject = null; - } + SignedJWT signedJWT = new SignedJWT(joseHeader, claimsSet); + signedJWT.sign(signature); + signedJwtObject = signedJWT; } catch (ParseException | JOSEException e) { throw new InvalidTokenException(INVALID_TOKEN, e); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHeaderHelperTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHeaderHelperTest.java index 962526f39c9..907d7de0dca 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHeaderHelperTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHeaderHelperTest.java @@ -14,6 +14,7 @@ import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertThrows; @Tag("https://tools.ietf.org/html/rfc7519#section-5") @DisplayName("JOSE Header") @@ -174,6 +175,11 @@ void shouldAllowCritHeader() { assertThat(header.parameters.crit, hasItems("first-val", "value-2")); } + + @Test + void testInvalidHeader() { + assertThrows(IllegalArgumentException.class, () ->JwtHeaderHelper.create("")); + } } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java index 8f7163249f7..e6417bf2068 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelperTest.java @@ -9,6 +9,8 @@ import org.cloudfoundry.identity.uaa.util.UaaTokenUtils; import org.junit.Before; import org.junit.Test; +import org.springframework.security.authentication.InsufficientAuthenticationException; +import org.springframework.security.oauth2.common.exceptions.InvalidTokenException; import java.util.Arrays; import java.util.HashMap; @@ -99,4 +101,15 @@ public void testLegacyHmacVerify() { public void testLegacyHmacFailed() { assertThrows(InvalidSignatureException.class, () -> UaaMacSigner.verify("x", null)); } + + @Test + public void testJwtInvalidPayload() { + assertThrows(InvalidTokenException.class, () -> JwtHelper.encode(null, keyInfo)); + } + + @Test + public void testJwtInvalidContent() { + assertThrows(InvalidTokenException.class, () -> JwtHelper.decode("invalid")); + assertThrows(InsufficientAuthenticationException.class, () -> JwtHelper.decode("")); + } } From 4d692b7007e0c9494126b7815d4959818b9b8224 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 13 Jan 2024 10:29:32 +0100 Subject: [PATCH 4/7] sonar --- .../org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java index 5915de8f464..3fd0a71140e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java @@ -159,7 +159,7 @@ public String userId() { } @JsonIgnore - public HashMap getClaimObject() { + public Map getClaimObject() { return JsonUtils.convertValue(this, HashMap.class); } } From 5a0735bb27e158a335c86453e8c52136d9997801 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 13 Jan 2024 11:32:12 +0100 Subject: [PATCH 5/7] parse token only once in JwtTokenSignedByThisUAA further simplify utils --- .../oauth/jwt/JwtClientAuthentication.java | 4 +-- .../uaa/util/JwtTokenSignedByThisUAA.java | 2 +- .../identity/uaa/util/UaaTokenUtils.java | 25 ++++++++----------- .../uaa/oauth/RefreshRotationTest.java | 16 ++++++------ .../refresh/RefreshTokenCreatorTest.java | 4 +-- .../identity/uaa/util/UaaTokenUtilsTest.java | 6 ++--- .../PasswordGrantIntegrationTests.java | 2 +- .../feature/OpenIdTokenGrantsIT.java | 2 +- .../mock/token/RefreshTokenMockMvcTests.java | 12 ++++----- .../uaa/mock/token/TokenMvcMockTests.java | 2 +- .../uaa/oauth/UaaTokenServicesTests.java | 6 ++--- 11 files changed, 37 insertions(+), 44 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java index d47b42301eb..a9c06b46b16 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java @@ -41,8 +41,6 @@ import java.util.Set; import java.util.UUID; -import static org.cloudfoundry.identity.uaa.util.UaaTokenUtils.getMapFromClaims; - public class JwtClientAuthentication { public static final String GRANT_TYPE = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; @@ -81,7 +79,7 @@ public String getClientAssertion(OIDCIdentityProviderDefinition config) { claims.setExp(Instant.now().plusSeconds(300).getEpochSecond()); KeyInfo signingKeyInfo = Optional.ofNullable(keyInfoService.getKey(kid)).orElseThrow(() -> new BadCredentialsException("Missing requested signing key")); return signingKeyInfo.verifierCertificate().isPresent() ? - JwtHelper.encodePlusX5t(getMapFromClaims(claims), signingKeyInfo, signingKeyInfo.verifierCertificate().orElseThrow()).getEncoded() : + JwtHelper.encodePlusX5t(claims.getClaimObject(), signingKeyInfo, signingKeyInfo.verifierCertificate().orElseThrow()).getEncoded() : JwtHelper.encode(claims.getClaimObject(), signingKeyInfo).getEncoded(); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/util/JwtTokenSignedByThisUAA.java b/server/src/main/java/org/cloudfoundry/identity/uaa/util/JwtTokenSignedByThisUAA.java index 8cdd7b89c6b..3696e3567e1 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/util/JwtTokenSignedByThisUAA.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/util/JwtTokenSignedByThisUAA.java @@ -103,9 +103,9 @@ List requestedScopes() { private JwtTokenSignedByThisUAA(String token, KeyInfoService keyInfoService) { this.token = token; - this.claims = UaaTokenUtils.getClaims(token); this.tokenJwt = JwtHelper.decode(token); this.keyInfoService = keyInfoService; + this.claims = tokenJwt.getClaimSet().toType(UaaTokenUtils.getClaimsSetTransformer(Map.class)); } private SignatureVerifier fetchSignatureVerifierFromToken(Jwt tokenJwt) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtils.java b/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtils.java index 0e2804f4536..3e96aebaa22 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtils.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtils.java @@ -249,30 +249,25 @@ public static boolean hasRequiredUserGroups(Collection requiredGroups, C public static T getClaims(String jwtToken, Class toClazz) { Object claims; try { - JWTClaimsSetTransformer claimsTransformer = claimsSet -> { - Map claimMap = claimsSet.toJSONObject(); - Object audObject = claimsSet.getAudience(); - if (isNotEmpty(audObject)) { - claimMap.put(ClaimConstants.AUD, claimsSet.getAudience()); - } - return JsonUtils.convertValue(claimMap, toClazz); - }; - claims = JwtHelper.decode(jwtToken).getClaimSet().toType(claimsTransformer); + claims = JwtHelper.decode(jwtToken).getClaimSet().toType(getClaimsSetTransformer(toClazz)); } catch (Exception ex) { throw new InvalidTokenException("Invalid token (could not decode): " + jwtToken, ex); } return claims != null ? (T) claims : (T) new Object(); } - public static Map getClaims(String jwtToken) { - return getClaims(jwtToken, Map.class); - } - public static Claims getClaimsFromTokenString(String jwtToken) { return getClaims(jwtToken, Claims.class); } - public static Map getMapFromClaims(Claims claims) { - return JsonUtils.convertValue(claims, Map.class); + public static JWTClaimsSetTransformer getClaimsSetTransformer(Class toClazz) { + return claimsSet -> { + Map claimMap = claimsSet.toJSONObject(); + Object audObject = claimsSet.getAudience(); + if (isNotEmpty(audObject)) { + claimMap.put(ClaimConstants.AUD, claimsSet.getAudience()); + } + return JsonUtils.convertValue(claimMap, toClazz); + }; } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/RefreshRotationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/RefreshRotationTest.java index 95ef330e183..3b8c63512c8 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/RefreshRotationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/RefreshRotationTest.java @@ -127,7 +127,7 @@ void testRefreshPublicClientWithRotation() { new IdentityZoneManagerImpl().getCurrentIdentityZone().getConfig().getTokenPolicy().setRefreshTokenRotate(true); CompositeToken accessToken = (CompositeToken) tokenServices.createAccessToken(authentication); - assertThat(UaaTokenUtils.getClaims(accessToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(accessToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); String refreshTokenValue = accessToken.getRefreshToken().getValue(); assertThat(refreshTokenValue, is(notNullValue())); @@ -135,11 +135,11 @@ void testRefreshPublicClientWithRotation() { OAuth2AccessToken refreshedToken = tokenServices.refreshAccessToken(refreshTokenValue, new TokenRequest(new HashMap<>(), CLIENT_ID, Lists.newArrayList("openid"), GRANT_TYPE_REFRESH_TOKEN)); assertThat(refreshedToken, is(notNullValue())); assertNotEquals("New access token should be different from the old one.", refreshTokenValue, refreshedToken.getRefreshToken().getValue()); - assertThat(UaaTokenUtils.getClaims(refreshedToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(refreshedToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); refreshedToken = tokenServices.refreshAccessToken(refreshTokenValue, new TokenRequest(new HashMap<>(), CLIENT_ID, Lists.newArrayList("openid"), GRANT_TYPE_REFRESH_TOKEN)); assertNotEquals("New access token should be different from the old one.", refreshTokenValue, refreshedToken.getRefreshToken().getValue()); - assertThat(UaaTokenUtils.getClaims(refreshedToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(refreshedToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); } @Test @@ -159,7 +159,7 @@ void testRefreshPublicClientWithRotationAndEmpyAuthentication() { new IdentityZoneManagerImpl().getCurrentIdentityZone().getConfig().getTokenPolicy().setRefreshTokenRotate(true); CompositeToken accessToken = (CompositeToken) tokenServices.createAccessToken(authentication); - assertThat(UaaTokenUtils.getClaims(accessToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(accessToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); String refreshTokenValue = accessToken.getRefreshToken().getValue(); assertThat(refreshTokenValue, is(notNullValue())); @@ -168,11 +168,11 @@ void testRefreshPublicClientWithRotationAndEmpyAuthentication() { OAuth2AccessToken refreshedToken = tokenServices.refreshAccessToken(refreshTokenValue, new TokenRequest(new HashMap<>(), CLIENT_ID, Lists.newArrayList("openid"), GRANT_TYPE_REFRESH_TOKEN)); assertThat(refreshedToken, is(notNullValue())); assertNotEquals("New access token should be different from the old one.", refreshTokenValue, refreshedToken.getRefreshToken().getValue()); - assertThat(UaaTokenUtils.getClaims(refreshedToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(refreshedToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); refreshedToken = tokenServices.refreshAccessToken(refreshTokenValue, new TokenRequest(new HashMap<>(), CLIENT_ID, Lists.newArrayList("openid"), GRANT_TYPE_REFRESH_TOKEN)); assertNotEquals("New access token should be different from the old one.", refreshTokenValue, refreshedToken.getRefreshToken().getValue()); - assertThat(UaaTokenUtils.getClaims(refreshedToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(refreshedToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); } @Test @@ -191,7 +191,7 @@ void testRefreshPublicClientWithoutRotation() { OAuth2Authentication authentication = new OAuth2Authentication(oAuth2Request, tokenSupport.defaultUserAuthentication); CompositeToken accessToken = (CompositeToken) tokenServices.createAccessToken(authentication); - assertThat(UaaTokenUtils.getClaims(accessToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(accessToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); String refreshTokenValue = accessToken.getRefreshToken().getValue(); assertThat(refreshTokenValue, is(notNullValue())); @@ -218,7 +218,7 @@ void testRefreshPublicClientButExistingTokenWasEmptyAuthentication() { new IdentityZoneManagerImpl().getCurrentIdentityZone().getConfig().getTokenPolicy().setRefreshTokenRotate(true); CompositeToken accessToken = (CompositeToken) tokenServices.createAccessToken(authentication); - assertThat(UaaTokenUtils.getClaims(accessToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); + assertThat((Map) UaaTokenUtils.getClaims(accessToken.getValue(), Map.class), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); String refreshTokenValue = accessToken.getRefreshToken().getValue(); assertThat(refreshTokenValue, is(notNullValue())); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreatorTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreatorTest.java index 28a1cde4d79..a1c49125e9d 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreatorTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreatorTest.java @@ -96,7 +96,7 @@ public void refreshToken_includesClaimsNeededToBuildIdTokens() { ExpiringOAuth2RefreshToken refreshToken = refreshTokenCreator.createRefreshToken(user, refreshTokenRequestData, "abcdef"); - Map refreshClaims = UaaTokenUtils.getClaims(refreshToken.getValue()); + Map refreshClaims = UaaTokenUtils.getClaims(refreshToken.getValue(), Map.class); assertThat(refreshClaims.get(AUTH_TIME), is(1L)); assertThat((List) refreshClaims.get(AMR), hasItem("pwd")); assertThat((Map>) refreshClaims.get(ACR), hasKey("values")); @@ -131,7 +131,7 @@ public void refreshToken_ifIdTokenClaimsAreUnknown_omitsThem() { ExpiringOAuth2RefreshToken refreshToken = refreshTokenCreator.createRefreshToken(user, refreshTokenRequestData, "abcdef"); - Map refreshClaims = UaaTokenUtils.getClaims(refreshToken.getValue()); + Map refreshClaims = UaaTokenUtils.getClaims(refreshToken.getValue(), Map.class); assertFalse(refreshClaims.containsKey(AUTH_TIME)); assertFalse(refreshClaims.containsKey(AMR)); assertFalse(refreshClaims.containsKey(ACR)); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java index 4ae23df29eb..e6d51bf23f4 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaTokenUtilsTest.java @@ -142,7 +142,7 @@ public void getClaims() throws KeyLengthException { content.put("aud", "openidclient"); String jwt = UaaTokenUtils.constructToken(headers, content, new UaaMacSigner("foobar")); - Map claims = UaaTokenUtils.getClaims(jwt); + Map claims = UaaTokenUtils.getClaims(jwt, Map.class); assertEquals("openidclient", claims.get("cid")); assertEquals("uaa", claims.get("origin")); @@ -157,7 +157,7 @@ public void getClaims() throws KeyLengthException { @Test(expected = InvalidTokenException.class) public void getClaims_throwsExceptionWhenJwtIsMalformed() { - UaaTokenUtils.getClaims("not.a.jwt"); + UaaTokenUtils.getClaims("not.a.jwt", Map.class); } @Test @@ -167,7 +167,7 @@ public void getClaims_WhenClaimsAreMissing_returnsEmptyMap() { headers.put("alg", "HS256"); String tokenWithNoClaims = UaaTokenUtils.constructToken(headers, new HashMap<>(), new UaaMacSigner("foobar")); - Map claims = UaaTokenUtils.getClaims(tokenWithNoClaims); + Map claims = UaaTokenUtils.getClaims(tokenWithNoClaims, Map.class); assertNotNull(claims); assertEquals(0, claims.size()); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/PasswordGrantIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/PasswordGrantIntegrationTests.java index 6069075dc07..62e31257093 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/PasswordGrantIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/PasswordGrantIntegrationTests.java @@ -157,7 +157,7 @@ protected static String validateClientAuthenticationMethod(ResponseEntity jsonBody = JsonUtils.readValue(responseEntity.getBody(), new TypeReference>() {}); String accessToken = (String) jsonBody.get("access_token"); assertThat(accessToken, is(notNullValue())); - Map claims = UaaTokenUtils.getClaims(accessToken); + Map claims = UaaTokenUtils.getClaims(accessToken, Map.class); if (isNone) { assertThat(claims, hasEntry(ClaimConstants.CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); } else { diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OpenIdTokenGrantsIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OpenIdTokenGrantsIT.java index 4af42689dda..3800ea91289 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OpenIdTokenGrantsIT.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OpenIdTokenGrantsIT.java @@ -170,7 +170,7 @@ public void testImplicitGrant() { } private void validateToken(String paramName, Map params, String[] scopes, String[] aud) { - Map claims = UaaTokenUtils.getClaims((String)params.get(paramName)); + Map claims = UaaTokenUtils.getClaims((String)params.get(paramName), Map.class); assertThat(claims.get("jti"), is(params.get("jti"))); assertThat(claims.get("client_id"), is("cf")); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/RefreshTokenMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/RefreshTokenMockMvcTests.java index e76c3fda70d..f4f6f7aa6c6 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/RefreshTokenMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/RefreshTokenMockMvcTests.java @@ -321,7 +321,7 @@ void test_opaque_refresh_tokens_sets_revocable_claim() throws Exception { String tokenId = getJwtRefreshToken(client.getClientId(), SECRET, user.getUserName(), SECRET, getZoneHostUrl(zone)); IdentityZoneHolder.set(zone); String token = revocableTokenProvisioning.retrieve(tokenId, IdentityZoneHolder.get().getId()).getValue(); - Map claims = UaaTokenUtils.getClaims(token); + Map claims = UaaTokenUtils.getClaims(token, Map.class); assertNotNull(claims.get(ClaimConstants.REVOCABLE)); assertTrue((Boolean) claims.get(ClaimConstants.REVOCABLE)); } @@ -341,8 +341,8 @@ void test_opaque_refresh_unique_tokens_count() throws Exception { private void assertRefreshIdTokenCorrect(String originalIdTokenJwt, String idTokenJwtFromRefreshGrant) { assertNotNull(idTokenJwtFromRefreshGrant); - Map originalIdClaims = getClaims(originalIdTokenJwt); - Map idClaims = getClaims(idTokenJwtFromRefreshGrant); + Map originalIdClaims = getClaims(originalIdTokenJwt, Map.class); + Map idClaims = getClaims(idTokenJwtFromRefreshGrant, Map.class); // These claims should be the same in the old and new id tokens: auth_time, iss, sub, azp // http://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse @@ -431,13 +431,13 @@ void refreshTokenGrantType_withJwtTokens_preservesRefreshTokenExpiryClaim() thro assertEquals(HttpStatus.SC_OK, refreshResponse.getStatus()); CompositeToken compositeToken = JsonUtils.readValue(refreshResponse.getContentAsString(), CompositeToken.class); String refreshTokenJwt = compositeToken.getRefreshToken().getValue(); - assertThat(getClaims(refreshTokenJwt).get(EXPIRY_IN_SECONDS), equalTo(getClaims(refreshToken).get(EXPIRY_IN_SECONDS))); + assertThat(getClaims(refreshTokenJwt, Map.class).get(EXPIRY_IN_SECONDS), equalTo(getClaims(refreshToken, Map.class).get(EXPIRY_IN_SECONDS))); CompositeToken newTokenResponse = getTokensWithPasswordGrant(client.getClientId(), SECRET, user.getUserName(), SECRET, getZoneHostUrl(zone), "jwt"); String newRefreshToken = newTokenResponse.getRefreshToken().getValue(); - assertThat(getClaims(newRefreshToken).get(EXPIRY_IN_SECONDS), not(nullValue())); - assertThat(getClaims(newRefreshToken).get(EXPIRY_IN_SECONDS), not(equalTo(getClaims(refreshToken).get(EXPIRY_IN_SECONDS)))); + assertThat(getClaims(newRefreshToken, Map.class).get(EXPIRY_IN_SECONDS), not(nullValue())); + assertThat(getClaims(newRefreshToken, Map.class).get(EXPIRY_IN_SECONDS), not(equalTo(getClaims(refreshToken, Map.class).get(EXPIRY_IN_SECONDS)))); } @Test diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockTests.java index c4b669000cd..4ecc1e5f32c 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockTests.java @@ -3955,7 +3955,7 @@ void testRefreshGrant_returnsValidAccessToken() throws Exception { body = doRefreshGrant(refreshToken, clientId, SECRET, status().isOk()).getResponse().getContentAsString(); CompositeToken tokenResponse = JsonUtils.readValue(body, CompositeToken.class); - Map claims = UaaTokenUtils.getClaims(tokenResponse.getValue()); + Map claims = UaaTokenUtils.getClaims(tokenResponse.getValue(), Map.class); assertThat(claims.get(JTI).toString(), not(endsWith("-r"))); } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java index 606925d201a..3bea3ef0944 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java @@ -294,7 +294,7 @@ void happyCase() { OAuth2AccessToken refreshedToken = tokenServices.refreshAccessToken(this.refreshToken.getValue(), new TokenRequest(new HashMap<>(), "jku_test", Lists.newArrayList("openid", "user_attributes"), GRANT_TYPE_REFRESH_TOKEN)); assertThat(refreshedToken, is(notNullValue())); - Map claims = UaaTokenUtils.getClaims(refreshedToken.getValue()); + Map claims = UaaTokenUtils.getClaims(refreshedToken.getValue(), Map.class); assertThat(claims, hasEntry(ClaimConstants.CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE)); } @@ -361,7 +361,7 @@ void happyCase(List acrs) { assertThat(refreshedToken, is(notNullValue())); - Map claims = UaaTokenUtils.getClaims(refreshedToken.getIdTokenValue()); + Map claims = UaaTokenUtils.getClaims(refreshedToken.getIdTokenValue(), Map.class); assertThat(claims.size(), greaterThan(0)); assertThat(claims, hasKey(ClaimConstants.ACR)); assertThat(claims.get(ClaimConstants.ACR), notNullValue()); @@ -488,7 +488,7 @@ void happyCase(List amrs) { assertThat(refreshedToken, is(notNullValue())); - Map claims = UaaTokenUtils.getClaims(refreshedToken.getIdTokenValue()); + Map claims = UaaTokenUtils.getClaims(refreshedToken.getIdTokenValue(), Map.class); assertThat(claims.size(), greaterThan(0)); assertThat(claims, hasKey(ClaimConstants.AMR)); assertThat(claims.get(ClaimConstants.AMR), notNullValue()); From c3dabb4c5db579e5f7cf993acf68855d8531dd54 Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 18 Jan 2024 18:58:56 +0100 Subject: [PATCH 6/7] check some entries --- .../identity/uaa/oauth/token/IntrospectionClaimsTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java index b3b00ae44c0..73d8aabf91f 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java @@ -6,7 +6,10 @@ import org.junit.jupiter.api.Test; import java.text.ParseException; +import java.util.Arrays; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasEntry; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -38,5 +41,9 @@ void isActive() { void testSerialize() { assertTrue(JsonUtils.writeValueAsString(INTROSPECTION_PAYLOAD).contains(TokenConstants.CLIENT_AUTH_NONE)); assertNotNull(INTROSPECTION_PAYLOAD.getClaimObject()); + assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("grant_type", "authorization_code")); + assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("client_id", "login")); + assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("aud", Arrays.asList("openid", "login"))); + assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("scope", Arrays.asList("openid"))); } } From a43dc7127bc40d0d5d7ba0979e7336549dc9ead9 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 19 Jan 2024 07:09:27 +0100 Subject: [PATCH 7/7] review --- .../cloudfoundry/identity/uaa/oauth/token/Claims.java | 2 +- .../uaa/oauth/token/IntrospectionClaimsTest.java | 10 +++++----- .../identity/uaa/oauth/UaaTokenServices.java | 2 +- .../uaa/oauth/jwt/JwtClientAuthentication.java | 4 ++-- .../identity/uaa/oauth/openid/IdToken.java | 2 +- .../uaa/oauth/refresh/RefreshTokenCreator.java | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java index e47edc3ef19..d22eb189292 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java @@ -368,7 +368,7 @@ public void setClientAuth(final String clientAuth) { } @JsonIgnore - public Map getClaimObject() { + public Map getClaimMap() { return JsonUtils.convertValue(this, HashMap.class); } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java index 73d8aabf91f..79a2d7a2853 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java @@ -40,10 +40,10 @@ void isActive() { @Test void testSerialize() { assertTrue(JsonUtils.writeValueAsString(INTROSPECTION_PAYLOAD).contains(TokenConstants.CLIENT_AUTH_NONE)); - assertNotNull(INTROSPECTION_PAYLOAD.getClaimObject()); - assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("grant_type", "authorization_code")); - assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("client_id", "login")); - assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("aud", Arrays.asList("openid", "login"))); - assertThat(INTROSPECTION_PAYLOAD.getClaimObject(), hasEntry("scope", Arrays.asList("openid"))); + assertNotNull(INTROSPECTION_PAYLOAD.getClaimMap()); + assertThat(INTROSPECTION_PAYLOAD.getClaimMap(), hasEntry("grant_type", "authorization_code")); + assertThat(INTROSPECTION_PAYLOAD.getClaimMap(), hasEntry("client_id", "login")); + assertThat(INTROSPECTION_PAYLOAD.getClaimMap(), hasEntry("aud", Arrays.asList("openid", "login"))); + assertThat(INTROSPECTION_PAYLOAD.getClaimMap(), hasEntry("scope", Arrays.asList("openid"))); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java index 0ed19812581..3ef9e5b7b00 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java @@ -475,7 +475,7 @@ private CompositeToken createCompositeToken(String tokenId, } catch (RuntimeException | IdTokenCreationException ignored) { throw new IllegalStateException("Cannot convert id token to JSON"); } - String encodedIdTokenContent = JwtHelper.encode(idTokenContent.getClaimObject(), keyInfoService.getActiveKey()).getEncoded(); + String encodedIdTokenContent = JwtHelper.encode(idTokenContent.getClaimMap(), keyInfoService.getActiveKey()).getEncoded(); compositeToken.setIdTokenValue(encodedIdTokenContent); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java index a9c06b46b16..e98423874cb 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java @@ -79,8 +79,8 @@ public String getClientAssertion(OIDCIdentityProviderDefinition config) { claims.setExp(Instant.now().plusSeconds(300).getEpochSecond()); KeyInfo signingKeyInfo = Optional.ofNullable(keyInfoService.getKey(kid)).orElseThrow(() -> new BadCredentialsException("Missing requested signing key")); return signingKeyInfo.verifierCertificate().isPresent() ? - JwtHelper.encodePlusX5t(claims.getClaimObject(), signingKeyInfo, signingKeyInfo.verifierCertificate().orElseThrow()).getEncoded() : - JwtHelper.encode(claims.getClaimObject(), signingKeyInfo).getEncoded(); + JwtHelper.encodePlusX5t(claims.getClaimMap(), signingKeyInfo, signingKeyInfo.verifierCertificate().orElseThrow()).getEncoded() : + JwtHelper.encode(claims.getClaimMap(), signingKeyInfo).getEncoded(); } public MultiValueMap getClientAuthenticationParameters(MultiValueMap params, OIDCIdentityProviderDefinition config) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java index 3fd0a71140e..d89e159653a 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java @@ -159,7 +159,7 @@ public String userId() { } @JsonIgnore - public Map getClaimObject() { + public Map getClaimMap() { return JsonUtils.convertValue(this, HashMap.class); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java index 65ec62f5f0d..eb4c73021dc 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/refresh/RefreshTokenCreator.java @@ -171,7 +171,7 @@ public boolean shouldRotateRefreshTokens() { private Map getRefreshedTokenMap(Claims claims) { claims.setJti(UUID.randomUUID().toString().replace("-", "") + REFRESH_TOKEN_SUFFIX); - return claims.getClaimObject(); + return claims.getClaimMap(); } public String createRefreshTokenValue(JwtTokenSignedByThisUAA jwtToken, Claims claims) {