diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenRevocationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenRevocationEndpoint.java index 4d4dae73e08..c02d6a424c2 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenRevocationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenRevocationEndpoint.java @@ -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; @@ -38,6 +36,7 @@ import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.headers.SecurityHeadersProvider; +import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -52,7 +51,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.UserSessionCrossDCManager; import org.keycloak.services.managers.UserSessionManager; import org.keycloak.util.TokenUtil; @@ -113,8 +111,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()); @@ -245,26 +246,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() { diff --git a/services/src/main/java/org/keycloak/services/managers/UserSessionManager.java b/services/src/main/java/org/keycloak/services/managers/UserSessionManager.java index 280b5a7a11b..29cfae63e31 100644 --- a/services/src/main/java/org/keycloak/services/managers/UserSessionManager.java +++ b/services/src/main/java/org/keycloak/services/managers/UserSessionManager.java @@ -97,15 +97,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); } }); @@ -113,6 +105,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()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java index 23401d20a8d..a7776b5c4de 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java @@ -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); @@ -2888,7 +2888,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 { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java index 7de341ac2e5..a49a732dc35 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/AbstractClientPoliciesTest.java @@ -661,7 +661,7 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest { events.expect(EventType.INTROSPECT_TOKEN).client(clientId).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, clientSecret); - doTokenRevoke(refreshResponse.getRefreshToken(), clientId, clientSecret, userId, false); + doTokenRevoke(refreshResponse.getRefreshToken(), clientId, clientSecret, userId, sessionId, false); } protected void failLoginByNotFollowingPKCE(String clientId) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenRevocationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenRevocationTest.java index 6d7e465ebe2..f6ff1946995 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenRevocationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/TokenRevocationTest.java @@ -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; @@ -71,6 +74,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 Yoshiyuki Tabata @@ -282,6 +286,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();