From 969b8c153fbe9636a6c075967a84eb63778f4ea9 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 12 Feb 2016 12:54:08 +0100 Subject: [PATCH] KEYCLOAK-1989 Refreshing offline tokens didn't work correctly in cluster with revokeRefreshToken enabled --- .../InfinispanUserSessionProvider.java | 14 +++++++ .../InfinispanUserSessionProviderFactory.java | 5 ++- .../InfinispanUserSessionInitializer.java | 39 +++++-------------- .../initializer/OfflineUserSessionLoader.java | 7 ++-- .../infinispan/initializer/SessionLoader.java | 2 +- .../TimeAwareInitializerState.java | 34 ++++++++++++++++ .../keycloak/models/UserSessionProvider.java | 3 ++ .../keycloak/protocol/oidc/TokenManager.java | 4 +- .../resources/KeycloakApplication.java | 3 +- .../model/UserSessionInitializerTest.java | 2 +- 10 files changed, 74 insertions(+), 39 deletions(-) create mode 100644 model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/TimeAwareInitializerState.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java index 9cf19c2390..1fc04defc4 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java @@ -24,6 +24,7 @@ import org.keycloak.common.util.Time; import org.keycloak.models.*; import org.keycloak.models.session.UserSessionPersisterProvider; import org.keycloak.models.sessions.infinispan.entities.*; +import org.keycloak.models.sessions.infinispan.initializer.TimeAwareInitializerState; import org.keycloak.models.sessions.infinispan.stream.*; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RealmInfoUtil; @@ -410,6 +411,19 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { loginFailureCache.remove(new LoginFailureKey(realm.getId(), user.getEmail())); } + @Override + public int getClusterStartupTime() { + TimeAwareInitializerState state = (TimeAwareInitializerState) offlineSessionCache.get(InfinispanUserSessionProviderFactory.SESSION_INITIALIZER_STATE_KEY); + int startTime; + if (state == null) { + log.warn("Cluster startup time not yet available. Fallback to local startup time"); + startTime = (int)(session.getKeycloakSessionFactory().getServerStartupTimestamp() / 1000); + } else { + startTime = state.getClusterStartupTime(); + } + return startTime; + } + @Override public void close() { } 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 7f20f0e8b3..1bb82a1b48 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 @@ -34,6 +34,9 @@ import org.keycloak.provider.ProviderEventListener; public class InfinispanUserSessionProviderFactory implements UserSessionProviderFactory { + private static final String STATE_KEY_PREFIX = "initializerState"; + public static final String SESSION_INITIALIZER_STATE_KEY = STATE_KEY_PREFIX + "::offlineUserSessions"; + private static final Logger log = Logger.getLogger(InfinispanUserSessionProviderFactory.class); private Config.Scope config; @@ -84,7 +87,7 @@ public class InfinispanUserSessionProviderFactory implements UserSessionProvider InfinispanConnectionProvider connections = session.getProvider(InfinispanConnectionProvider.class); Cache cache = connections.getCache(InfinispanConnectionProvider.OFFLINE_SESSION_CACHE_NAME); - InfinispanUserSessionInitializer initializer = new InfinispanUserSessionInitializer(sessionFactory, cache, new OfflineUserSessionLoader(), maxErrors, sessionsPerSegment, "offlineUserSessions"); + InfinispanUserSessionInitializer initializer = new InfinispanUserSessionInitializer(sessionFactory, cache, new OfflineUserSessionLoader(), maxErrors, sessionsPerSegment, SESSION_INITIALIZER_STATE_KEY); initializer.initCache(); initializer.loadPersistentSessions(); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanUserSessionInitializer.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanUserSessionInitializer.java index 2ce4ac205e..deae897c1a 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanUserSessionInitializer.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/InfinispanUserSessionInitializer.java @@ -35,6 +35,7 @@ import org.infinispan.notifications.cachemanagerlistener.annotation.ViewChanged; import org.infinispan.notifications.cachemanagerlistener.event.ViewChangedEvent; import org.infinispan.remoting.transport.Transport; import org.jboss.logging.Logger; +import org.keycloak.common.util.Time; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.KeycloakSessionTask; @@ -51,8 +52,6 @@ public class InfinispanUserSessionInitializer { private static final Logger log = Logger.getLogger(InfinispanUserSessionInitializer.class); - private static final String STATE_KEY_PREFIX = "initializerState"; - private final KeycloakSessionFactory sessionFactory; private final Cache cache; private final SessionLoader sessionLoader; @@ -60,21 +59,18 @@ public class InfinispanUserSessionInitializer { private final int sessionsPerSegment; private final String stateKey; - private volatile CountDownLatch latch = new CountDownLatch(1); - - public InfinispanUserSessionInitializer(KeycloakSessionFactory sessionFactory, Cache cache, SessionLoader sessionLoader, int maxErrors, int sessionsPerSegment, String stateKeySuffix) { + public InfinispanUserSessionInitializer(KeycloakSessionFactory sessionFactory, Cache cache, SessionLoader sessionLoader, int maxErrors, int sessionsPerSegment, String stateKey) { this.sessionFactory = sessionFactory; this.cache = cache; this.sessionLoader = sessionLoader; this.maxErrors = maxErrors; this.sessionsPerSegment = sessionsPerSegment; - this.stateKey = STATE_KEY_PREFIX + "::" + stateKeySuffix; + this.stateKey = stateKey; } public void initCache() { this.cache.getAdvancedCache().getComponentRegistry().registerComponent(sessionFactory, KeycloakSessionFactory.class); - cache.getCacheManager().addListener(new ViewChangeListener()); } @@ -86,7 +82,7 @@ public class InfinispanUserSessionInitializer { while (!isFinished()) { if (!isCoordinator()) { try { - latch.await(500, TimeUnit.MILLISECONDS); + Thread.sleep(1000); } catch (InterruptedException ie) { log.error("Interrupted", ie); } @@ -104,8 +100,10 @@ public class InfinispanUserSessionInitializer { private InitializerState getOrCreateInitializerState() { - InitializerState state = (InitializerState) cache.get(stateKey); + TimeAwareInitializerState state = (TimeAwareInitializerState) cache.get(stateKey); if (state == null) { + int startTime = (int)(sessionFactory.getServerStartupTimestamp() / 1000); + final int[] count = new int[1]; // Rather use separate transactions for update and counting @@ -113,7 +111,7 @@ public class InfinispanUserSessionInitializer { KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() { @Override public void run(KeycloakSession session) { - sessionLoader.init(session); + sessionLoader.init(session, startTime); } }); @@ -126,8 +124,9 @@ public class InfinispanUserSessionInitializer { }); - state = new InitializerState(); + state = new TimeAwareInitializerState(); state.init(count[0], sessionsPerSegment); + state.setClusterStartupTime(startTime); saveStateToCache(state); } return state; @@ -251,24 +250,6 @@ public class InfinispanUserSessionInitializer { } } - - @Listener - public class ViewChangeListener { - - @ViewChanged - public void viewChanged(ViewChangedEvent event) { - boolean isCoordinator = isCoordinator(); - log.debug("View Changed: is coordinator: " + isCoordinator); - - if (isCoordinator) { - latch.countDown(); - latch = new CountDownLatch(1); - } - } - - } - - public static class WorkerResult implements Serializable { private Integer segment; diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/OfflineUserSessionLoader.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/OfflineUserSessionLoader.java index c847d51496..8b651c0e31 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/OfflineUserSessionLoader.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/OfflineUserSessionLoader.java @@ -33,14 +33,13 @@ public class OfflineUserSessionLoader implements SessionLoader { private static final Logger log = Logger.getLogger(OfflineUserSessionLoader.class); @Override - public void init(KeycloakSession session) { + public void init(KeycloakSession session, int clusterStartupTime) { UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); - int startTime = (int)(session.getKeycloakSessionFactory().getServerStartupTimestamp() / 1000); - log.debugf("Clearing detached sessions from persistent storage and updating timestamps to %d", startTime); + log.debugf("Clearing detached sessions from persistent storage and updating timestamps to %d", clusterStartupTime); persister.clearDetachedUserSessions(); - persister.updateAllTimestamps(startTime); + persister.updateAllTimestamps(clusterStartupTime); } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/SessionLoader.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/SessionLoader.java index 3185a39370..b8aa0f8065 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/SessionLoader.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/SessionLoader.java @@ -26,7 +26,7 @@ import org.keycloak.models.KeycloakSession; */ public interface SessionLoader extends Serializable { - void init(KeycloakSession session); + void init(KeycloakSession session, int clusterStartupTime); int getSessionsCount(KeycloakSession session); diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/TimeAwareInitializerState.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/TimeAwareInitializerState.java new file mode 100644 index 0000000000..f5b7f04595 --- /dev/null +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/initializer/TimeAwareInitializerState.java @@ -0,0 +1,34 @@ +/* + * Copyright 2016 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.sessions.infinispan.initializer; + +/** + * @author Marek Posolda + */ +public class TimeAwareInitializerState extends InitializerState { + + private int clusterStartupTime; + + public int getClusterStartupTime() { + return clusterStartupTime; + } + + public void setClusterStartupTime(int clusterStartupTime) { + this.clusterStartupTime = clusterStartupTime; + } +} diff --git a/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java b/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java index 57cd49c3d2..9d7c413b6a 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java @@ -82,6 +82,9 @@ public interface UserSessionProvider extends Provider { void removeClientInitialAccessModel(RealmModel realm, String id); List listClientInitialAccess(RealmModel realm); + // Will use startup time of this server in non-cluster environment + int getClusterStartupTime(); + void close(); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 45dc04325c..e4174d9d5c 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -193,9 +193,9 @@ public class TokenManager { int currentTime = Time.currentTime(); if (realm.isRevokeRefreshToken()) { - int serverStartupTime = (int)(session.getKeycloakSessionFactory().getServerStartupTimestamp() / 1000); + int clusterStartupTime = session.sessions().getClusterStartupTime(); - if (refreshToken.getIssuedAt() < validation.clientSession.getTimestamp() && (serverStartupTime != validation.clientSession.getTimestamp())) { + if (refreshToken.getIssuedAt() < validation.clientSession.getTimestamp() && (clusterStartupTime != validation.clientSession.getTimestamp())) { throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Stale token"); } diff --git a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java index 6fb06f6cb4..4b49d797aa 100644 --- a/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java +++ b/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java @@ -91,7 +91,6 @@ public class KeycloakApplication extends Application { singletons.add(new ObjectMapperResolver(Boolean.parseBoolean(System.getProperty("keycloak.jsonPrettyPrint", "false")))); migrateModel(); - sessionFactory.publish(new PostMigrationEvent()); boolean bootstrapAdminUser = false; @@ -138,6 +137,8 @@ public class KeycloakApplication extends Application { session.close(); } + sessionFactory.publish(new PostMigrationEvent()); + singletons.add(new WelcomeResource(bootstrapAdminUser)); setupScheduledTasks(sessionFactory); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionInitializerTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionInitializerTest.java index 929de9bcb8..2d763b635d 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionInitializerTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionInitializerTest.java @@ -83,7 +83,7 @@ public class UserSessionInitializerTest { // Create and persist offline sessions int started = Time.currentTime(); - int serverStartTime = (int)(session.getKeycloakSessionFactory().getServerStartupTimestamp() / 1000); + int serverStartTime = session.sessions().getClusterStartupTime(); for (UserSessionModel origSession : origSessions) { UserSessionModel userSession = session.sessions().getUserSession(realm, origSession.getId());