[KEYCLOAK-7365] - No need to check roles when refreshing tokens

This commit is contained in:
Pedro Igor 2018-11-28 21:22:37 -02:00
parent 1b37394276
commit 4355c89b9d
2 changed files with 21 additions and 31 deletions

View file

@ -184,7 +184,6 @@ public class TokenManager {
// recreate token.
AccessToken newToken = createClientAccessToken(session, realm, client, user, userSession, clientSessionCtx);
verifyAccess(oldToken, newToken);
return new TokenValidation(user, userSession, clientSessionCtx, newToken);
}
@ -539,33 +538,6 @@ public class TokenManager {
return true;
}
// TODO: Remove this check entirely? It should be sufficient to check granted consents (client scopes) during refresh token
private void verifyAccess(AccessToken token, AccessToken newToken) throws OAuthErrorException {
if (token.getRealmAccess() != null) {
if (newToken.getRealmAccess() == null) throw new OAuthErrorException(OAuthErrorException.INVALID_SCOPE, "User no long has permission for realm roles");
for (String roleName : token.getRealmAccess().getRoles()) {
if (!newToken.getRealmAccess().getRoles().contains(roleName)) {
throw new OAuthErrorException(OAuthErrorException.INVALID_SCOPE, "User no long has permission for realm role: " + roleName);
}
}
}
if (token.getResourceAccess() != null) {
for (Map.Entry<String, AccessToken.Access> entry : token.getResourceAccess().entrySet()) {
AccessToken.Access appAccess = newToken.getResourceAccess(entry.getKey());
if (appAccess == null && !entry.getValue().getRoles().isEmpty()) {
throw new OAuthErrorException(OAuthErrorException.INVALID_SCOPE, "User or client no longer has role permissions for client key: " + entry.getKey());
}
for (String roleName : entry.getValue().getRoles()) {
if (!appAccess.getRoles().contains(roleName)) {
throw new OAuthErrorException(OAuthErrorException.INVALID_SCOPE, "User no long has permission for client role " + roleName);
}
}
}
}
}
public AccessToken transformAccessToken(KeycloakSession session, AccessToken token,
UserSessionModel userSession, ClientSessionContext clientSessionCtx) {

View file

@ -63,6 +63,7 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsername;
@ -91,6 +92,12 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
RealmRepresentation realmRepresentation = loadJson(getClass().getResourceAsStream("/testrealm.json"), RealmRepresentation.class);
realmRepresentation.getClients().add(org.keycloak.testsuite.util.ClientBuilder.create()
.clientId("service-account-app")
.serviceAccount()
.secret("secret")
.build());
RealmBuilder realm = RealmBuilder.edit(realmRepresentation)
.testEventListener();
@ -155,7 +162,7 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
EventRepresentation tokenEvent = events.expectCodeToToken(codeId, sessionId).assertEvent();
Assert.assertNotNull(refreshTokenString);
assertNotNull(refreshTokenString);
assertEquals("bearer", tokenResponse.getTokenType());
@ -854,6 +861,17 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
events.expectRefresh(refreshToken.getId(), sessionId).user(userId).clearDetails().error(Errors.INVALID_TOKEN).assertEvent();
}
@Test
public void refreshTokenServiceAccount() throws Exception {
OAuthClient.AccessTokenResponse response = oauth.clientId("service-account-app").doClientCredentialsGrantAccessTokenRequest("secret");
assertNotNull(response.getRefreshToken());
response = oauth.doRefreshTokenRequest(response.getRefreshToken(), "secret");
assertNotNull(response.getRefreshToken());
}
protected Response executeRefreshToken(WebTarget refreshTarget, String refreshToken) {
String header = BasicAuthHelper.createHeader("test-app", "password");
Form form = new Form();
@ -908,7 +926,7 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
EventRepresentation tokenEvent = events.expectCodeToToken(codeId, sessionId).assertEvent();
Assert.assertNotNull(refreshTokenString);
assertNotNull(refreshTokenString);
assertEquals("bearer", tokenResponse.getTokenType());
@ -1031,7 +1049,7 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
EventRepresentation tokenEvent = events.expectCodeToToken(codeId, sessionId).assertEvent();
Assert.assertNotNull(refreshTokenString);
assertNotNull(refreshTokenString);
assertEquals("bearer", tokenResponse.getTokenType());