When using the token revocation endpoint with refresh-token, only particular clientSession related to given refresh token should be terminated

closes #35486

Signed-off-by: mposolda <mposolda@gmail.com>
This commit is contained in:
mposolda 2024-12-06 13:04:02 +01:00 committed by Marek Posolda
parent cde8f25cc2
commit 3fca2f3b7f
5 changed files with 91 additions and 37 deletions

View File

@ -18,8 +18,6 @@
package org.keycloak.protocol.oidc.endpoints;
import java.util.Collections;
import java.util.Objects;
import java.util.stream.Collectors;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.OPTIONS;
@ -37,6 +35,7 @@ import org.keycloak.events.EventBuilder;
import org.keycloak.events.EventType;
import org.keycloak.headers.SecurityHeadersProvider;
import org.keycloak.http.HttpRequest;
import org.keycloak.models.AuthenticatedClientSessionModel;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
@ -51,7 +50,6 @@ import org.keycloak.services.clientpolicy.ClientPolicyException;
import org.keycloak.services.clientpolicy.context.TokenRevokeContext;
import org.keycloak.services.clientpolicy.context.TokenRevokeResponseContext;
import org.keycloak.services.cors.Cors;
import org.keycloak.services.managers.UserConsentManager;
import org.keycloak.services.managers.UserSessionManager;
import org.keycloak.util.TokenUtil;
@ -111,8 +109,11 @@ public class TokenRevocationEndpoint {
checkUser();
if (TokenUtil.TOKEN_TYPE_REFRESH.equals(token.getType()) || TokenUtil.TOKEN_TYPE_OFFLINE.equals(token.getType())) {
revokeClient();
revokeClientSession();
event.detail(Details.REVOKED_CLIENT, client.getClientId());
event.session(token.getSessionId());
event.detail(Details.REFRESH_TOKEN_ID, token.getId());
event.detail(Details.REFRESH_TOKEN_TYPE, token.getType());
} else {
revokeAccessToken();
event.detail(Details.TOKEN_ID, token.getId());
@ -242,26 +243,25 @@ public class TokenRevocationEndpoint {
}
}
private void revokeClient() {
UserConsentManager.revokeConsentForClient(session, realm, user, client.getId());
private void revokeClientSession() {
if (TokenUtil.TOKEN_TYPE_OFFLINE.equals(token.getType())) {
new UserSessionManager(session).revokeOfflineToken(user, client);
}
session.sessions().getUserSessionsStream(realm, user)
.map(userSession -> userSession.getAuthenticatedClientSessionByClient(client.getId()))
.filter(Objects::nonNull)
.collect(Collectors.toList()) // collect to avoid concurrent modification as dettachClientSession removes the user sessions.
.forEach(clientSession -> {
UserSessionModel userSession = clientSession.getUserSession();
UserSessionModel userSession = session.sessions().getOfflineUserSession(realm, token.getSessionId());
if (userSession != null) {
new UserSessionManager(session).removeClientFromOfflineUserSession(realm, userSession, client, user);
}
} else {
UserSessionModel userSession = session.sessions().getUserSession(realm, token.getSessionId());
if (userSession != null) {
AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(client.getId());
if (clientSession != null) {
TokenManager.dettachClientSession(clientSession);
if (userSession != null) {
// TODO: Might need optimization to prevent loading client sessions from cache in getAuthenticatedClientSessions()
if (userSession.getAuthenticatedClientSessions().isEmpty()) {
session.sessions().removeUserSession(realm, userSession);
}
// TODO: Might need optimization to prevent loading client sessions from cache in getAuthenticatedClientSessions()
if (userSession.getAuthenticatedClientSessions().isEmpty()) {
session.sessions().removeUserSession(realm, userSession);
}
});
}
}
}
}
private void revokeAccessToken() {

View File

@ -99,15 +99,7 @@ public class UserSessionManager {
AtomicBoolean anyRemoved = new AtomicBoolean(false);
kcSession.sessions().getOfflineUserSessionsStream(realm, user).collect(Collectors.toList())
.forEach(userSession -> {
AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(client.getId());
if (clientSession != null) {
if (logger.isTraceEnabled()) {
logger.tracef("Removing existing offline token for user '%s' and client '%s' .",
user.getUsername(), client.getClientId());
}
clientSession.detachFromUserSession();
checkOfflineUserSessionHasClientSessions(realm, user, userSession);
if (removeClientFromOfflineUserSession(realm, userSession, client, user)) {
anyRemoved.set(true);
}
});
@ -115,6 +107,22 @@ public class UserSessionManager {
return anyRemoved.get();
}
public boolean removeClientFromOfflineUserSession(RealmModel realm, UserSessionModel userSession, ClientModel client, UserModel user) {
AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(client.getId());
if (clientSession != null) {
if (logger.isTraceEnabled()) {
logger.tracef("Removing existing offline token for user '%s' and client '%s' .",
user.getUsername(), client.getClientId());
}
clientSession.detachFromUserSession();
checkOfflineUserSessionHasClientSessions(realm, user, userSession);
return true;
} else {
return false;
}
}
public void revokeOfflineUserSession(UserSessionModel userSession) {
if (logger.isTraceEnabled()) {
logger.tracef("Removing offline user session '%s' for user '%s' ", userSession.getId(), userSession.getLoginUsername());

View File

@ -512,7 +512,7 @@ public class CIBATest extends AbstractClientPoliciesTest {
tokenResponse = doIntrospectAccessTokenWithClientCredential(tokenRes, username);
// revoke by refresh token
EventRepresentation logoutEvent = doTokenRevokeByRefreshToken(tokenRes.getRefreshToken(), sessionId, userId, true);
EventRepresentation logoutEvent = doTokenRevokeByRefreshToken(tokenRes.getRefreshToken(), tokenRes.getSessionState(), userId, true);
} finally {
revertCIBASettings(clientResource, clientRep);
@ -614,7 +614,7 @@ public class CIBATest extends AbstractClientPoliciesTest {
tokenResponse = doIntrospectAccessTokenWithClientCredential(tokenRes, username);
// revoke by refresh token
EventRepresentation logoutEvent = doTokenRevokeByRefreshToken(tokenRes.getRefreshToken(), sessionId, userId, false);
EventRepresentation logoutEvent = doTokenRevokeByRefreshToken(tokenRes.getRefreshToken(), tokenRes.getSessionState(), userId, false);
} finally {
revertCIBASettings(clientResource, clientRep);
@ -678,7 +678,7 @@ public class CIBATest extends AbstractClientPoliciesTest {
tokenResponse = doIntrospectAccessTokenWithClientCredential(tokenRes, username);
// revoke by refresh token
EventRepresentation logoutEvent = doTokenRevokeByRefreshToken(tokenRes.getRefreshToken(), sessionId, userId, false);
EventRepresentation logoutEvent = doTokenRevokeByRefreshToken(tokenRes.getRefreshToken(), tokenRes.getSessionState(), userId, false);
} finally {
revertCIBASettings(clientResource, clientRep);
@ -2889,7 +2889,11 @@ public class CIBATest extends AbstractClientPoliciesTest {
else assertThat(tokenRes.getErrorDescription(), is(equalTo("Session not active")));
RefreshToken rt = oauth.parseRefreshToken(refreshToken);
return events.expect(EventType.REVOKE_GRANT).clearDetails().client(TEST_CLIENT_NAME).user(rt.getSubject()).assertEvent();
return events.expect(EventType.REVOKE_GRANT).clearDetails()
.client(TEST_CLIENT_NAME)
.user(rt.getSubject())
.session(sessionId)
.assertEvent();
}
private void testBackchannelAuthenticationFlow(boolean isOfflineAccess) throws Exception {

View File

@ -661,7 +661,7 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest {
events.expect(EventType.INTROSPECT_TOKEN).client(clientId).session(sessionId).user((String)null).clearDetails().assertEvent();
}
protected void doTokenRevoke(String refreshToken, String clientId, String clientSecret, String userId, boolean isOfflineAccess) throws IOException {
protected void doTokenRevoke(String refreshToken, String clientId, String clientSecret, String userId, String sessionId, boolean isOfflineAccess) throws IOException {
oauth.clientId(clientId);
oauth.doTokenRevoke(refreshToken, "refresh_token", clientSecret);
@ -672,7 +672,11 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest {
if (isOfflineAccess) assertEquals("Offline user session not found", tokenRes.getErrorDescription());
else assertEquals("Session not active", tokenRes.getErrorDescription());
events.expect(EventType.REVOKE_GRANT).clearDetails().client(clientId).user(userId).assertEvent();
events.expect(EventType.REVOKE_GRANT).clearDetails()
.client(clientId)
.user(userId)
.session(sessionId)
.assertEvent();
}
// Client CRUD operation by Admin REST API primitives
@ -1532,7 +1536,7 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest {
doIntrospectAccessToken(refreshResponse, userName, clientId, sessionId, clientSecret);
doTokenRevoke(refreshResponse.getRefreshToken(), clientId, clientSecret, userId, false);
doTokenRevoke(refreshResponse.getRefreshToken(), clientId, clientSecret, userId, sessionId, false);
}
protected void failLoginByNotFollowingPKCE(String clientId) {

View File

@ -22,6 +22,7 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.keycloak.testsuite.AssertEvents.isUUID;
import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
import java.io.IOException;
@ -54,6 +55,8 @@ 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.events.Details;
import org.keycloak.events.EventType;
import org.keycloak.representations.idm.OAuth2ErrorRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserSessionRepresentation;
@ -72,6 +75,7 @@ import org.keycloak.testsuite.util.RealmBuilder;
import org.keycloak.testsuite.util.UserInfoClientUtil;
import org.keycloak.testsuite.util.InfinispanTestTimeServiceRule;
import org.keycloak.util.JsonSerialization;
import org.keycloak.util.TokenUtil;
/**
* @author <a href="mailto:yoshiyuki.tabata.jy@hitachi.com">Yoshiyuki Tabata</a>
@ -283,6 +287,40 @@ public class TokenRevocationTest extends AbstractKeycloakTest {
assertEquals(OAuthErrorException.INVALID_REQUEST, errorRep.getError());
}
@Test
public void testRevokeSingleNormalSession() throws Exception {
testRevokeSingleSession(TokenUtil.TOKEN_TYPE_REFRESH);
}
@Test
public void testRevokeSingleOfflineSession() throws Exception {
oauth.scope(OAuth2Constants.OFFLINE_ACCESS);
testRevokeSingleSession(TokenUtil.TOKEN_TYPE_OFFLINE);
}
private void testRevokeSingleSession(String expectedTokenType) throws Exception {
oauth.clientId("test-app");
OAuthClient.AccessTokenResponse tokenResponse = oauth.doGrantAccessTokenRequest("password", "test-user@localhost",
"password");
OAuthClient.AccessTokenResponse tokenResponse2 = oauth.doGrantAccessTokenRequest("password", "test-user@localhost",
"password");
isTokenEnabled(tokenResponse, "test-app");
isTokenEnabled(tokenResponse2, "test-app");
CloseableHttpResponse response = oauth.doTokenRevoke(tokenResponse.getRefreshToken(), "refresh_token", "password");
assertThat(response, Matchers.statusCodeIsHC(Status.OK));
events.expect(EventType.REVOKE_GRANT)
.session(tokenResponse.getSessionState())
.detail(Details.REFRESH_TOKEN_ID, isUUID())
.detail(Details.REFRESH_TOKEN_TYPE, expectedTokenType)
.client("test-app")
.assertEvent(true);
isTokenDisabled(tokenResponse, "test-app");
isTokenEnabled(tokenResponse2, "test-app");
}
private AccessTokenResponse login(String clientId, String username, String password) {
oauth.clientId(clientId);
oauth.openLoginForm();