Fixing for token revocation checks only (#9707)

Closes #9705
This commit is contained in:
Marek Posolda 2022-02-02 15:21:44 +01:00 committed by GitHub
parent b24c7ecaae
commit d27635fb1b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 178 additions and 30 deletions

View file

@ -18,7 +18,6 @@
package org.keycloak.authorization.util;
import org.keycloak.models.KeycloakContext;
import org.keycloak.models.KeycloakSession;
import org.keycloak.representations.AccessToken;
import org.keycloak.services.managers.AppAuthManager;

View file

@ -240,18 +240,13 @@ public class TokenManager {
try {
TokenVerifier.createWithoutSignature(token)
.withChecks(NotBeforeCheck.forModel(client), TokenVerifier.IS_ACTIVE)
.withChecks(NotBeforeCheck.forModel(client), TokenVerifier.IS_ACTIVE, new TokenRevocationCheck(session))
.verify();
} catch (VerificationException e) {
logger.debugf("JWT check failed: %s", e.getMessage());
return false;
}
TokenRevocationStoreProvider revocationStore = session.getProvider(TokenRevocationStoreProvider.class);
if (revocationStore.isRevoked(token.getId())) {
return false;
}
boolean valid = false;
// Tokens without sessions are considered valid. Signature check and revocation check are sufficient checks for them
@ -1346,6 +1341,24 @@ public class TokenManager {
}
}
/**
* Check if access token was revoked with OAuth revocation endpoint
*/
public static class TokenRevocationCheck implements TokenVerifier.Predicate<AccessToken> {
private final KeycloakSession session;
public TokenRevocationCheck(KeycloakSession session) {
this.session = session;
}
@Override
public boolean test(AccessToken token) {
TokenRevocationStoreProvider revocationStore = session.getProvider(TokenRevocationStoreProvider.class);
return !revocationStore.isRevoked(token.getId());
}
}
public LogoutTokenValidationCode verifyLogoutToken(KeycloakSession session, RealmModel realm, String encodedLogoutToken) {
Optional<LogoutToken> logoutTokenOptional = toLogoutToken(encodedLogoutToken);
if (!logoutTokenOptional.isPresent()) {

View file

@ -174,7 +174,7 @@ public class UserInfoEndpoint {
cors.allowedOrigins(session, clientModel);
TokenVerifier.createWithoutSignature(token)
.withChecks(NotBeforeCheck.forModel(clientModel))
.withChecks(NotBeforeCheck.forModel(clientModel), new TokenManager.TokenRevocationCheck(session))
.verify();
} catch (VerificationException e) {
if (clientModel == null) {

View file

@ -1395,6 +1395,11 @@ public class AuthenticationManager {
verifier.audience(checkAudience);
}
// Check token revocation in case of access token
if (checkTokenType) {
verifier.withChecks(new TokenManager.TokenRevocationCheck(session));
}
String kid = verifier.getHeader().getKeyId();
String algorithm = verifier.getHeader().getAlgorithm().name();
@ -1432,7 +1437,11 @@ public class AuthenticationManager {
UserSessionModel offlineUserSession = session.sessions().getOfflineUserSession(realm, token.getSessionState());
if (isOfflineSessionValid(realm, offlineUserSession)) {
user = offlineUserSession.getUser();
return new AuthResult(user, offlineUserSession, token);
ClientModel client = realm.getClientByClientId(token.getIssuedFor());
if (!isClientValid(offlineUserSession, client, token)) {
return null;
}
return new AuthResult(user, offlineUserSession, token, client);
}
}
@ -1443,13 +1452,45 @@ public class AuthenticationManager {
session.setAttribute("state_checker", token.getOtherClaims().get("state_checker"));
return new AuthResult(user, userSession, token);
ClientModel client;
if (isCookie) {
client = null;
} else {
client = realm.getClientByClientId(token.getIssuedFor());
if (!isClientValid(userSession, client, token)) {
return null;
}
}
return new AuthResult(user, userSession, token, client);
} catch (VerificationException e) {
logger.debugf("Failed to verify identity token: %s", e.getMessage());
}
return null;
}
// Verify client and whether clientSession exists
private static boolean isClientValid(UserSessionModel userSession, ClientModel client, AccessToken token) {
if (client == null || !client.isEnabled()) {
logger.debugf("Identity token issued for unknown or disabled client '%s'", token.getIssuedFor());
return false;
}
if (token.getIssuedAt() < client.getNotBefore()) {
logger.debug("Client notBefore newer than token");
return false;
}
// User session may not exists for example during client credentials auth
if (userSession == null) return true;
AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(client.getId());
if (clientSession == null) {
logger.debugf("Client session for client '%s' not present in user session '%s'", client.getClientId(), userSession.getId());
return false;
}
return true;
}
private static boolean isUserValid(KeycloakSession session, RealmModel realm, UserModel user, AccessToken token) {
if (user == null || !user.isEnabled()) {
logger.debug("Unknown user in identity token");
@ -1473,11 +1514,13 @@ public class AuthenticationManager {
private final UserModel user;
private final UserSessionModel session;
private final AccessToken token;
private final ClientModel client;
public AuthResult(UserModel user, UserSessionModel session, AccessToken token) {
public AuthResult(UserModel user, UserSessionModel session, AccessToken token, ClientModel client) {
this.user = user;
this.session = session;
this.token = token;
this.client = client;
}
public UserSessionModel getSession() {
@ -1491,6 +1534,10 @@ public class AuthenticationManager {
public AccessToken getToken() {
return token;
}
public ClientModel getClient() {
return client;
}
}
public static void setKcActionStatus(String executedProviderId, RequiredActionContext.KcActionStatus status, AuthenticationSessionModel authSession) {

View file

@ -451,15 +451,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
if (authResult != null) {
AccessToken token = authResult.getToken();
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");
}
ClientModel clientModel = authResult.getClient();
session.getContext().setClient(clientModel);

View file

@ -179,13 +179,7 @@ public class AdminRoot {
throw new NotAuthorizedException("Bearer");
}
ClientModel client = realm.getClientByClientId(token.getIssuedFor());
if (client == null) {
throw new NotFoundException("Could not find client for authorization");
}
return new AdminAuth(realm, authResult.getToken(), authResult.getUser(), client);
return new AdminAuth(realm, authResult.getToken(), authResult.getUser(), authResult.getClient());
}
public static UriBuilder realmsUrl(UriInfo uriInfo) {

View file

@ -20,11 +20,14 @@ package org.keycloak.testsuite.admin;
import java.util.List;
import javax.ws.rs.NotAuthorizedException;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.keycloak.admin.client.Keycloak;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.common.constants.ServiceAccountConstants;
import org.keycloak.models.AdminRoles;
import org.keycloak.models.Constants;
@ -88,7 +91,9 @@ public class AdminClientTest extends AbstractKeycloakTest {
UserBuilder defaultUser = UserBuilder.create()
.id(KeycloakModelUtils.generateId())
.username("test-user@localhost");
.username("test-user@localhost")
.password("password")
.role(Constants.REALM_MANAGEMENT_CLIENT_ID, AdminRoles.REALM_ADMIN);
realm.user(defaultUser);
testRealms.add(realm.build());
@ -103,9 +108,60 @@ public class AdminClientTest extends AbstractKeycloakTest {
setTimeOffset(1000);
// Check still possible to load the realm after token expired
// Check still possible to load the realm after original token expired (admin client should automatically re-authenticate)
realm = adminClient.realm("test").toRepresentation();
Assert.assertEquals("test", realm.getRealm());
}
}
@Test
public void clientCredentialsClientDisabled() throws Exception {
try (Keycloak adminClient = AdminClientUtil.createAdminClientWithClientCredentials("test", "service-account-cl", "secret1")) {
// Check possible to load the realm
RealmRepresentation realm = adminClient.realm("test").toRepresentation();
Assert.assertEquals("test", realm.getRealm());
// Disable client and check it should not be possible to load the realms anymore
setClientEnabled("service-account-cl", false);
// Check not possible to invoke anymore
try {
realm = adminClient.realm("test").toRepresentation();
Assert.fail("Not expected to successfully get realm");
} catch (NotAuthorizedException nae) {
// Expected
}
} finally {
setClientEnabled("service-account-cl", true);
}
}
@Test
public void adminAuthClientDisabled() throws Exception {
try (Keycloak adminClient = AdminClientUtil.createAdminClient(false, "test", "test-user@localhost", "password", Constants.ADMIN_CLI_CLIENT_ID, null)) {
// Check possible to load the realm
RealmRepresentation realm = adminClient.realm("test").toRepresentation();
Assert.assertEquals("test", realm.getRealm());
// Disable client and check it should not be possible to load the realms anymore
setClientEnabled(Constants.ADMIN_CLI_CLIENT_ID, false);
// Check not possible to invoke anymore
try {
realm = adminClient.realm("test").toRepresentation();
Assert.fail("Not expected to successfully get realm");
} catch (NotAuthorizedException nae) {
// Expected
}
} finally {
setClientEnabled(Constants.ADMIN_CLI_CLIENT_ID, true);
}
}
private void setClientEnabled(String clientId, boolean enabled) {
ClientResource client = ApiUtil.findClientByClientId(adminClient.realms().realm("test"), clientId);
ClientRepresentation clientRep = client.toRepresentation();
clientRep.setEnabled(enabled);
client.update(clientRep);
}
}

View file

@ -30,10 +30,14 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import javax.ws.rs.NotAuthorizedException;
import javax.ws.rs.client.Client;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import org.apache.commons.io.output.ByteArrayOutputStream;
import org.apache.http.NameValuePair;
import org.apache.http.client.HttpClient;
import org.apache.http.client.entity.UrlEncodedFormEntity;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpPost;
@ -41,25 +45,32 @@ import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.message.BasicNameValuePair;
import org.jboss.arquillian.graphene.page.Page;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.keycloak.OAuth2Constants;
import org.keycloak.OAuthErrorException;
import org.keycloak.admin.client.Keycloak;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.broker.provider.util.SimpleHttp;
import org.keycloak.representations.account.UserRepresentation;
import org.keycloak.representations.idm.OAuth2ErrorRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserSessionRepresentation;
import org.keycloak.representations.oidc.TokenMetadataRepresentation;
import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.util.AdminClientUtil;
import org.keycloak.testsuite.util.ClientManager;
import org.keycloak.testsuite.util.Matchers;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.OAuthClient.AccessTokenResponse;
import org.keycloak.testsuite.util.RealmBuilder;
import org.keycloak.testsuite.util.UserInfoClientUtil;
import org.keycloak.util.JsonSerialization;
/**
@ -69,6 +80,9 @@ public class TokenRevocationTest extends AbstractKeycloakTest {
private RealmResource realm;
private Client userInfoClient;
private CloseableHttpClient restHttpClient;
@Rule
public AssertEvents events = new AssertEvents(this);
@ -87,10 +101,21 @@ public class TokenRevocationTest extends AbstractKeycloakTest {
}
@Before
public void clientConfiguration() {
public void beforeTest() {
// Create client configuration
realm = adminClient.realm("test");
ClientManager.realm(realm).clientId("test-app").directAccessGrant(true);
ClientManager.realm(realm).clientId("test-app-scope").directAccessGrant(true);
// Create clients
userInfoClient = AdminClientUtil.createResteasyClient();
restHttpClient = HttpClientBuilder.create().build();
}
@After
public void afterTest() throws IOException {
userInfoClient.close();
restHttpClient.close();
}
@Page
@ -270,10 +295,32 @@ public class TokenRevocationTest extends AbstractKeycloakTest {
}
private void isAccessTokenDisabled(String accessTokenString, String clientId) throws IOException {
// Test introspection endpoint not possible
String introspectionResponse = oauth.introspectAccessTokenWithClientCredential(clientId, "password",
accessTokenString);
TokenMetadataRepresentation rep = JsonSerialization.readValue(introspectionResponse, TokenMetadataRepresentation.class);
assertFalse(rep.isActive());
// Test userInfo endpoint not possible
Response response = UserInfoClientUtil.executeUserInfoRequest_getMethod(userInfoClient, accessTokenString);
assertEquals(Status.UNAUTHORIZED.getStatusCode(), response.getStatus());
// Test account REST not possible
String accountUrl = OAuthClient.AUTH_SERVER_ROOT + "/realms/test/account";
SimpleHttp accountRequest = SimpleHttp.doGet(accountUrl, restHttpClient)
.auth(accessTokenString)
.acceptJson();
assertEquals(Status.UNAUTHORIZED.getStatusCode(), accountRequest.asStatus());
// Test admin REST not possible
try (Keycloak adminClient = Keycloak.getInstance(OAuthClient.AUTH_SERVER_ROOT, "test", "test-app", accessTokenString)) {
try {
adminClient.realms().realm("test").toRepresentation();
Assert.fail("Not expected to obtain realm");
} catch (NotAuthorizedException nae) {
// Expected
}
}
}
private String doTokenRevokeWithDuplicateParams(String token, String tokenTypeHint, String clientSecret)