mirror of
https://github.com/keycloak/keycloak.git
synced 2026-01-09 23:12:06 -03:30
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> (cherry picked from commit 3fca2f3b7f132bb775a2158e978a3a546b887d7c)
This commit is contained in:
parent
60bf57cd13
commit
9f5b60cdc3
@ -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() {
|
||||
|
||||
@ -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());
|
||||
|
||||
@ -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 {
|
||||
|
||||
@ -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) {
|
||||
|
||||
@ -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 <a href="mailto:yoshiyuki.tabata.jy@hitachi.com">Yoshiyuki Tabata</a>
|
||||
@ -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();
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user