Skip to content

Commit

Permalink
Do not use LastSessionRefreshPersister with persistent user sessions …
Browse files Browse the repository at this point in the history 
…enabled

Closes keycloak#29144

Signed-off-by: Michal Hajas <mhajas@redhat.com>
  • Loading branch information
@mhajas
mhajas committed May 3, 2024
1 parent 64824bb commit 7fe4563
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 24 deletions.
12 changes: 4 additions & 8 deletions ...in/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ public UserSessionProvider create(KeycloakSession session) {
remoteCacheInvoker,
lastSessionRefreshStore,
offlineLastSessionRefreshStore,
persisterLastSessionRefreshStore,
keyGenerator,
cache,
offlineSessionsCache,
Expand Down Expand Up @@ -178,7 +177,9 @@ public void onEvent(ProviderEvent event) {

keyGenerator = new InfinispanKeyGenerator();
checkRemoteCaches(session);
loadPersistentSessions(factory, getMaxErrors(), getSessionsPerSegment());
if (!Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS)) {
initializeLastSessionRefreshStore(factory);
}
registerClusterListeners(session);
loadSessionsFromRemoteCaches(session);

Expand Down Expand Up @@ -234,20 +235,15 @@ private int getStalledTimeoutInSeconds(int defaultTimeout) {
return config.getInt("sessionPreloadStalledTimeoutInSeconds", defaultTimeout);
}

@Override
public void loadPersistentSessions(final KeycloakSessionFactory sessionFactory, final int maxErrors, final int sessionsPerSegment) {

public void initializeLastSessionRefreshStore(final KeycloakSessionFactory sessionFactory) {
KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() {

@Override
public void run(KeycloakSession session) {
// Initialize persister for periodically doing bulk DB updates of lastSessionRefresh timestamps of refreshed sessions
persisterLastSessionRefreshStore = new PersisterLastSessionRefreshStoreFactory().createAndInit(session, true);
}

});


}


Expand Down
5 changes: 1 addition & 4 deletions .../src/main/java/org/keycloak/models/sessions/infinispan/PersistentUserSessionProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ public class PersistentUserSessionProvider implements UserSessionProvider, Sessi

protected final CrossDCLastSessionRefreshStore lastSessionRefreshStore;
protected final CrossDCLastSessionRefreshStore offlineLastSessionRefreshStore;
protected final PersisterLastSessionRefreshStore persisterLastSessionRefreshStore;

protected final RemoteCacheInvoker remoteCacheInvoker;
protected final InfinispanKeyGenerator keyGenerator;
Expand All @@ -116,7 +115,6 @@ public PersistentUserSessionProvider(KeycloakSession session,
RemoteCacheInvoker remoteCacheInvoker,
CrossDCLastSessionRefreshStore lastSessionRefreshStore,
CrossDCLastSessionRefreshStore offlineLastSessionRefreshStore,
PersisterLastSessionRefreshStore persisterLastSessionRefreshStore,
InfinispanKeyGenerator keyGenerator,
Cache<String, SessionEntityWrapper<UserSessionEntity>> sessionCache,
Cache<String, SessionEntityWrapper<UserSessionEntity>> offlineSessionCache,
Expand Down Expand Up @@ -153,7 +151,6 @@ public PersistentUserSessionProvider(KeycloakSession session,

this.lastSessionRefreshStore = lastSessionRefreshStore;
this.offlineLastSessionRefreshStore = offlineLastSessionRefreshStore;
this.persisterLastSessionRefreshStore = persisterLastSessionRefreshStore;
this.remoteCacheInvoker = remoteCacheInvoker;
this.keyGenerator = keyGenerator;

Expand Down Expand Up @@ -182,7 +179,7 @@ public CrossDCLastSessionRefreshStore getOfflineLastSessionRefreshStore() {

@Override
public PersisterLastSessionRefreshStore getPersisterLastSessionRefreshStore() {
return persisterLastSessionRefreshStore;
throw new IllegalStateException("PersisterLastSessionRefreshStore is not supported in PersistentUserSessionProvider");
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion .../infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.keycloak.models.sessions.infinispan;

import org.keycloak.common.Profile;
import org.keycloak.models.AuthenticatedClientSessionModel;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
Expand Down Expand Up @@ -230,7 +231,7 @@ public void setLastSessionRefresh(int lastSessionRefresh) {
return;
}

if (offline) {
if (!Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS) && offline) {
// Received the message from the other DC that we should update the lastSessionRefresh in local cluster. Don't update DB in that case.
// The other DC already did.
Boolean ignoreRemoteCacheUpdate = (Boolean) session.getAttribute(CrossDCLastSessionRefreshListener.IGNORE_REMOTE_CACHE_UPDATE);
Expand Down
4 changes: 3 additions & 1 deletion server-spi-private/src/main/java/org/keycloak/models/UserSessionProviderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
public interface UserSessionProviderFactory<T extends UserSessionProvider> extends ProviderFactory<T> {

// This is supposed to prefill all userSessions and clientSessions from userSessionPersister to the userSession infinispan/memory storage
void loadPersistentSessions(KeycloakSessionFactory sessionFactory, final int maxErrors, final int sessionsPerSegment);
// This method is no longer used as we don't have offline sessions preloading anymore
@Deprecated
default void loadPersistentSessions(KeycloakSessionFactory sessionFactory, final int maxErrors, final int sessionsPerSegment) {}

}
15 changes: 9 additions & 6 deletions ...odel/src/test/java/org/keycloak/testsuite/model/session/UserSessionProviderModelTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Test;
import org.keycloak.common.Profile;
import org.keycloak.models.AuthenticatedClientSessionModel;
import org.keycloak.models.ClientModel;
import org.keycloak.models.Constants;
Expand Down Expand Up @@ -119,15 +120,16 @@ public void testMultipleSessionsRemovalInOneTransaction() {
@Test
public void testExpiredClientSessions() {
// Suspend periodic tasks to avoid race-conditions, which may cause missing updates of lastSessionRefresh times to UserSessionPersisterProvider
// skip for persistent user sessions as the periodic task is not used there
TimerProvider timer = kcSession.getProvider(TimerProvider.class);
TimerProvider.TimerTaskContext timerTaskCtx = null;
if (timer != null) {
if (timer != null && !Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS)) {
timerTaskCtx = timer.cancelTask(PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);
log.info("Cancelled periodic task " + PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);

InfinispanTestUtil.setTestingTimeService(kcSession);
}

InfinispanTestUtil.setTestingTimeService(kcSession);

try {
UserSessionModel[] origSessions = inComittedTransaction(session -> {
// create some user and client sessions
Expand Down Expand Up @@ -172,11 +174,12 @@ public void testExpiredClientSessions() {
} finally {
setTimeOffset(0);
kcSession.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent());
if (timer != null && timerTaskCtx != null) {
// Enable periodic task again, skip for persistent user sessions as the periodic task is not used there
if (timer != null && timerTaskCtx != null && !Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS)) {
timer.schedule(timerTaskCtx.getRunnable(), timerTaskCtx.getIntervalMillis(), PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);

InfinispanTestUtil.revertTimeService(kcSession);
}

InfinispanTestUtil.revertTimeService(kcSession);
}
}

Expand Down
13 changes: 9 additions & 4 deletions ...c/test/java/org/keycloak/testsuite/model/session/UserSessionProviderOfflineModelTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,10 @@ public void cleanEnvironment(KeycloakSession s) {
@Test
public void testExpired() {
// Suspend periodic tasks to avoid race-conditions, which may cause missing updates of lastSessionRefresh times to UserSessionPersisterProvider
// skip for persistent user sessions as the periodic task is not used there
TimerProvider timer = kcSession.getProvider(TimerProvider.class);
TimerProvider.TimerTaskContext timerTaskCtx = null;
if (timer != null) {
if (timer != null && !Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS)) {
timerTaskCtx = timer.cancelTask(PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);
log.info("Cancelled periodic task " + PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);
}
Expand Down Expand Up @@ -236,7 +237,8 @@ public void testExpired() {
} finally {
setTimeOffset(0);
kcSession.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent());
if (timer != null) {
// Enable periodic task again, skip for persistent user sessions as the periodic task is not used there
if (timer != null && !Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS)) {
timer.schedule(timerTaskCtx.getRunnable(), timerTaskCtx.getIntervalMillis(), PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);
}

Expand All @@ -247,9 +249,10 @@ public void testExpired() {
@Test
public void testLoadUserSessionsWithNotDeletedOfflineClientSessions() {
// Suspend periodic tasks to avoid race-conditions, which may cause missing updates of lastSessionRefresh times to UserSessionPersisterProvider
// skip for persistent user sessions as the periodic task is not used there
TimerProvider timer = kcSession.getProvider(TimerProvider.class);
TimerProvider.TimerTaskContext timerTaskCtx = null;
if (timer != null) {
if (timer != null && !Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS)) {
timerTaskCtx = timer.cancelTask(PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);
log.info("Cancelled periodic task " + PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);
}
Expand Down Expand Up @@ -318,7 +321,9 @@ public void testLoadUserSessionsWithNotDeletedOfflineClientSessions() {
} finally {
setTimeOffset(0);
kcSession.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent());
if (timer != null) {

// Enable periodic task again, skip for persistent user sessions as the periodic task is not used there
if (timer != null && !Profile.isFeatureEnabled(Profile.Feature.PERSISTENT_USER_SESSIONS)) {
timer.schedule(timerTaskCtx.getRunnable(), timerTaskCtx.getIntervalMillis(), PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME);
}

Expand Down

0 comments on commit 7fe4563

Please sign in to comment.