From 7a671052a32cf36b92c7c89d14ff6bcee1a04d3a Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 15 Apr 2019 22:43:23 +0200 Subject: [PATCH] KEYCLOAK-9988 Fix unstable UserSessionPersisterOfflineTest.testExpired. Adding ResetTimeOffsetEvent --- .../InfinispanUserSessionProviderFactory.java | 11 +++++++ .../AbstractLastSessionRefreshStore.java | 12 +++++-- .../models/utils/ResetTimeOffsetEvent.java | 28 +++++++++++++++++ .../rest/TestingResourceProvider.java | 7 +++++ .../AuthenticationSessionProviderTest.java | 2 ++ .../UserSessionPersisterProviderTest.java | 2 ++ .../model/UserSessionProviderOfflineTest.java | 31 +++++++++++++------ .../model/UserSessionProviderTest.java | 4 +++ 8 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 server-spi-private/src/main/java/org/keycloak/models/utils/ResetTimeOffsetEvent.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java index a59706af73..dc7b465bed 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProviderFactory.java @@ -58,6 +58,7 @@ import org.keycloak.models.sessions.infinispan.util.InfinispanKeyGenerator; import org.keycloak.models.sessions.infinispan.util.InfinispanUtil; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.PostMigrationEvent; +import org.keycloak.models.utils.ResetTimeOffsetEvent; import org.keycloak.provider.ProviderEvent; import org.keycloak.provider.ProviderEventListener; @@ -133,6 +134,16 @@ public class InfinispanUserSessionProviderFactory implements UserSessionProvider InfinispanUserSessionProvider provider = (InfinispanUserSessionProvider) userRemovedEvent.getKeycloakSession().getProvider(UserSessionProvider.class, getId()); provider.onUserRemoved(userRemovedEvent.getRealm(), userRemovedEvent.getUser()); + } else if (event instanceof ResetTimeOffsetEvent) { + if (persisterLastSessionRefreshStore != null) { + persisterLastSessionRefreshStore.reset(); + } + if (lastSessionRefreshStore != null) { + lastSessionRefreshStore.reset(); + } + if (offlineLastSessionRefreshStore != null) { + offlineLastSessionRefreshStore.reset(); + } } } }); diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStore.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStore.java index 69db5a26e1..cb1593db56 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStore.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/sessions/AbstractLastSessionRefreshStore.java @@ -54,7 +54,7 @@ public abstract class AbstractLastSessionRefreshStore { void checkSendingMessage(KeycloakSession kcSession, int currentTime) { if (lastSessionRefreshes.size() >= maxCount || lastRun + maxIntervalBetweenMessagesSeconds <= currentTime) { - Map refreshesToSend = prepareSendingMessage(currentTime); + Map refreshesToSend = prepareSendingMessage(); // Sending message doesn't need to be synchronized if (refreshesToSend != null) { @@ -65,7 +65,9 @@ public abstract class AbstractLastSessionRefreshStore { // synchronized manipulation with internal object instances. Will return map if message should be sent. Otherwise return null - private synchronized Map prepareSendingMessage(int currentTime) { + private synchronized Map prepareSendingMessage() { + // Safer to retrieve currentTime to avoid race conditions during testsuite + int currentTime = Time.currentTime(); if (lastSessionRefreshes.size() >= maxCount || lastRun + maxIntervalBetweenMessagesSeconds <= currentTime) { // Create new map instance, so that new writers will use that one Map copiedRefreshesToSend = lastSessionRefreshes; @@ -79,6 +81,12 @@ public abstract class AbstractLastSessionRefreshStore { } + public synchronized void reset() { + lastRun = Time.currentTime(); + lastSessionRefreshes = new ConcurrentHashMap<>(); + } + + /** * Bulk update the underlying store with all the user sessions, which were refreshed by Keycloak since the last call of this method * diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ResetTimeOffsetEvent.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ResetTimeOffsetEvent.java new file mode 100644 index 0000000000..80abf91a4d --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ResetTimeOffsetEvent.java @@ -0,0 +1,28 @@ +/* + * Copyright 2017 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.models.utils; + +import org.keycloak.provider.ProviderEvent; + +/** + * Useful when there is a need for callback when time offset is restarted. Time offset is typically used in testsuite only + * + * @author Marek Posolda + */ +public class ResetTimeOffsetEvent implements ProviderEvent { +} diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java index b4641b676e..adaad4df58 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/TestingResourceProvider.java @@ -47,6 +47,7 @@ import org.keycloak.models.UserProvider; import org.keycloak.models.UserSessionModel; import org.keycloak.models.sessions.infinispan.changes.sessions.CrossDCLastSessionRefreshStoreFactory; import org.keycloak.models.utils.ModelToRepresentation; +import org.keycloak.models.utils.ResetTimeOffsetEvent; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.mappers.AudienceProtocolMapper; import org.keycloak.provider.ProviderFactory; @@ -207,6 +208,12 @@ public class TestingResourceProvider implements RealmResourceProvider { public Map setTimeOffset(Map time) { int offset = Integer.parseInt(time.get("offset")); Time.setOffset(offset); + + // Time offset was restarted + if (offset == 0) { + session.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent()); + } + return getTimeOffset(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/AuthenticationSessionProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/AuthenticationSessionProviderTest.java index 60f53351ef..609fee1fcf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/AuthenticationSessionProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/AuthenticationSessionProviderTest.java @@ -27,6 +27,7 @@ import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.util.Time; import org.keycloak.models.*; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.models.utils.ResetTimeOffsetEvent; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.services.managers.ClientManager; import org.keycloak.services.managers.RealmManager; @@ -239,6 +240,7 @@ public class AuthenticationSessionProviderTest extends AbstractTestRealmKeycloak } finally { Time.setOffset(0); + session.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent()); setAccessCodeLifespan(mainSession, 60, 300, 1800); } }); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java index 6f8032808e..7851cf49ac 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionPersisterProviderTest.java @@ -26,6 +26,7 @@ import org.keycloak.common.util.Time; import org.keycloak.models.*; import org.keycloak.models.session.UserSessionPersisterProvider; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.models.utils.ResetTimeOffsetEvent; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.services.managers.ClientManager; @@ -521,6 +522,7 @@ public class UserSessionPersisterProviderTest extends AbstractTestRealmKeycloakT } finally { // Cleanup Time.setOffset(0); + session.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent()); } }); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderOfflineTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderOfflineTest.java index 9e8518b588..711c2e3c37 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderOfflineTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderOfflineTest.java @@ -30,6 +30,7 @@ import org.keycloak.models.*; import org.keycloak.models.session.UserSessionPersisterProvider; import org.keycloak.models.sessions.infinispan.changes.sessions.PersisterLastSessionRefreshStoreFactory; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.models.utils.ResetTimeOffsetEvent; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.services.managers.ClientManager; @@ -438,6 +439,7 @@ public class UserSessionProviderOfflineTest extends AbstractTestRealmKeycloakTes // Suspend periodic tasks to avoid race-conditions, which may cause missing updates of lastSessionRefresh times to UserSessionPersisterProvider TimerProvider timer = session.getProvider(TimerProvider.class); TimerProvider.TimerTaskContext timerTaskCtx = timer.cancelTask(PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME); + log.info("Cancelled periodic task " + PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME); try { AtomicReference origSessionsAt = new AtomicReference<>(); @@ -474,6 +476,8 @@ public class UserSessionProviderOfflineTest extends AbstractTestRealmKeycloakTes } }); + log.info("Persisted 3 sessions to UserSessionPersisterProvider"); + KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession sessionExpired3) -> { currentSession = sessionExpired3; realm = currentSession.realms().getRealm("test"); @@ -487,22 +491,29 @@ public class UserSessionProviderOfflineTest extends AbstractTestRealmKeycloakTes Assert.assertEquals(3, persister.getUserSessionsCount(true)); Time.setOffset(300); + log.infof("Set time offset to 300. Time is: %d", Time.currentTime()); // Set lastSessionRefresh to currentSession[0] to 0 session0.setLastSessionRefresh(Time.currentTime()); }); - KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession sessionExpired4) -> { - currentSession = sessionExpired4; - realm = currentSession.realms().getRealm("test"); - UserSessionModel[] origSessions = origSessionsAt.get(); - // Increase timeOffset - 20 days - Time.setOffset(1728000); + // Increase timeOffset and update LSR of the session two times - first to 20 days and then to 21 days. At least one of updates + // will propagate to PersisterLastSessionRefreshStore and update DB (Single update is not 100% sure as there is still a + // chance of delayed periodic task to be run in the meantime and causing race-condition, which would mean LSR not updated in the DB) + for (int i=0 ; i<2 ; i++) { + int timeOffset = 1728000 + (i * 86400); + KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession sessionExpired4) -> { + currentSession = sessionExpired4; + realm = currentSession.realms().getRealm("test"); + UserSessionModel[] origSessions = origSessionsAt.get(); + Time.setOffset(timeOffset); + log.infof("Set time offset to %d. Time is: %d", timeOffset, Time.currentTime()); - UserSessionModel session0 = currentSession.sessions().getOfflineUserSession(realm, origSessions[0].getId()); - session0.setLastSessionRefresh(Time.currentTime()); - }); + UserSessionModel session0 = currentSession.sessions().getOfflineUserSession(realm, origSessions[0].getId()); + session0.setLastSessionRefresh(Time.currentTime()); + }); + } KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession sessionExpired5) -> { currentSession = sessionExpired5; @@ -511,6 +522,7 @@ public class UserSessionProviderOfflineTest extends AbstractTestRealmKeycloakTes // Increase timeOffset - 40 days Time.setOffset(3456000); + log.infof("Set time offset to 3456000. Time is: %d", Time.currentTime()); // Expire and ensure that all sessions despite session0 were removed currentSession.sessions().removeExpired(realm); @@ -552,6 +564,7 @@ public class UserSessionProviderOfflineTest extends AbstractTestRealmKeycloakTes } finally { Time.setOffset(0); + session.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent()); timer.schedule(timerTaskCtx.getRunnable(), timerTaskCtx.getIntervalMillis(), PersisterLastSessionRefreshStoreFactory.DB_LSR_PERIODIC_TASK_NAME); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java index fa947448f5..af10d50fea 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java @@ -29,6 +29,7 @@ import org.keycloak.common.util.Time; import org.keycloak.models.*; import org.keycloak.models.sessions.infinispan.entities.UserSessionEntity; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.models.utils.ResetTimeOffsetEvent; import org.keycloak.models.utils.SessionTimeoutHelper; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.representations.idm.RealmRepresentation; @@ -380,6 +381,7 @@ public class UserSessionProviderTest extends AbstractTestRealmKeycloakTest { } } finally { Time.setOffset(0); + session.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent()); } } @@ -457,6 +459,7 @@ public class UserSessionProviderTest extends AbstractTestRealmKeycloakTest { } finally { Time.setOffset(0); + session.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent()); // restore the original remember-me timeout values in the realm. KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), (KeycloakSession kcSession) -> { RealmModel r = kcSession.realms().getRealmByName("test"); @@ -481,6 +484,7 @@ public class UserSessionProviderTest extends AbstractTestRealmKeycloakTest { session.sessions().removeUserSession(realm, userSession); } finally { Time.setOffset(0); + session.getKeycloakSessionFactory().publish(new ResetTimeOffsetEvent()); } }