From 0d9e6d087eeeedab0017ca40040c4c09774733eb Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Wed, 26 Aug 2026 14:31:06 +0200 Subject: [PATCH 1/2] Persist client session note removals with persistent user sessions JpaChangesPerformer replayed update tasks against a synthetic notes map that only forwarded get/put (client sessions) or get/put/remove/clear (user sessions) to the persisted model, so AuthenticatedClientSessionModel#removeNote and restartClientSession() never reached the database. Both merges now run the tasks the map directly. Fixes #52038 Signed-off-by: Thomas Darimont Signed-off-by: Alexander Schwartz Signed-off-by: Alexander Schwartz Co-authored-by: Alexander Schwartz Co-authored-by: Alexander Schwartz Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> (cherry picked from commit b333c0fadf9ac5bf919cfbcb5a9a6bf37fc7493b) --- .../infinispan/UserSessionAdapter.java | 4 +- .../changes/JpaChangesPerformer.java | 55 +---- ...tentAuthenticatedClientSessionAdapter.java | 5 +- .../session/SessionNotePersistenceTest.java | 228 ++++++++++++++++++ 4 files changed, 241 insertions(+), 51 deletions(-) create mode 100644 tests/base/src/test/java/org/keycloak/tests/session/SessionNotePersistenceTest.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java index 4c901448490b..9cd4a62ec5f1 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java @@ -270,9 +270,7 @@ public void setNote(String name, String value) { @Override public void runUpdate(UserSessionEntity entity) { if (value == null) { - if (entity.getNotes().containsKey(name)) { - removeNote(name); - } + entity.getNotes().remove(name); return; } entity.getNotes().put(name, value); diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/JpaChangesPerformer.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/JpaChangesPerformer.java index e9318bea1d4a..94870176fac2 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/JpaChangesPerformer.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/JpaChangesPerformer.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -203,19 +202,7 @@ public String getId() { AuthenticatedClientSessionEntity authenticatedClientSessionEntity = new AuthenticatedClientSessionEntity() { @Override public Map getNotes() { - return new HashMap<>() { - @Override - public String get(Object key) { - return clientSessionModel.getNotes().get(key); - } - - @Override - public String put(String key, String value) { - String oldValue = clientSessionModel.getNotes().get(key); - clientSessionModel.setNote(key, value); - return oldValue; - } - }; + return clientSessionModel.getNotes(); } @Override @@ -284,9 +271,9 @@ public String getAction() { } @Override - public void setNotes(Map notes) { - clientSessionModel.getNotes().keySet().forEach(clientSessionModel::removeNote); - notes.forEach(clientSessionModel::setNote); + public void setNotes(Map newNotes) { + clientSessionModel.getNotes().clear(); + clientSessionModel.getNotes().putAll(newNotes); } @Override @@ -605,32 +592,7 @@ private static void mergeUserSession(KeycloakSessio UserSessionEntity userSessionEntity = new UserSessionEntity(userSessionModel.getId()) { @Override public Map getNotes() { - return new HashMap<>() { - - @Override - public String get(Object key) { - return userSessionModel.getNotes().get(key); - } - - @Override - public String put(String key, String value) { - String oldValue = userSessionModel.getNotes().get(key); - userSessionModel.setNote(key, value); - return oldValue; - } - - @Override - public String remove(Object key) { - String oldValue = userSessionModel.getNotes().get(key); - userSessionModel.removeNote(key.toString()); - return oldValue; - } - - @Override - public void clear() { - userSessionModel.getNotes().clear(); - } - }; + return userSessionModel.getNotes(); } @Override @@ -719,9 +681,9 @@ public int getLastSessionRefresh() { } @Override - public void setNotes(Map notes) { - userSessionModel.getNotes().keySet().forEach(userSessionModel::removeNote); - notes.forEach(userSessionModel::setNote); + public void setNotes(Map newNotes) { + userSessionModel.getNotes().clear(); + userSessionModel.getNotes().putAll(newNotes); } @Override @@ -762,4 +724,5 @@ public SessionEntityWrapper mergeRemoteEntityWithLocalEntity(SessionEntityWrappe }); userSessionModel.getUpdatedModel(); } + } diff --git a/model/storage-private/src/main/java/org/keycloak/models/session/PersistentAuthenticatedClientSessionAdapter.java b/model/storage-private/src/main/java/org/keycloak/models/session/PersistentAuthenticatedClientSessionAdapter.java index 71ac4fc95a8b..163cfe64be62 100644 --- a/model/storage-private/src/main/java/org/keycloak/models/session/PersistentAuthenticatedClientSessionAdapter.java +++ b/model/storage-private/src/main/java/org/keycloak/models/session/PersistentAuthenticatedClientSessionAdapter.java @@ -18,7 +18,6 @@ package org.keycloak.models.session; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -240,7 +239,9 @@ public void removeNote(String name) { @Override public Map getNotes() { PersistentClientSessionData entity = getData(); - if (entity.getNotes() == null || entity.getNotes().isEmpty()) return Collections.emptyMap(); + if (entity.getNotes() == null) { + entity.setNotes(new HashMap<>()); + } return entity.getNotes(); } diff --git a/tests/base/src/test/java/org/keycloak/tests/session/SessionNotePersistenceTest.java b/tests/base/src/test/java/org/keycloak/tests/session/SessionNotePersistenceTest.java new file mode 100644 index 000000000000..cb1f43a05102 --- /dev/null +++ b/tests/base/src/test/java/org/keycloak/tests/session/SessionNotePersistenceTest.java @@ -0,0 +1,228 @@ +package org.keycloak.tests.session; + +import java.util.Map; + +import org.keycloak.common.util.MultiSiteUtils; +import org.keycloak.infinispan.util.InfinispanUtils; +import org.keycloak.models.AuthenticatedClientSessionModel; +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.UserSessionModel; +import org.keycloak.models.session.UserSessionPersisterProvider; +import org.keycloak.testframework.annotations.InjectRealm; +import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.realm.ManagedRealm; +import org.keycloak.testframework.realm.RealmConfig; +import org.keycloak.testframework.realm.RealmConfigBuilder; +import org.keycloak.testframework.remote.runonserver.InjectRunOnServer; +import org.keycloak.testframework.remote.runonserver.RunOnServerClient; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that note removals on persistent user/client sessions reach the database and not only the cached entity. + *

+ * Each step runs in its own server request (and therefore its own transaction), so that removals are replayed by + * {@code JpaChangesPerformer} as an update of an already persisted session. The persisted state is read back through + * {@link UserSessionPersisterProvider}, bypassing the session cache. + */ +@KeycloakIntegrationTest +public class SessionNotePersistenceTest { + + private static final String USERNAME = "user1"; + private static final String CLIENT_ID = "test-app"; + + @InjectRealm(config = SessionNotePersistenceRealmConfig.class) + ManagedRealm managedRealm; + + @InjectRunOnServer + RunOnServerClient runOnServer; + + @BeforeEach + public void assumePersistentUserSessionsWithEmbeddedCaches() { + boolean supported = runOnServer.fetch(session -> MultiSiteUtils.isPersistentSessionsEnabled() && InfinispanUtils.isEmbeddedInfinispan(), Boolean.class); + Assumptions.assumeTrue(supported, "Requires persistent user sessions with embedded Infinispan caches"); + } + + @Test + public void testClientSessionNoteRemovalIsPersisted() { + final String realmName = managedRealm.getName(); + final String noteName = "note-to-remove"; + final String userSessionId = createUserSessionWithClientSessionNote(realmName, noteName, "value"); + + // sanity check: setting the note is persisted + runOnServer.run(session -> assertEquals("value", loadClientSessionFromDatabase(session, realmName, userSessionId).getNote(noteName))); + + runOnServer.run(session -> { + AuthenticatedClientSessionModel clientSession = loadClientSessionFromProvider(session, realmName, userSessionId); + clientSession.removeNote(noteName); + assertNull(clientSession.getNote(noteName), "note must be gone from the in-memory session"); + }); + + runOnServer.run(session -> assertNull(loadClientSessionFromDatabase(session, realmName, userSessionId).getNote(noteName), + "removed client session note must not survive in the database")); + } + + @Test + public void testClientSessionRestartClearsPersistedNotes() { + final String realmName = managedRealm.getName(); + final String noteName = "note-before-restart"; + final String userSessionId = createUserSessionWithClientSessionNote(realmName, noteName, "value"); + + runOnServer.run(session -> assertEquals("value", loadClientSessionFromDatabase(session, realmName, userSessionId).getNote(noteName))); + + runOnServer.run(session -> { + AuthenticatedClientSessionModel clientSession = loadClientSessionFromProvider(session, realmName, userSessionId); + clientSession.restartClientSession(); + assertNull(clientSession.getNote(noteName), "note must be gone from the in-memory session"); + assertNotNull(clientSession.getNote(AuthenticatedClientSessionModel.STARTED_AT_NOTE)); + }); + + runOnServer.run(session -> { + AuthenticatedClientSessionModel persisted = loadClientSessionFromDatabase(session, realmName, userSessionId); + assertNotNull(persisted.getNote(AuthenticatedClientSessionModel.STARTED_AT_NOTE), "restart writes the started-at note"); + assertNull(persisted.getNote(noteName), "notes of the previous authentication must not survive restartClientSession() in the database"); + }); + } + + @Test + public void testUserSessionNoteRemovalIsPersisted() { + final String realmName = managedRealm.getName(); + final String noteName = "note-to-remove"; + final String userSessionId = createUserSessionWithNote(realmName, noteName, "value"); + + runOnServer.run(session -> assertEquals("value", loadUserSessionFromDatabase(session, realmName, userSessionId).getNote(noteName))); + + runOnServer.run(session -> { + RealmModel realm = setRealmContext(session, realmName); + UserSessionModel userSession = session.sessions().getUserSession(realm, userSessionId); + userSession.removeNote(noteName); + assertNull(userSession.getNote(noteName), "note must be gone from the in-memory session"); + }); + + runOnServer.run(session -> assertNull(loadUserSessionFromDatabase(session, realmName, userSessionId).getNote(noteName), + "removed user session note must not survive in the database")); + } + + @Test + public void testUserSessionRestartClearsPersistedNotes() { + // Regression guard: UserSessionAdapter#restartSession clears the notes via Map#clear on the synthetic entity used by + // JpaChangesPerformer#mergeUserSession, which delegates to the live notes map of PersistentUserSessionAdapter. + final String realmName = managedRealm.getName(); + final String noteName = "note-before-restart"; + final String userSessionId = createUserSessionWithNote(realmName, noteName, "value"); + + runOnServer.run(session -> assertEquals("value", loadUserSessionFromDatabase(session, realmName, userSessionId).getNote(noteName))); + + runOnServer.run(session -> { + RealmModel realm = setRealmContext(session, realmName); + UserSessionModel userSession = session.sessions().getUserSession(realm, userSessionId); + UserModel user = session.users().getUserByUsername(realm, USERNAME); + userSession.restartSession(realm, user, USERNAME, "127.0.0.2", "form", false, null, null); + assertNull(userSession.getNote(noteName), "note must be gone from the in-memory session"); + }); + + runOnServer.run(session -> { + UserSessionModel persisted = loadUserSessionFromDatabase(session, realmName, userSessionId); + assertEquals("127.0.0.2", persisted.getIpAddress(), "restart is persisted"); + Map notes = persisted.getNotes(); + assertTrue(notes == null || notes.isEmpty(), "notes of the previous authentication must not survive restartSession() in the database, but got: " + notes); + }); + } + + @Test + public void testUserSessionNoteSetToNullIsRemovedFromDatabase() { + // Regression guard: UserSessionAdapter#setNote(name, null) must remove directly from the replay entity so the + // removal is applied both to the cached session and to the persisted session during task replay. + final String realmName = managedRealm.getName(); + final String noteName = "note-to-null"; + final String userSessionId = createUserSessionWithNote(realmName, noteName, "value"); + + runOnServer.run(session -> assertEquals("value", loadUserSessionFromDatabase(session, realmName, userSessionId).getNote(noteName))); + + runOnServer.run(session -> { + RealmModel realm = setRealmContext(session, realmName); + UserSessionModel userSession = session.sessions().getUserSession(realm, userSessionId); + userSession.setNote(noteName, null); + assertNull(userSession.getNote(noteName), "note must be gone from the in-memory session"); + }); + + runOnServer.run(session -> assertNull(loadUserSessionFromDatabase(session, realmName, userSessionId).getNote(noteName), + "user session note set to null must be removed from the database")); + } + + private String createUserSessionWithNote(String realmName, String noteName, String noteValue) { + return runOnServer.fetch(session -> { + RealmModel realm = setRealmContext(session, realmName); + UserSessionModel userSession = createUserSession(session, realm); + userSession.setNote(noteName, noteValue); + return userSession.getId(); + }, String.class); + } + + private String createUserSessionWithClientSessionNote(String realmName, String noteName, String noteValue) { + return runOnServer.fetch(session -> { + RealmModel realm = setRealmContext(session, realmName); + UserSessionModel userSession = createUserSession(session, realm); + AuthenticatedClientSessionModel clientSession = session.sessions().createClientSession(realm, realm.getClientByClientId(CLIENT_ID), userSession); + clientSession.setRedirectUri("http://redirect"); + clientSession.setNote(noteName, noteValue); + return userSession.getId(); + }, String.class); + } + + private static RealmModel setRealmContext(KeycloakSession session, String realmName) { + RealmModel realm = session.realms().getRealmByName(realmName); + session.getContext().setRealm(realm); + return realm; + } + + private static UserSessionModel createUserSession(KeycloakSession session, RealmModel realm) { + UserModel user = session.users().getUserByUsername(realm, USERNAME); + return session.sessions().createUserSession(null, realm, user, USERNAME, "127.0.0.1", "form", false, null, null, UserSessionModel.SessionPersistenceState.PERSISTENT); + } + + private static AuthenticatedClientSessionModel loadClientSessionFromProvider(KeycloakSession session, String realmName, String userSessionId) { + RealmModel realm = setRealmContext(session, realmName); + UserSessionModel userSession = session.sessions().getUserSession(realm, userSessionId); + return session.sessions().getClientSession(userSession, realm.getClientByClientId(CLIENT_ID), false); + } + + /** + * Bypasses the session cache and reads the client session as stored by the {@link UserSessionPersisterProvider}. + */ + private static AuthenticatedClientSessionModel loadClientSessionFromDatabase(KeycloakSession session, String realmName, String userSessionId) { + RealmModel realm = setRealmContext(session, realmName); + ClientModel client = realm.getClientByClientId(CLIENT_ID); + UserSessionModel userSession = session.sessions().getUserSession(realm, userSessionId); + return session.getProvider(UserSessionPersisterProvider.class).loadClientSession(realm, client, userSession, false); + } + + /** + * Bypasses the session cache and reads the user session as stored by the {@link UserSessionPersisterProvider}. + */ + private static UserSessionModel loadUserSessionFromDatabase(KeycloakSession session, String realmName, String userSessionId) { + RealmModel realm = setRealmContext(session, realmName); + return session.getProvider(UserSessionPersisterProvider.class).loadUserSession(realm, userSessionId, false); + } + + private static class SessionNotePersistenceRealmConfig implements RealmConfig { + + @Override + public RealmConfigBuilder configure(RealmConfigBuilder realm) { + realm.name("client-session-notes"); + realm.addUser(USERNAME); + realm.addClient(CLIENT_ID); + return realm; + } + } +} From 59c8ed138a8ed90a02067e8ffc8dcfa0a7519eb1 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Wed, 26 Aug 2026 16:37:47 +0200 Subject: [PATCH 2/2] Adding sessions to base testsuite Signed-off-by: Alexander Schwartz --- .../src/test/java/org/keycloak/tests/suites/Base2TestSuite.java | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/base/src/test/java/org/keycloak/tests/suites/Base2TestSuite.java b/tests/base/src/test/java/org/keycloak/tests/suites/Base2TestSuite.java index c02ce62550c3..aa5112053889 100644 --- a/tests/base/src/test/java/org/keycloak/tests/suites/Base2TestSuite.java +++ b/tests/base/src/test/java/org/keycloak/tests/suites/Base2TestSuite.java @@ -23,6 +23,7 @@ "org.keycloak.tests.organization", "org.keycloak.tests.oid4vc", "org.keycloak.tests.securityprofile", + "org.keycloak.tests.sessions", "org.keycloak.tests.tracing", "org.keycloak.tests.transactions", "org.keycloak.tests.welcomepage",