Skip to content

Commit

Permalink
Make sure that token signature validation happens in check-token.
Browse files Browse the repository at this point in the history 
[#158577941] https://www.pivotaltracker.com/story/show/158577941

Signed-off-by: Jennifer Hamon <jhamon@pivotal.io>
  • Loading branch information
@bruce-ricard @jhamon
bruce-ricard authored and jhamon committed Jul 18, 2018
1 parent 0db7677 commit 71cf5fb
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 18 deletions.
31 changes: 16 additions & 15 deletions server/src/main/java/org/cloudfoundry/identity/uaa/util/TokenValidation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ private TokenValidation validateHeader(Jwt tokenJwt) {
}

public boolean isValid() {
return validationErrors.size() == 0;
return validationErrors.isEmpty();
}


Expand All @@ -169,20 +169,21 @@ private TokenValidation(TokenValidation source) {


public TokenValidation checkSignature(SignatureVerifier verifier) {
if(!decoded) { return this; }
try {
tokenJwt.verifySignature(verifier);
} catch (Exception ex) {
logger.debug("Invalid token (could not verify signature)", ex);
addError("Could not verify token signature.", new InvalidSignatureException(token));
if(isValid()) {
try {
tokenJwt.verifySignature(verifier);
} catch (RuntimeException ex) {
logger.debug("Invalid token (could not verify signature)", ex);
addError("Could not verify token signature.", new InvalidSignatureException(token));
}
}
return this;
}

public TokenValidation checkIssuer(String issuer) {
if(issuer == null) { return this; }

if(!decoded || !claims.containsKey(ISS)) {
if(!isValid() || !claims.containsKey(ISS)) {
addError("Token does not bear an ISS claim.");
return this;
}
Expand All @@ -194,7 +195,7 @@ public TokenValidation checkIssuer(String issuer) {
}

protected TokenValidation checkExpiry(Instant asOf) {
if(!decoded || !claims.containsKey(EXP)) {
if(!isValid() || !claims.containsKey(EXP)) {
addError("Token does not bear an EXP claim.");
return this;
}
Expand All @@ -215,7 +216,7 @@ public TokenValidation checkExpiry() {
}

protected TokenValidation checkUser(Function<String, UaaUser> getUser) {
if(!decoded || !isUserToken(claims)) {
if(!isValid() || !isUserToken(claims)) {
addError("Token is not a user token.");
return this;
}
Expand Down Expand Up @@ -322,7 +323,7 @@ protected TokenValidation checkRequiredUserGroups(Collection<String> requiredGro
}

protected TokenValidation checkClient(Function<String, ClientDetails> getClient) {
if(!decoded || !claims.containsKey(CID)) {
if(!isValid() || !claims.containsKey(CID)) {
addError("Token bears no client ID.");
return this;
}
Expand Down Expand Up @@ -366,7 +367,7 @@ protected TokenValidation checkClient(Function<String, ClientDetails> getClient)
}

public TokenValidation checkRevocationSignature(List<String> revocableSignatureList) {
if(!decoded) {
if(!isValid()) {
addError("Token does not bear a revocation hash.");
return this;
}
Expand Down Expand Up @@ -401,7 +402,7 @@ public TokenValidation checkAudience(String... clients) {
}

protected TokenValidation checkAudience(Collection<String> clients) {
if (!decoded || !claims.containsKey(AUD)) {
if (!isValid() || !claims.containsKey(AUD)) {
addError("The token does not bear an AUD claim.");
return this;
}
Expand Down Expand Up @@ -435,7 +436,7 @@ else if(audClaim == null) {
}

public TokenValidation checkRevocableTokenStore(RevocableTokenProvisioning revocableTokenProvisioning) {
if(!decoded) {
if(!isValid()) {
addError("The token could not be checked for revocation.");
return this;
}
Expand Down Expand Up @@ -485,7 +486,7 @@ private Optional<List<String>> getScopes() {
}

private Optional<List<String>> readScopesFromClaim(String claimName) {
if (!decoded || !claims.containsKey(claimName)) {
if (!isValid() || !claims.containsKey(claimName)) {
addError(
String.format("The token does not bear a %s claim.", claimName)
);
Expand Down
7 changes: 4 additions & 3 deletions server/src/test/java/org/cloudfoundry/identity/uaa/oauth/CheckTokenEndpointTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.security.oauth2.common.OAuth2AccessToken;
import org.springframework.security.oauth2.common.exceptions.InsufficientScopeException;
import org.springframework.security.oauth2.common.exceptions.InvalidScopeException;
import org.springframework.security.oauth2.common.exceptions.InvalidTokenException;
import org.springframework.security.oauth2.provider.AuthorizationRequest;
Expand Down Expand Up @@ -524,15 +525,15 @@ public void testValidateScopesSomeNotPresent() throws Exception {
}
}

@Test(expected = InvalidTokenException.class)
@Test(expected = InsufficientScopeException.class)
public void revokingScopesFromUser_invalidatesToken() throws Exception {
setAccessToken(tokenServices.createAccessToken(authentication));
user = user.authorities(UaaAuthority.NONE_AUTHORITIES);
mockUserDatabase(userId, user);
endpoint.checkToken(getAccessToken(), Collections.emptyList(), request);
}

@Test(expected = InvalidTokenException.class)
@Test(expected = InsufficientScopeException.class)
public void revokingScopesFromClient_invalidatesToken() throws Exception {
setAccessToken(tokenServices.createAccessToken(authentication));
defaultClient = new BaseClientDetails("client", "scim, cc", "write", "authorization_code, password", "scim.read, scim.write", "http://localhost:8080/uaa");
Expand All @@ -544,7 +545,7 @@ public void revokingScopesFromClient_invalidatesToken() throws Exception {
endpoint.checkToken(getAccessToken(), Collections.emptyList(), request);
}

@Test(expected = InvalidTokenException.class)
@Test(expected = InsufficientScopeException.class)
public void revokingAuthoritiesFromClients_invalidatesToken() throws Exception {
defaultClient = new BaseClientDetails("client", "scim, cc", "write,read", "authorization_code, password", "scim.write", "http://localhost:8080/uaa");
clientDetailsStore = Collections.singletonMap(
Expand Down

0 comments on commit 71cf5fb

Please sign in to comment.