External-Mirrors/keycloak
2
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-10 15:32:05 -03:30
Code Issues Packages Projects Releases Wiki Activity

More eagerly clear references to client sessions from the user session (#39652)

Browse Source 
Closes #39651

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
This commit is contained in:
Alexander Schwartz 2025-05-13 11:35:47 +02:00 committed by GitHub
parent 27b8a4ffcf
commit 32a8c124be
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 25 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java
View File

@ -94,8 +94,13 @@ public class UserSessionAdapter<T extends SessionRefreshStore & UserSessionProvi
final AuthenticatedClientSessionModel clientSession = provider.getClientSession(this, client, value.toString(), offline);
if (clientSession != null) {
result.put(key, clientSession);
} else {
// Either the client session has expired, or it hasn't been added by a concurrently running login yet.
// So it is unsafe to clear it, so we need to keep it for now. Otherwise, the test ConcurrentLoginTest.concurrentLoginSingleUser will fail.
// removedClientUUIDS.add(key);
}
} else {
// client does no longer exist
removedClientUUIDS.add(key);
}
});
@ -118,6 +123,8 @@ public class UserSessionAdapter<T extends SessionRefreshStore & UserSessionProvi
ClientModel client = realm.getClientById(clientUUID);
if (client != null) {
// Might return null either the client session has expired, or it hasn't been added by a concurrently running login yet.
// So it is unsafe to clear it, so we need to keep it for now. Otherwise, the test ConcurrentLoginTest.concurrentLoginSingleUser will fail.
return provider.getClientSession(this, client, clientSessionId, offline);
}
@ -125,32 +132,12 @@ public class UserSessionAdapter<T extends SessionRefreshStore & UserSessionProvi
return null;
}
private static final int MINIMUM_INACTIVE_CLIENT_SESSIONS_TO_CLEANUP = 5;
@Override
public void removeAuthenticatedClientSessions(Collection<String> removedClientUUIDS) {
if (removedClientUUIDS == null || removedClientUUIDS.isEmpty()) {
return;
}
// Performance: do not remove the clientUUIDs from the user session until there is enough of them;
// an invalid session is handled as nonexistent in UserSessionAdapter.getAuthenticatedClientSessions()
if (removedClientUUIDS.size() >= MINIMUM_INACTIVE_CLIENT_SESSIONS_TO_CLEANUP) {
// Update user session
UserSessionUpdateTask task = new UserSessionUpdateTask() {
@Override
public void runUpdate(UserSessionEntity entity) {
removedClientUUIDS.forEach(entity.getAuthenticatedClientSessions()::remove);
}
@Override
public boolean isOffline() {
return offline;
}
};
update(task);
}
// do not iterate the removedClientUUIDS and remove the clientSession directly as the addTask can manipulate
// the collection being iterated, and that can lead to unpredictable behaviour (e.g. NPE)
List<UUID> clientSessionUuids = removedClientUUIDS.stream()
@ -158,6 +145,20 @@ public class UserSessionAdapter<T extends SessionRefreshStore & UserSessionProvi
.filter(Objects::nonNull)
.collect(Collectors.toList());
// Update user session
UserSessionUpdateTask task = new UserSessionUpdateTask() {
@Override
public void runUpdate(UserSessionEntity entity) {
removedClientUUIDS.forEach(entity.getAuthenticatedClientSessions()::remove);
}
@Override
public boolean isOffline() {
return offline;
}
};
update(task);
clientSessionUuids.forEach(clientSessionId -> this.clientSessionUpdateTx.addTask(clientSessionId, Tasks.removeSync(offline)));
}

6
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java
View File

@ -1527,7 +1527,8 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
String code = oauth.parseLoginResponse().getCode();
AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code);
assertTrue("Invalid ExpiresIn", 0 < tokenResponse.getRefreshExpiresIn() && tokenResponse.getRefreshExpiresIn() <= 7200);
assertEquals(200, tokenResponse.getStatusCode());
assertTrue("Invalid ExpiresIn: " + tokenResponse.getRefreshExpiresIn(), 0 < tokenResponse.getRefreshExpiresIn() && tokenResponse.getRefreshExpiresIn() <= 7200);
String clientSessionId = getClientSessionUuid(sessionId, loginEvent.getClientId());
assertEquals(2, checkIfUserAndClientSessionExist(sessionId, loginEvent.getClientId(), clientSessionId));
@ -1551,7 +1552,8 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
sessionId = loginEvent.getSessionId();
code = oauth.parseLoginResponse().getCode();
tokenResponse = oauth.doAccessTokenRequest(code);
assertTrue("Invalid ExpiresIn", 0 < tokenResponse.getRefreshExpiresIn() && tokenResponse.getRefreshExpiresIn() <= 3000);
assertEquals(200, tokenResponse.getStatusCode());
assertTrue("Invalid ExpiresIn: " + tokenResponse.getRefreshExpiresIn(), 0 < tokenResponse.getRefreshExpiresIn() && tokenResponse.getRefreshExpiresIn() <= 3000);
events.expectCodeToToken(loginEvent.getDetails().get(Details.CODE_ID), sessionId).assertEvent();
clientSessionId = getClientSessionUuid(sessionId, loginEvent.getClientId());