KEYCLOAK-8483 Remove application from the aud claim of accessToken and refreshToken

This commit is contained in:
mposolda 2018-10-17 16:47:21 +02:00 committed by Marek Posolda
parent 6f8f8e6a28
commit c36b577566
18 changed files with 104 additions and 35 deletions

View file

@ -74,8 +74,9 @@ public class AdapterTokenVerifier {
IDToken idToken = TokenVerifier.create(idTokenString, IDToken.class).getToken();
TokenVerifier<IDToken> idTokenVerifier = TokenVerifier.createWithoutSignature(idToken);
// Always verify audience on IDToken
// Always verify audience and azp on IDToken
idTokenVerifier.audience(deployment.getResourceName());
idTokenVerifier.issuedFor(deployment.getResourceName());
idTokenVerifier.verify();
return new VerifiedTokens(accessToken, idToken);

View file

@ -153,10 +153,8 @@ public class TokenVerifier<T extends JsonWebToken> {
throw new VerificationException("No audience in the token");
}
for (String aud : audience) {
if (expectedAudience.equals(aud)) {
return true;
}
if (t.hasAudience(expectedAudience)) {
return true;
}
throw new VerificationException("Expected audience not available in the token");
@ -164,6 +162,29 @@ public class TokenVerifier<T extends JsonWebToken> {
};
public static class IssuedForCheck implements Predicate<JsonWebToken> {
private final String expectedIssuedFor;
public IssuedForCheck(String expectedIssuedFor) {
this.expectedIssuedFor = expectedIssuedFor;
}
@Override
public boolean test(JsonWebToken jsonWebToken) throws VerificationException {
if (expectedIssuedFor == null) {
throw new VerificationException("Missing expectedIssuedFor");
}
if (expectedIssuedFor.equals(jsonWebToken.getIssuedFor())) {
return true;
}
throw new VerificationException("Expected issuedFor doesn't match");
}
}
private String tokenString;
private Class<? extends T> clazz;
private PublicKey publicKey;
@ -352,6 +373,16 @@ public class TokenVerifier<T extends JsonWebToken> {
return this.replaceCheck(AudienceCheck.class, true, new AudienceCheck(expectedAudience));
}
/**
* Add check for verifying that token issuedFor (azp claim) is the expected value
*
* @param expectedIssuedFor issuedFor, which needs to be in the target token. Can't be null
* @return This token verifier
*/
public TokenVerifier<T> issuedFor(String expectedIssuedFor) {
return this.replaceCheck(IssuedForCheck.class, true, new IssuedForCheck(expectedIssuedFor));
}
public TokenVerifier<T> parse() throws VerificationException {
if (jws == null) {
if (tokenString == null) {

View file

@ -46,7 +46,7 @@ public class RefreshToken extends AccessToken {
this.issuedFor = token.issuedFor;
this.sessionState = token.sessionState;
this.nonce = token.nonce;
this.audience = token.audience;
this.audience = new String[] { token.issuer };
this.scope = token.scope;
if (token.realmAccess != null) {
realmAccess = token.realmAccess.clone();

View file

@ -258,10 +258,13 @@ public class PolicyEvaluationService {
}
AccessToken.Access realmAccess = accessToken.getRealmAccess();
if (representation.getRoleIds() != null && !representation.getRoleIds().isEmpty()) {
if (accessToken.getRealmAccess() == null) {
accessToken.setRealmAccess(new AccessToken.Access());
}
AccessToken.Access realmAccess = accessToken.getRealmAccess();
if (representation.getRoleIds() != null) {
representation.getRoleIds().forEach(roleName -> realmAccess.addRole(roleName));
representation.getRoleIds().forEach(realmAccess::addRole);
}
return new CloseableKeycloakIdentity(accessToken, keycloakSession, userSession);

View file

@ -64,7 +64,8 @@ public class KeycloakOIDCIdentityProvider extends OIDCIdentityProvider {
@Override
protected void processAccessTokenResponse(BrokeredIdentityContext context, AccessTokenResponse response) {
JsonWebToken access = validateToken(response.getToken());
// Don't verify audience on accessToken as it may not be there. It was verified on IDToken already
JsonWebToken access = validateToken(response.getToken(), true);
context.getContextData().put(VALIDATED_ACCESS_TOKEN, access);
}

View file

@ -640,7 +640,6 @@ public class TokenManager {
token.id(KeycloakModelUtils.generateId());
token.type(TokenUtil.TOKEN_TYPE_BEARER);
token.subject(user.getId());
token.audience(client.getClientId());
token.issuedNow();
token.issuedFor(client.getClientId());

View file

@ -854,6 +854,10 @@ public class TokenEndpoint {
.generateAccessToken();
responseBuilder.getAccessToken().issuedFor(client.getClientId());
if (audience != null) {
responseBuilder.getAccessToken().addAudience(audience);
}
if (requestedTokenType.equals(OAuth2Constants.REFRESH_TOKEN_TYPE)) {
responseBuilder.generateRefreshToken();
responseBuilder.getRefreshToken().issuedFor(client.getClientId());

View file

@ -439,12 +439,15 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
if (authResult != null) {
AccessToken token = authResult.getToken();
String[] audience = token.getAudience();
ClientModel clientModel = this.realmModel.getClientByClientId(audience[0]);
String issuedFor = token.getIssuedFor();
ClientModel clientModel = this.realmModel.getClientByClientId(issuedFor);
if (clientModel == null) {
return badRequest("Invalid client.");
}
if (!clientModel.isEnabled()) {
return badRequest("Client is disabled");
}
session.getContext().setClient(clientModel);

View file

@ -96,8 +96,8 @@ class MgmtPermissions implements AdminPermissionEvaluator, AdminPermissionManage
}
private void initIdentity(KeycloakSession session, AdminAuth auth) {
if (auth.getToken().hasAudience(Constants.ADMIN_CLI_CLIENT_ID)
|| auth.getToken().hasAudience(Constants.ADMIN_CONSOLE_CLIENT_ID)) {
if (Constants.ADMIN_CLI_CLIENT_ID.equals(auth.getToken().getIssuedFor())
|| Constants.ADMIN_CONSOLE_CLIENT_ID.equals(auth.getToken().getIssuedFor())) {
this.identity = new UserModelIdentity(auth.getRealm(), auth.getUser());
} else {

View file

@ -47,7 +47,7 @@ public class ClientInitiatedAccountLinkServlet extends HttpServlet {
String realm = request.getParameter("realm");
KeycloakSecurityContext session = (KeycloakSecurityContext) request.getAttribute(KeycloakSecurityContext.class.getName());
AccessToken token = session.getToken();
String clientId = token.getAudience()[0];
String clientId = token.getIssuedFor();
String nonce = UUID.randomUUID().toString();
MessageDigest md = null;
try {

View file

@ -126,7 +126,7 @@ public class LinkAndExchangeServlet extends HttpServlet {
AccessToken token = session.getToken();
String tokenString = session.getTokenString();
String clientId = token.getAudience()[0];
String clientId = token.getIssuedFor();
String linkUrl = null;
try {
AccessTokenResponse response = doTokenExchange(realm, tokenString, provider, clientId, "password");
@ -176,7 +176,7 @@ public class LinkAndExchangeServlet extends HttpServlet {
String realm = request.getParameter("realm");
KeycloakSecurityContext session = (KeycloakSecurityContext) request.getAttribute(KeycloakSecurityContext.class.getName());
AccessToken token = session.getToken();
String clientId = token.getAudience()[0];
String clientId = token.getIssuedFor();
String tokenString = session.getTokenString();
AccessTokenResponse response = doTokenExchange(realm, tokenString, provider, clientId, "password");
error = (String)response.getOtherClaims().get("error");

View file

@ -307,7 +307,7 @@ public class OIDCJwksClientRegistrationTest extends AbstractClientRegistrationTe
OAuthClient.AccessTokenResponse accessTokenResponse = doClientCredentialsGrantRequest(signedJwt);
Assert.assertEquals(200, accessTokenResponse.getStatusCode());
AccessToken accessToken = oauth.verifyToken(accessTokenResponse.getAccessToken());
Assert.assertEquals(response.getClientId(), accessToken.getAudience()[0]);
Assert.assertEquals(response.getClientId(), accessToken.getIssuedFor());
}
private void assertAuthenticateClientError(Map<String, String> generatedKeys, OIDCClientRepresentation response, String kid) throws Exception {

View file

@ -397,9 +397,6 @@ public class OIDCPairwiseClientRegistrationTest extends AbstractClientRegistrati
// its iat Claim MUST represent the time that the new ID Token is issued
Assert.assertEquals(refreshedIdToken.getIssuedAt(), refreshedRefreshToken.getIssuedAt());
// its aud Claim Value MUST be the same as in the ID Token issued when the original authentication occurred
Assert.assertArrayEquals(idToken.getAudience(), refreshedRefreshToken.getAudience());
// if the ID Token contains an auth_time Claim, its value MUST represent the time of the original authentication
// - not the time that the new ID token is issued
Assert.assertEquals(idToken.getAuthTime(), refreshedIdToken.getAuthTime());

View file

@ -143,6 +143,16 @@ public class AccessTokenTest extends AbstractKeycloakTest {
.password("password");
realm.getUsers().add(user.build());
realm.getClients().stream().filter(clientRepresentation -> {
return "test-app".equals(clientRepresentation.getClientId());
}).forEach(clientRepresentation -> {
clientRepresentation.setFullScopeAllowed(false);
});
testRealms.add(realm);
}

View file

@ -301,7 +301,7 @@ public class ClientTokenExchangeTest extends AbstractKeycloakTest {
TokenVerifier<AccessToken> verifier = TokenVerifier.create(exchangedTokenString, AccessToken.class);
AccessToken exchangedToken = verifier.parse().getToken();
Assert.assertEquals("client-exchanger", exchangedToken.getIssuedFor());
Assert.assertEquals("client-exchanger", exchangedToken.getAudience()[0]);
Assert.assertNull(exchangedToken.getAudience());
Assert.assertEquals(exchangedToken.getPreferredUsername(), "impersonated-user");
Assert.assertNull(exchangedToken.getRealmAccess());
}
@ -409,7 +409,7 @@ public class ClientTokenExchangeTest extends AbstractKeycloakTest {
TokenVerifier<AccessToken> verifier = TokenVerifier.create(exchangedTokenString, AccessToken.class);
AccessToken exchangedToken = verifier.parse().getToken();
Assert.assertEquals("direct-exchanger", exchangedToken.getIssuedFor());
Assert.assertEquals("direct-exchanger", exchangedToken.getAudience()[0]);
Assert.assertNull(exchangedToken.getAudience());
Assert.assertEquals(exchangedToken.getPreferredUsername(), "impersonated-user");
Assert.assertNull(exchangedToken.getRealmAccess());
}

View file

@ -228,7 +228,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
assertEquals(Arrays.asList("A","B"), accessToken.getOtherClaims().get("multiValued-via-script"));
// Assert audiences added through AudienceResolve mapper
Assert.assertThat(accessToken.getAudience(), arrayContainingInAnyOrder("test-app", "app", "account"));
Assert.assertThat(accessToken.getAudience(), arrayContainingInAnyOrder( "app", "account"));
// Assert allowed origins
String expectedOrigin = UriUtils.getOrigin(oauth.getRedirectUri());
@ -415,7 +415,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
List<String> roles = (List<String>) cst1.get("roles");
Assert.assertNames(roles, "offline_access", "user", "customer-user", "hardcoded", AccountRoles.VIEW_PROFILE, AccountRoles.MANAGE_ACCOUNT, AccountRoles.MANAGE_ACCOUNT_LINKS);
// Assert audience
// Assert audience - "test-app" is added due the AudienceResolveProtocolMapper
Assert.assertNames(Arrays.asList(accessToken.getAudience()), "account", "test-app");
} finally {
// Revert
@ -430,13 +430,15 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
@Test
public void testAllowedOriginsRemovedFromAccessToken() throws Exception {
public void testRolesAndAllowedOriginsRemovedFromAccessToken() throws Exception {
RealmResource realm = adminClient.realm("test");
ClientScopeRepresentation allowedOriginsScope = ApiUtil.findClientScopeByName(realm, OIDCLoginProtocolFactory.WEB_ORIGINS_SCOPE).toRepresentation();
ClientScopeRepresentation rolesScope = ApiUtil.findClientScopeByName(realm, OIDCLoginProtocolFactory.ROLES_SCOPE).toRepresentation();
// Remove 'web-origins' scope from the client
// Remove 'roles' and 'web-origins' scope from the client
ClientResource testApp = ApiUtil.findClientByClientId(realm, "test-app");
testApp.removeDefaultClientScope(allowedOriginsScope.getId());
testApp.removeDefaultClientScope(rolesScope.getId());
try {
OAuthClient.AccessTokenResponse response = browserLogin("password", "test-user@localhost", "password");
@ -445,9 +447,22 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
// Assert web origins are not in the token
Assert.assertNull(accessToken.getAllowedOrigins());
// Assert roles are not in the token
Assert.assertNull(accessToken.getRealmAccess());
Assert.assertTrue(accessToken.getResourceAccess().isEmpty());
// Assert client not in the token audience. Just in "issuedFor"
Assert.assertEquals("test-app", accessToken.getIssuedFor());
Assert.assertFalse(accessToken.hasAudience("test-app"));
// Assert IDToken still has "test-app" as an audience
IDToken idToken = oauth.verifyIDToken(response.getIdToken());
Assert.assertEquals("test-app", idToken.getIssuedFor());
Assert.assertTrue(idToken.hasAudience("test-app"));
} finally {
// Revert
testApp.addDefaultClientScope(allowedOriginsScope.getId());
testApp.addDefaultClientScope(rolesScope.getId());
}
}

View file

@ -179,6 +179,10 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
Assert.assertThat(refreshedToken.getExpiration() - token.getExpiration(), allOf(greaterThanOrEqualTo(1), lessThanOrEqualTo(10)));
Assert.assertThat(refreshedRefreshToken.getExpiration() - refreshToken.getExpiration(), allOf(greaterThanOrEqualTo(1), lessThanOrEqualTo(10)));
// "test-app" should not be an audience in the refresh token
assertEquals("test-app", refreshedRefreshToken.getIssuedFor());
Assert.assertFalse(refreshedRefreshToken.hasAudience("test-app"));
Assert.assertNotEquals(token.getId(), refreshedToken.getId());
Assert.assertNotEquals(refreshToken.getId(), refreshedRefreshToken.getId());

View file

@ -19,6 +19,7 @@ package org.keycloak.testsuite.oidc;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import javax.ws.rs.core.Response;
@ -136,8 +137,8 @@ public class AudienceTest extends AbstractOIDCScopeTest {
.user(userId)
.assertEvent();
Tokens tokens = sendTokenRequest(loginEvent, userId,"openid profile email audience-scope", "test-app");
// TODO: Frontend client itself should not be in the audiences of access token. Will be fixed in the future
assertAudiences(tokens.accessToken, "test-app", "service-client");
assertAudiences(tokens.accessToken, "service-client");
assertAudiences(tokens.idToken, "test-app");
// Revert
@ -168,8 +169,8 @@ public class AudienceTest extends AbstractOIDCScopeTest {
.user(userId)
.assertEvent();
Tokens tokens = sendTokenRequest(loginEvent, userId,"openid profile email audience-scope", "test-app");
// TODO: Frontend client itself should not be in the audiences of access token. Will be fixed in the future
assertAudiences(tokens.accessToken, "test-app", "http://host/service/ctx1", "http://host/service/ctx2");
assertAudiences(tokens.accessToken, "http://host/service/ctx1", "http://host/service/ctx2");
assertAudiences(tokens.idToken, "test-app", "http://host/service/ctx2");
// Revert
@ -192,7 +193,7 @@ public class AudienceTest extends AbstractOIDCScopeTest {
.user(userId)
.assertEvent();
Tokens tokens = sendTokenRequest(loginEvent, userId,"openid profile email", "test-app");
assertAudiences(tokens.accessToken, "test-app");
assertAudiences(tokens.accessToken);
assertAudiences(tokens.idToken, "test-app");
Assert.assertFalse(tokens.accessToken.getResourceAccess().containsKey("service-client"));
@ -215,7 +216,7 @@ public class AudienceTest extends AbstractOIDCScopeTest {
.user(userId)
.assertEvent();
tokens = sendTokenRequest(loginEvent, userId,"openid profile email service-client", "test-app");
assertAudiences(tokens.accessToken, "test-app", "service-client");
assertAudiences(tokens.accessToken, "service-client");
assertAudiences(tokens.idToken, "test-app");
Assert.assertTrue(tokens.accessToken.getResourceAccess().containsKey("service-client"));
Assert.assertNames(tokens.accessToken.getResourceAccess().get("service-client").getRoles(), "role1");
@ -228,7 +229,7 @@ public class AudienceTest extends AbstractOIDCScopeTest {
private void assertAudiences(JsonWebToken token, String... expectedAudience) {
Collection<String> audiences = Arrays.asList(token.getAudience());
Collection<String> audiences = token.getAudience() == null ? Collections.emptyList() : Arrays.asList(token.getAudience());
Collection<String> expectedAudiences = Arrays.asList(expectedAudience);
Assert.assertTrue("Not matched. expectedAudiences: " + expectedAudiences + ", audiences: " + audiences,
expectedAudiences.containsAll(audiences) && audiences.containsAll(expectedAudiences));