From 2e5051d4aa5457566f72043461667bc7007042b1 Mon Sep 17 00:00:00 2001 From: Jennifer Hamon Date: Wed, 18 Jul 2018 12:20:50 -0700 Subject: [PATCH] Fix tests after granted_scopes refactor [#158577941] https://www.pivotaltracker.com/story/show/158577941 Signed-off-by: Bruce Ricard --- .../identity/uaa/oauth/UaaTokenServices.java | 2 + .../identity/uaa/util/TokenValidation.java | 76 +++++++++++++++--- .../uaa/util/TokenValidationTest.java | 80 ++++++++++++------- 3 files changed, 120 insertions(+), 38 deletions(-) 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 e54573a30d7..5401241ba2e 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 @@ -52,6 +52,7 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.jwt.crypto.sign.SignatureVerifier; import org.springframework.security.oauth2.common.DefaultExpiringOAuth2RefreshToken; import org.springframework.security.oauth2.common.DefaultOAuth2RefreshToken; import org.springframework.security.oauth2.common.ExpiringOAuth2RefreshToken; @@ -1065,6 +1066,7 @@ protected TokenValidation validateToken(String token, boolean isAccessToken) { token = revocableToken.getValue(); } + TokenValidation tokenValidation = isAccessToken ? buildAccessTokenValidator(token) : buildRefreshTokenValidator(token); tokenValidation diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/util/TokenValidation.java b/server/src/main/java/org/cloudfoundry/identity/uaa/util/TokenValidation.java index ddf68f5b0fb..66573a59dbd 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/util/TokenValidation.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/util/TokenValidation.java @@ -114,20 +114,58 @@ private TokenValidation(String token, boolean isAccessToken) { this.claims = new HashMap<>(); } - if (tokenJwt != null) { - validateHeader(tokenJwt); + Optional signatureVerifier = + fetchSignatureVerifierFromToken(tokenJwt); + + signatureVerifier.ifPresent(this::validateHeader); + + this.decoded = isValid(); + } + + private TokenValidation(String token, boolean isAccessToken, SignatureVerifier signatureVerifier) { + this.token = token; + this.isAccessToken = isAccessToken; + + Jwt tokenJwt; + try { + tokenJwt = JwtHelper.decode(token); + } catch (Exception ex) { + tokenJwt = null; + validationErrors.add(new InvalidTokenException("Invalid token (could not decode): " + token, ex)); } + this.tokenJwt = tokenJwt; + + String tokenJwtClaims; + if(tokenJwt != null && StringUtils.hasText(tokenJwtClaims = tokenJwt.getClaims())) { + Map claims; + try { + claims = JsonUtils.readValue(tokenJwtClaims, new TypeReference>() {}); + } + catch (JsonUtils.JsonUtilException ex) { + claims = null; + validationErrors.add(new InvalidTokenException("Invalid token (cannot read token claims): " + token, ex)); + } + this.claims = claims; + } else { + this.claims = new HashMap<>(); + } + + validateHeader(signatureVerifier); this.decoded = isValid(); } - private TokenValidation validateHeader(Jwt tokenJwt) { + private Optional fetchSignatureVerifierFromToken(Jwt tokenJwt) { + if (tokenJwt == null) { + return Optional.empty(); + } + String kid = tokenJwt.getHeader().getKid(); if (kid == null) { validationErrors.add( new InvalidTokenException("kid claim not found in JWT token header") ); - return this; + return Optional.empty(); } KeyInfo signingKey = KeyInfo.getKey(kid); @@ -137,10 +175,15 @@ private TokenValidation validateHeader(Jwt tokenJwt) { "Token header claim [kid] references unknown signing key : [%s]", kid )) ); - return this; + return Optional.empty(); } SignatureVerifier signatureVerifier = signingKey.getVerifier(); + + return Optional.of(signatureVerifier); + } + + private TokenValidation validateHeader(SignatureVerifier signatureVerifier) { return checkSignature(signatureVerifier); } @@ -167,7 +210,7 @@ private TokenValidation(TokenValidation source) { this.scopes = source.scopes; } - + //TODO: make private public TokenValidation checkSignature(SignatureVerifier verifier) { if(isValid()) { try { @@ -272,7 +315,8 @@ protected TokenValidation checkScopesWithin(String... scopes) { } protected TokenValidation checkScopesWithin(Collection scopes) { - getScopes().ifPresent(tokenScopes -> { + Optional> scopesGot = getScopes(); + scopesGot.ifPresent(tokenScopes -> { Set scopePatterns = UaaStringUtils.constructWildcards(scopes); List missingScopes = tokenScopes.stream().filter(s -> !scopePatterns.stream().anyMatch(p -> p.matcher(s).matches())).collect(Collectors.toList()); if(!missingScopes.isEmpty()) { @@ -500,9 +544,11 @@ private Optional> readScopesFromClaim(String claimName) { } try { - return scopes = Optional.of(((List) scopeClaim).stream() + List scopeList = ((List) scopeClaim).stream() .map(s -> (String) s) - .collect(Collectors.toList())); + .collect(Collectors.toList()); + scopes = Optional.of(scopeList); + return scopes; } catch (ClassCastException ex) { addError("The token's scope claim is invalid or unparseable.", ex); return scopes = Optional.empty(); @@ -559,4 +605,16 @@ public UaaUser getUserDetails(UaaUserDatabase userDatabase) { } return null; } + + + public static TokenValidation buildAccessTokenValidatorForTesting(String tokenJwtValue, + SignatureVerifier signatureVerifier) { + return new TokenValidation(tokenJwtValue, true, signatureVerifier); + } + + public static TokenValidation buildRefreshTokenValidatorForTesting(String tokenJwtValue, + SignatureVerifier signatureVerifier) { + return new TokenValidation(tokenJwtValue, false, signatureVerifier); + } + } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/util/TokenValidationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/util/TokenValidationTest.java index a66621df3d7..0a7acbb2519 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/util/TokenValidationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/util/TokenValidationTest.java @@ -56,7 +56,9 @@ import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.SCOPE; import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.USER_NAME; import static org.cloudfoundry.identity.uaa.util.TokenValidation.buildAccessTokenValidator; +import static org.cloudfoundry.identity.uaa.util.TokenValidation.buildAccessTokenValidatorForTesting; import static org.cloudfoundry.identity.uaa.util.TokenValidation.buildRefreshTokenValidator; +import static org.cloudfoundry.identity.uaa.util.TokenValidation.buildRefreshTokenValidatorForTesting; import static org.cloudfoundry.identity.uaa.util.UaaMapUtils.entry; import static org.cloudfoundry.identity.uaa.util.UaaMapUtils.map; import static org.hamcrest.CoreMatchers.equalTo; @@ -84,9 +86,21 @@ public class TokenValidationTest { + private static class TrivialSignatureVerifier implements SignatureVerifier { + @Override + public void verify(byte[] content, byte[] signature) { + } + + @Override + public String algorithm() { + return null; + } + } + public static final String CLIENT_ID = "app"; public static final String USER_ID = "a7f07bf6-e720-4652-8999-e980189cef54"; private final SignatureVerifier verifier = new MacSigner("secret"); + private final SignatureVerifier trivialVerifier = new TrivialSignatureVerifier(); private final Instant oneSecondAfterTheTokenExpires = Instant.ofEpochSecond(1458997132 + 1); private final Instant oneSecondBeforeTheTokenExpires = Instant.ofEpochSecond(1458997132 - 1); @@ -291,7 +305,7 @@ public void validate_required_groups_is_invoked() throws Exception { @Test public void required_groups_are_present() throws Exception { - TokenValidation validation = buildAccessTokenValidator(getToken()); + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier); uaaClient.addAdditionalInformation(REQUIRED_USER_GROUPS, uaaUserGroups); assertTrue(validation.checkClientAndUser(uaaClient, uaaUser).throwIfInvalid().isValid()); } @@ -299,7 +313,7 @@ public void required_groups_are_present() throws Exception { @Test public void required_groups_are_missing() throws Exception { - TokenValidation validation = buildAccessTokenValidator(getToken()); + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier); uaaUserGroups.add("group-missing-from-user"); uaaClient.addAdditionalInformation(REQUIRED_USER_GROUPS, uaaUserGroups); assertFalse(validation.checkClientAndUser(uaaClient, uaaUser).isValid()); @@ -312,8 +326,7 @@ public void required_groups_are_missing() throws Exception { @Test public void testValidateAccessToken() throws Exception { - TokenValidation validation = buildAccessTokenValidator(getToken()) - .checkSignature(verifier) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkIssuer("http://localhost:8080/uaa/oauth/token") .checkClient((clientId) -> clientDetailsService.loadClientByClientId(clientId)) .checkExpiry(oneSecondBeforeTheTokenExpires) @@ -332,7 +345,7 @@ public void testValidateAccessToken() throws Exception { public void testValidateAccessToken_givenRefreshToken() throws Exception { content.put(JTI, "8b14f193-8212-4af2-9927-e3ae903f94a6-r"); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkAccessToken(); assertThat(validation.getValidationErrors(), not(empty())); @@ -347,7 +360,7 @@ public void testValidateAccessToken_givenRefreshToken() throws Exception { public void validateAccessToken_with_dashR_in_JTI_should_fail_validation() throws Exception { content.put(JTI, "8b14f193-r-8212-4af2-9927-e3ae903f94a6"); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkAccessToken(); assertThat(validation.getValidationErrors(), empty()); @@ -358,7 +371,7 @@ public void validateAccessToken_with_dashR_in_JTI_should_fail_validation() throw public void validateAccessToken_without_jti_should_fail_validation() throws Exception { content.put(JTI, null); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkAccessToken(); assertThat(validation.getValidationErrors(), hasSize(1)); @@ -370,7 +383,9 @@ public void validateAccessToken_without_jti_should_fail_validation() throws Exce @Test public void validateToken_Without_Email_And_Username() throws Exception { - TokenValidation validation = buildAccessTokenValidator(getToken(Arrays.asList(EMAIL, USER_NAME))) + TokenValidation validation = buildAccessTokenValidatorForTesting( + getToken(Arrays.asList(EMAIL, USER_NAME)), + trivialVerifier) .checkSignature(verifier) .checkIssuer("http://localhost:8080/uaa/oauth/token") .checkClient((clientId) -> clientDetailsService.loadClientByClientId(clientId)) @@ -390,7 +405,7 @@ public void validateToken_Without_Email_And_Username() throws Exception { public void tokenSignedWithDifferentKey() throws Exception { signer = new MacSigner("some_other_key"); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkSignature(verifier); // opaque tokens should remain valid even through a signing key being removed assertFalse(validation.isValid()); @@ -406,7 +421,7 @@ public void invalidJwt() throws Exception { @Test public void tokenWithInvalidIssuer() throws Exception { - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), trivialVerifier) .checkIssuer("http://wrong.issuer/"); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InvalidTokenException.class))); @@ -415,7 +430,7 @@ public void tokenWithInvalidIssuer() throws Exception { @Test public void emptyBodyJwt() throws Exception { content = null; - TokenValidation validation = buildAccessTokenValidator(getToken()); + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), trivialVerifier); assertThat(validation.getValidationErrors(), empty()); assertTrue("Token with no claims is valid after decoding.", validation.isValid()); @@ -425,7 +440,7 @@ public void emptyBodyJwt() throws Exception { @Test public void expiredToken() { - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkExpiry(oneSecondAfterTheTokenExpires); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InvalidTokenException.class))); @@ -435,7 +450,7 @@ public void expiredToken() { public void nonExistentUser() { UaaUserDatabase userDb = new InMemoryUaaUserDatabase(Collections.emptySet()); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkUser((uid) -> userDb.retrieveUserById(uid)); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InvalidTokenException.class))); @@ -449,7 +464,7 @@ public void userHadScopeRevoked() { .withEmail("marissa@test.org") .withAuthorities(Collections.singletonList(new SimpleGrantedAuthority("a.different.scope")))); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkUser((uid) -> userDb.retrieveUserById(uid)); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InsufficientScopeException.class))); @@ -457,7 +472,7 @@ public void userHadScopeRevoked() { @Test public void tokenHasInsufficientScope() { - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkScopesWithin("a.different.scope"); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InsufficientScopeException.class))); @@ -465,7 +480,7 @@ public void tokenHasInsufficientScope() { @Test public void tokenContainsRevokedScope() { - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkScopesWithin("a.different.scope"); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InsufficientScopeException.class))); @@ -475,7 +490,7 @@ public void tokenContainsRevokedScope() { public void nonExistentClient() { InMemoryClientDetailsService clientDetailsService = new InMemoryClientDetailsService(); clientDetailsService.setClientDetailsStore(Collections.emptyMap()); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkClient((clientId) -> clientDetailsService.loadClientByClientId(clientId)); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InvalidTokenException.class))); @@ -486,7 +501,7 @@ public void clientHasScopeRevoked() { InMemoryClientDetailsService clientDetailsService = new InMemoryClientDetailsService(); clientDetailsService.setClientDetailsStore(Collections.singletonMap("app", new BaseClientDetails("app", "acme", "a.different.scope", "authorization_code", ""))); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkClient((clientId) -> clientDetailsService.loadClientByClientId(clientId)); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InsufficientScopeException.class))); @@ -494,24 +509,27 @@ public void clientHasScopeRevoked() { @Test public void clientRevocationHashChanged() { - TokenValidation validation = buildAccessTokenValidator(getToken()).checkRevocationSignature(Collections.singletonList("New-Hash")); + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier).checkRevocationSignature(Collections.singletonList("New-Hash")); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InvalidTokenException.class))); } @Test public void clientRevocationHashChanged_and_Should_Pass() { - TokenValidation validation = buildAccessTokenValidator(getToken()).checkRevocationSignature(Arrays.asList("fa1c787d", "New-Hash")); + TokenValidation validation = buildAccessTokenValidatorForTesting( + getToken(), + verifier + ).checkRevocationSignature(Arrays.asList("fa1c787d", "New-Hash")); assertTrue(validation.isValid()); - validation = buildAccessTokenValidator(getToken()).checkRevocationSignature(Arrays.asList("New-Hash", "fa1c787d")); + validation = buildAccessTokenValidatorForTesting(getToken(), verifier).checkRevocationSignature(Arrays.asList("New-Hash", "fa1c787d")); assertTrue(validation.isValid()); } @Test public void incorrectAudience() { - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkAudience("app", "somethingelse"); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InvalidTokenException.class))); @@ -519,7 +537,7 @@ public void incorrectAudience() { @Test public void emptyAudience() { - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkAudience(""); assertFalse(validation.isValid()); assertThat(validation.getValidationErrors(), hasItem(instanceOf(InvalidTokenException.class))); @@ -531,7 +549,7 @@ public void tokenIsRevoked() { when(revocableTokenProvisioning.retrieve("8b14f193-8212-4af2-9927-e3ae903f94a6", IdentityZoneHolder.get().getId())) .thenThrow(new EmptyResultDataAccessException(1)); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkRevocableTokenStore(revocableTokenProvisioning); assertFalse(validation.isValid()); @@ -546,7 +564,7 @@ public void nonRevocableToken() { content.remove("revocable"); - TokenValidation validation = buildAccessTokenValidator(getToken()) + TokenValidation validation = buildAccessTokenValidatorForTesting(getToken(), verifier) .checkRevocableTokenStore(revocableTokenProvisioning); verifyZeroInteractions(revocableTokenProvisioning); @@ -561,10 +579,12 @@ public void validateRefreshToken() { content.put(GRANTED_SCOPES, Collections.singletonList("some-granted-scope")); String refreshToken = getToken(); - assertTrue(buildRefreshTokenValidator(refreshToken).checkScopesWithin("some-granted-scope").isValid()); + assertTrue(buildRefreshTokenValidatorForTesting(refreshToken, trivialVerifier) + .checkScopesWithin("some-granted-scope").isValid()); TokenValidation tokenValidation = - buildRefreshTokenValidator(refreshToken).checkScopesWithin((Collection) content.get(SCOPE)); + buildRefreshTokenValidatorForTesting(refreshToken, trivialVerifier) + .checkScopesWithin((Collection) content.get(SCOPE)); assertFalse(tokenValidation.isValid()); assertThat(tokenValidation.getValidationErrors().get(0).getMessage(), equalTo("Some required granted_scopes are missing: some-granted-scope")); @@ -575,7 +595,8 @@ public void validateRefreshToken_ignoresScopesClaim() { String accessToken = getToken(); TokenValidation tokenValidation = - buildRefreshTokenValidator(accessToken).checkScopesWithin((Collection) content.get(SCOPE)); + buildRefreshTokenValidatorForTesting(accessToken, trivialVerifier) + .checkScopesWithin((Collection) content.get(SCOPE)); assertFalse(tokenValidation.isValid()); assertThat(tokenValidation.getValidationErrors().get(0).getMessage(), equalTo("The token does not bear a granted_scopes claim.")); @@ -590,7 +611,8 @@ public void validateAccessToken_ignoresGrantedScopesClaim() { String refreshToken = getToken(); TokenValidation tokenValidation = - buildAccessTokenValidator(refreshToken).checkScopesWithin((Collection) content.get(GRANTED_SCOPES)); + buildRefreshTokenValidatorForTesting(refreshToken, trivialVerifier) + .checkScopesWithin((Collection) content.get(GRANTED_SCOPES)); assertFalse(tokenValidation.isValid()); assertThat(tokenValidation.getValidationErrors().get(0).getMessage(), equalTo("The token does not bear a scope claim."));