Skip to content

Commit

Permalink
Post jwt token library refactoring (#2679)
Browse files Browse the repository at this point in the history 
* Post jwt token library refactorings

- remove and simplify token creation. Only from map instead of a string
- reduce complexity in token creation

* return map as sonar suggested

* more tests and therefore simplify code

* sonar

* parse token only once in JwtTokenSignedByThisUAA

further simplify utils

* check some entries

* review
  • Loading branch information
@strehle
strehle authored Jan 19, 2024
1 parent da2f80c commit 9e98740
Show file tree
Hide file tree
Showing 22 changed files with 161 additions and 176 deletions.
8 changes: 8 additions & 0 deletions model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/Claims.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -363,4 +366,9 @@ public String getClientAuth() {
public void setClientAuth(final String clientAuth) {
this.clientAuth = clientAuth;
}

@JsonIgnore
public Map<String, Object> getClaimMap() {
return JsonUtils.convertValue(this, HashMap.class);
}
}
7 changes: 4 additions & 3 deletions model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -321,11 +322,11 @@ public static String getSafeParameterValue(String[] value) {
return StringUtils.hasText(value[0]) ? value[0] : EMPTY_STRING;
}

public static Set<String> getValuesOrDefaultValue(Set<String> values, String defaultValue) {
public static List<String> getValuesOrDefaultValue(Set<String> values, String defaultValue) {
if (ObjectUtils.isEmpty(values)) {
return Set.of(defaultValue);
return List.of(defaultValue);
} else {
return values;
return new ArrayList<>(values);
}
}
}
9 changes: 9 additions & 0 deletions model/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/IntrospectionClaimsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
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;

class IntrospectionClaimsTest {
Expand Down Expand Up @@ -36,5 +40,10 @@ void isActive() {
@Test
void testSerialize() {
assertTrue(JsonUtils.writeValueAsString(INTROSPECTION_PAYLOAD).contains(TokenConstants.CLIENT_AUTH_NONE));
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")));
}
}
8 changes: 5 additions & 3 deletions model/src/test/java/org/cloudfoundry/identity/uaa/util/UaaStringUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 7 additions & 12 deletions server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -451,8 +452,7 @@ private CompositeToken createCompositeToken(String tokenId,

compositeToken.setAdditionalInformation(info);

String content;
Map<String, ?> jwtAccessToken = createJWTAccessToken(
Map<String, Object> jwtAccessToken = createJWTAccessToken(
compositeToken,
user,
userAuthenticationTime,
Expand All @@ -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.getClaimMap(), keyInfoService.getActiveKey()).getEncoded();
compositeToken.setIdTokenValue(encodedIdTokenContent);
}

Expand All @@ -501,7 +496,7 @@ private KeyInfo getActiveKeyInfo() {
.orElseThrow(() -> new InternalAuthenticationServiceException("Unable to sign token, misconfigured JWT signing keys"));
}

private Map<String, ?> createJWTAccessToken(OAuth2AccessToken token,
private Map<String, Object> createJWTAccessToken(OAuth2AccessToken token,
UaaUser user,
Date userAuthenticationTime,
Collection<GrantedAuthority> clientScopes,
Expand Down
7 changes: 2 additions & 5 deletions server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,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";
Expand Down Expand Up @@ -82,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(getMapFromClaims(claims), signingKeyInfo, signingKeyInfo.verifierCertificate().orElseThrow()).getEncoded() :
JwtHelper.encode(JsonUtils.writeValueAsString(claims), signingKeyInfo).getEncoded();
JwtHelper.encodePlusX5t(claims.getClaimMap(), signingKeyInfo, signingKeyInfo.verifierCertificate().orElseThrow()).getEncoded() :
JwtHelper.encode(claims.getClaimMap(), signingKeyInfo).getEncoded();
}

public MultiValueMap<String, String> getClientAuthenticationParameters(MultiValueMap<String, String> params, OIDCIdentityProviderDefinition config) {
Expand Down
44 changes: 8 additions & 36 deletions server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.text.ParseException;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

/**
* @author Luke Taylor
Expand Down Expand Up @@ -66,14 +67,10 @@ public static Jwt encodePlusX5t(Map<String, Object> payLoad, KeyInfo keyInfo, X5
return createJwt(header, payLoad, keyInfo);
}

public static Jwt encode(CharSequence content, KeyInfo keyInfo) {
public static Jwt encode(Map<String, Object> 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<String, Object> payLoad, KeyInfo keyInfo) {
Expand Down Expand Up @@ -160,46 +157,21 @@ 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<String, Object> payLoad, JWSSigner signature) {
this.header = header;
this.signature = signature;
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);
}
Expand Down
6 changes: 6 additions & 0 deletions server/src/main/java/org/cloudfoundry/identity/uaa/oauth/openid/IdToken.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -156,4 +157,9 @@ public Long getAuthTimeInSeconds() {
public String userId() {
return sub;
}

@JsonIgnore
public Map<String, Object> getClaimMap() {
return JsonUtils.convertValue(this, HashMap.class);
}
}
Loading

0 comments on commit 9e98740

Please sign in to comment.