From 3988989e88b2bc7055caa25f5b7a4d271a285509 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 17 Dec 2020 12:24:37 +0100 Subject: [PATCH 1/7] KEYCLOAK-11019 InfinispanUserSessionProvider should fallback to database query if in-memory lookup fails for offline sessions Allow to configure pre-loading of offline user-sessions We now allow to configure whether offline user-sessions should be preloaded on startup from the database. We now trigger offline user-sessions loading when UserSessionProvider#getOfflineUserSessionsStream(realm,user) is called. This ensures that the user can revoke the offline_access consent via the account console. Additionally the (offline) user consents are now displayed correctly for a user in the admin-console. Offline user-sessions are now correctly display if offline user-sessions are loaded lazily. Add note about unsupported lazy offline session loading by broker info: Currently we don't support lazy offline user-session lookup by brokerSessionId and brokerUserId. See: https://github.com/keycloak/keycloak/pull/7722#pullrequestreview-578669985 Keep behavior of 'logout all sessions' and don't delete dangling offline sessions. As discussed in https://github.com/keycloak/keycloak/pull/7722#discussion_r566989120 Throw on unsupported dynamic offline user-session lookups from database --- .../InfinispanUserSessionProvider.java | 140 +++++++++++++- .../InfinispanUserSessionProviderFactory.java | 37 ++-- .../stream/UserSessionPredicate.java | 16 ++ .../JpaUserSessionPersisterProvider.java | 175 ++++++++++++++++-- .../PersistentClientSessionEntity.java | 5 +- .../session/PersistentUserSessionEntity.java | 21 ++- .../DisabledUserSessionPersisterProvider.java | 33 +++- .../session/UserSessionPersisterProvider.java | 81 +++++++- 8 files changed, 465 insertions(+), 43 deletions(-) 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 765a00b58c6c..8aaecaacc98d 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 @@ -30,6 +30,7 @@ import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelException; import org.keycloak.models.OfflineUserSessionModel; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; @@ -60,9 +61,11 @@ import java.io.Serializable; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -105,6 +108,8 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { protected final RemoteCacheInvoker remoteCacheInvoker; protected final InfinispanKeyGenerator keyGenerator; + protected final boolean loadOfflineSessionsStatsFromDatabase; + public InfinispanUserSessionProvider(KeycloakSession session, RemoteCacheInvoker remoteCacheInvoker, CrossDCLastSessionRefreshStore lastSessionRefreshStore, @@ -114,7 +119,8 @@ public InfinispanUserSessionProvider(KeycloakSession session, Cache> sessionCache, Cache> offlineSessionCache, Cache> clientSessionCache, - Cache> offlineClientSessionCache) { + Cache> offlineClientSessionCache, + boolean loadOfflineSessionsStatsFromDatabase) { this.session = session; this.sessionCache = sessionCache; @@ -134,6 +140,7 @@ public InfinispanUserSessionProvider(KeycloakSession session, this.persisterLastSessionRefreshStore = persisterLastSessionRefreshStore; this.remoteCacheInvoker = remoteCacheInvoker; this.keyGenerator = keyGenerator; + this.loadOfflineSessionsStatsFromDatabase = loadOfflineSessionsStatsFromDatabase; session.getTransactionManager().enlistAfterCompletion(clusterEventsSenderTx); session.getTransactionManager().enlistAfterCompletion(sessionTx); @@ -239,19 +246,78 @@ void updateSessionEntity(UserSessionEntity entity, RealmModel realm, UserModel u entity.setStarted(currentTime); entity.setLastSessionRefresh(currentTime); - - } - @Override public UserSessionModel getUserSession(RealmModel realm, String id) { return getUserSession(realm, id, false); } protected UserSessionAdapter getUserSession(RealmModel realm, String id, boolean offline) { - UserSessionEntity entity = getUserSessionEntity(realm, id, offline); - return wrap(realm, entity, offline); + + UserSessionEntity userSessionEntityFromCache = getUserSessionEntity(realm, id, offline); + if (userSessionEntityFromCache != null) { + return wrap(realm, userSessionEntityFromCache, offline); + } + + if (!offline) { + return null; + } + + // Try to recover from potentially lost offline-sessions by attempting to fetch and re-import + // the offline session information from the PersistenceProvider. + UserSessionEntity userSessionEntityFromPersistenceProvider = getUserSessionEntityFromPersistenceProvider(realm, id, offline); + if (userSessionEntityFromPersistenceProvider != null) { + // we successfully recovered the offline session! + return wrap(realm, userSessionEntityFromPersistenceProvider, offline); + } + + // no luck, the session is really not there anymore + return null; + } + + private UserSessionEntity getUserSessionEntityFromPersistenceProvider(RealmModel realm, String sessionId, boolean offline) { + + log.debugf("Offline user-session not found in infinispan, attempting UserSessionPersisterProvider lookup for sessionId=%s", sessionId); + UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); + UserSessionModel persistentUserSession = persister.loadUserSession(realm, sessionId, offline); + + if (persistentUserSession == null) { + log.debugf("Offline user-session not found in UserSessionPersisterProvider for sessionId=%s", sessionId); + return null; + } + + return importUserSession(realm, offline, persistentUserSession); + } + + private UserSessionEntity getUserSessionEntityFromCacheOrImportIfNecessary(RealmModel realm, boolean offline, UserSessionModel persistentUserSession) { + + UserSessionEntity userSessionEntity = getUserSessionEntity(realm, persistentUserSession.getId(), offline); + if (userSessionEntity != null) { + // user session present in cache, return existing session + return userSessionEntity; + } + + return importUserSession(realm, offline, persistentUserSession); + } + + private UserSessionEntity importUserSession(RealmModel realm, boolean offline, UserSessionModel persistentUserSession) { + + String sessionId = persistentUserSession.getId(); + + log.debugf("Attempting to import user-session for sessionId=%s offline=%s", sessionId, offline); + session.sessions().importUserSessions(Collections.singleton(persistentUserSession), offline); + log.debugf("user-session imported, trying another lookup for sessionId=%s offline=%s", sessionId, offline); + + UserSessionEntity ispnUserSessionEntity = getUserSessionEntity(realm, sessionId, offline); + + if (ispnUserSessionEntity != null) { + log.debugf("user-session found after import for sessionId=%s offline=%s", sessionId, offline); + return ispnUserSessionEntity; + } + + log.debugf("user-session could not be found after import for sessionId=%s offline=%s", sessionId, offline); + return null; } private UserSessionEntity getUserSessionEntity(RealmModel realm, String id, boolean offline) { @@ -263,8 +329,41 @@ private UserSessionEntity getUserSessionEntity(RealmModel realm, String id, bool return entity; } + private Stream getUserSessionsFromPersistenceProviderStream(RealmModel realm, UserModel user, boolean offline) { + UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); + return persister.loadUserSessionsStream(realm, user, offline, 0, null) + .map(persistentUserSession -> getUserSessionEntityFromCacheOrImportIfNecessary(realm, offline, persistentUserSession)) + .filter(Objects::nonNull) + .map(userSessionEntity -> wrap(realm, userSessionEntity, offline)); + } + + + protected Stream getUserSessionsStream(RealmModel realm, UserSessionPredicate predicate, boolean offline) { + + if (offline && loadOfflineSessionsStatsFromDatabase) { + + // fetch the offline user-sessions from the persistence provider + UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); + + UserModel user = session.users().getUserById(realm, predicate.getUser()); + if (user != null) { + return persister.loadUserSessionsStream(realm, user, offline, 0, null); + } + + if (predicate.getBrokerSessionId() != null) { + // TODO add support for offline user-session lookup by brokerSessionId + // currently it is not possible to access the brokerSessionId in offline user-session in a database agnostic way + throw new ModelException("Dynamic database lookup for offline user-sessions by brokerSessionId is currently not supported."); + } + + if (predicate.getBrokerUserId() != null) { + // TODO add support for offline user-session lookup by brokerUserId + // currently it is not possible to access the brokerUserId in offline user-session in a database agnostic way + throw new ModelException("Dynamic database lookup for offline user-sessions by brokerUserId is currently not supported."); + } + + } - protected Stream getUserSessionsStream(RealmModel realm, Predicate>> predicate, boolean offline) { Cache> cache = getCache(offline); cache = CacheDecorators.skipCacheLoaders(cache); @@ -321,6 +420,13 @@ public Stream getUserSessionsStream(RealmModel realm, ClientMo } protected Stream getUserSessionsStream(final RealmModel realm, ClientModel client, Integer firstResult, Integer maxResults, final boolean offline) { + + if (offline && loadOfflineSessionsStatsFromDatabase) { + // fetch the actual offline user session count from the database + UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); + return persister.loadUserSessionsStream(realm, client, offline, firstResult, maxResults); + } + final String clientUuid = client.getId(); UserSessionPredicate predicate = UserSessionPredicate.create(realm.getId()).client(clientUuid); @@ -410,6 +516,12 @@ public long getActiveUserSessions(RealmModel realm, ClientModel client) { @Override public Map getActiveClientSessionStats(RealmModel realm, boolean offline) { + + if (offline && loadOfflineSessionsStatsFromDatabase) { + UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); + return persister.getUserSessionsCountsByClients(realm, offline); + } + Cache> cache = getCache(offline); cache = CacheDecorators.skipCacheLoaders(cache); return cache.entrySet().stream() @@ -424,6 +536,13 @@ public Map getActiveClientSessionStats(RealmModel realm, boolean o } protected long getUserSessionsCount(RealmModel realm, ClientModel client, boolean offline) { + + if (offline && loadOfflineSessionsStatsFromDatabase) { + // fetch the actual offline user session count from the database + UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); + return persister.getUserSessionsCount(realm, client, offline); + } + Cache> cache = getCache(offline); cache = CacheDecorators.skipCacheLoaders(cache); @@ -662,7 +781,12 @@ public AuthenticatedClientSessionModel createOfflineClientSession(AuthenticatedC @Override public Stream getOfflineUserSessionsStream(RealmModel realm, UserModel user) { - return this.getUserSessionsStream(realm, UserSessionPredicate.create(realm.getId()).user(user.getId()), true); + + if (loadOfflineSessionsStatsFromDatabase) { + return getUserSessionsFromPersistenceProviderStream(realm, user, true); + } + + return getUserSessionsStream(realm, UserSessionPredicate.create(realm.getId()).user(user.getId()), true); } @Override 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 aa757a33eded..8f848db57c65 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 @@ -95,8 +95,10 @@ public InfinispanUserSessionProvider create(KeycloakSession session) { Cache> clientSessionCache = connections.getCache(InfinispanConnectionProvider.CLIENT_SESSION_CACHE_NAME); Cache> offlineClientSessionsCache = connections.getCache(InfinispanConnectionProvider.OFFLINE_CLIENT_SESSION_CACHE_NAME); + boolean loadOfflineSessionsStatsFromDatabase = !isPreloadingOfflineSessionsFromDatabaseEnabled(); + return new InfinispanUserSessionProvider(session, remoteCacheInvoker, lastSessionRefreshStore, offlineLastSessionRefreshStore, - persisterLastSessionRefreshStore, keyGenerator, cache, offlineSessionsCache, clientSessionCache, offlineClientSessionsCache); + persisterLastSessionRefreshStore, keyGenerator, cache, offlineSessionsCache, clientSessionCache, offlineClientSessionsCache, loadOfflineSessionsStatsFromDatabase); } @Override @@ -145,6 +147,10 @@ public void onEvent(ProviderEvent event) { }); } + private boolean isPreloadingOfflineSessionsFromDatabaseEnabled() { + return config.getBoolean("preloadOfflineSessionsFromDatabase", true); + } + // Max count of worker errors. Initialization will end with exception when this number is reached private int getMaxErrors() { return config.getInt("maxErrors", 20); @@ -163,23 +169,32 @@ private int getTimeoutForPreloadingSessionsSeconds() { @Override public void loadPersistentSessions(final KeycloakSessionFactory sessionFactory, final int maxErrors, final int sessionsPerSegment) { - log.debug("Start pre-loading userSessions from persistent storage"); KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() { @Override public void run(KeycloakSession session) { - InfinispanConnectionProvider connections = session.getProvider(InfinispanConnectionProvider.class); - Cache workCache = connections.getCache(InfinispanConnectionProvider.WORK_CACHE_NAME); - InfinispanCacheInitializer ispnInitializer = new InfinispanCacheInitializer(sessionFactory, workCache, - new OfflinePersistentUserSessionLoader(sessionsPerSegment), "offlineUserSessions", sessionsPerSegment, maxErrors); + if (isPreloadingOfflineSessionsFromDatabaseEnabled()) { + // only preload offline-sessions if necessary + log.debug("Start pre-loading userSessions from persistent storage"); - // DB-lock to ensure that persistent sessions are loaded from DB just on one DC. The other DCs will load them from remote cache. - CacheInitializer initializer = new DBLockBasedCacheInitializer(session, ispnInitializer); + InfinispanConnectionProvider connections = session.getProvider(InfinispanConnectionProvider.class); + Cache workCache = connections.getCache(InfinispanConnectionProvider.WORK_CACHE_NAME); - initializer.initCache(); - initializer.loadSessions(); + InfinispanCacheInitializer ispnInitializer = new InfinispanCacheInitializer(sessionFactory, workCache, + new OfflinePersistentUserSessionLoader(sessionsPerSegment), "offlineUserSessions", sessionsPerSegment, maxErrors); + + // DB-lock to ensure that persistent sessions are loaded from DB just on one DC. The other DCs will load them from remote cache. + CacheInitializer initializer = new DBLockBasedCacheInitializer(session, ispnInitializer); + + initializer.initCache(); + initializer.loadSessions(); + + log.debug("Pre-loading userSessions from persistent storage finished"); + } else { + log.debug("Skipping pre-loading of userSessions from persistent storage"); + } // Initialize persister for periodically doing bulk DB updates of lastSessionRefresh timestamps of refreshed sessions persisterLastSessionRefreshStore = new PersisterLastSessionRefreshStoreFactory().createAndInit(session, true); @@ -187,7 +202,7 @@ public void run(KeycloakSession session) { }); - log.debug("Pre-loading userSessions from persistent storage finished"); + } diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java index 372fe24772c5..ece20181684d 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java @@ -108,6 +108,22 @@ public UserSessionPredicate brokerUserId(String id) { return this; } + /** + * Returns the user id. + * @return + */ + public String getUser() { + return user; + } + + public String getBrokerSessionId() { + return brokerSessionId; + } + + public String getBrokerUserId() { + return brokerUserId; + } + @Override public boolean test(Map.Entry> entry) { UserSessionEntity entity = entry.getValue().getEntity(); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java index 78c2fcb67c88..a047fc77ed76 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java @@ -38,6 +38,7 @@ import javax.persistence.TypedQuery; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -126,6 +127,17 @@ public void removeUserSession(String userSessionId, boolean offline) { } } + @Override + public int removeUserSessions(RealmModel realm, Boolean offline) { + + String offlineStr = offline == null ? null : offlineToString(offline); + + return em.createNamedQuery("deleteUserSessionsByRealm") + .setParameter("realmId", realm.getId()) + .setParameter("offline", offlineStr) + .executeUpdate(); + } + @Override public void removeClientSession(String userSessionId, String clientUUID, boolean offline) { String offlineStr = offlineToString(offline); @@ -171,8 +183,15 @@ private List getClientSessionsByUserSession(Strin @Override public void onRealmRemoved(RealmModel realm) { - int num = em.createNamedQuery("deleteClientSessionsByRealm").setParameter("realmId", realm.getId()).executeUpdate(); - num = em.createNamedQuery("deleteUserSessionsByRealm").setParameter("realmId", realm.getId()).executeUpdate(); + int deletedClientSessions = em.createNamedQuery("deleteClientSessionsByRealm") + .setParameter("realmId", realm.getId()) + .setParameter("offline", null) + .executeUpdate(); + + int deletedUserSessions = em.createNamedQuery("deleteUserSessionsByRealm") + .setParameter("realmId", realm.getId()) + .setParameter("offline", null) + .executeUpdate(); } @Override @@ -243,17 +262,115 @@ public void removeExpired(RealmModel realm) { } @Override + public Map getUserSessionsCountsByClients(RealmModel realm, boolean offline) { + + String offlineStr = offlineToString(offline); + + Query query = em.createNamedQuery("findUserSessionsCountsByClientId"); + + query.setParameter("offline", offlineStr); + query.setParameter("realmId", realm.getId()); + query.setParameter("clientId", null); + + Map offlineSessionsByClient = new HashMap<>(); + + closing(query.getResultStream()).forEach(record -> { + + Object[] row = (Object[]) record; + + String clientId = String.valueOf(row[0]); + Long count = ((Number)row[1]).longValue(); + + offlineSessionsByClient.put(clientId, count); + }); + + return offlineSessionsByClient; + } + + @Override + public UserSessionModel loadUserSession(RealmModel realm, String userSessionId, boolean offline) { + + String offlineStr = offlineToString(offline); + + TypedQuery userSessionQuery = em.createNamedQuery("findUserSession", PersistentUserSessionEntity.class); + userSessionQuery.setParameter("realmId", realm.getId()); + userSessionQuery.setParameter("offline", offlineStr); + userSessionQuery.setParameter("userSessionId", userSessionId); + userSessionQuery.setMaxResults(1); + + Stream persistentUserSessions = closing(userSessionQuery.getResultStream().map(this::toAdapter)); + + return persistentUserSessions.findAny().map(userSession -> { + + TypedQuery clientSessionQuery = em.createNamedQuery("findClientSessionsByUserSession", PersistentClientSessionEntity.class); + clientSessionQuery.setParameter("userSessionId", Collections.singleton(userSessionId)); + clientSessionQuery.setParameter("offline", offlineStr); + + Set removedClientUUIDs = new HashSet<>(); + + clientSessionQuery.getResultStream().forEach(clientSession -> { + boolean added = addClientSessionToAuthenticatedClientSessionsIfPresent(userSession, clientSession); + if (!added) { + // client was removed in the meantime + removedClientUUIDs.add(clientSession.getClientId()); + } + } + ); + + removedClientUUIDs.forEach(this::onClientRemoved); + + return userSession; + }).orElse(null); + } + + @Override + public Stream loadUserSessionsStream(RealmModel realm, ClientModel client, boolean offline, Integer firstResult, Integer maxResults) { + + String offlineStr = offlineToString(offline); + + TypedQuery query = paginateQuery( + em.createNamedQuery("findUserSessionsByClientId", PersistentUserSessionEntity.class), + firstResult, maxResults); + + query.setParameter("offline", offlineStr); + query.setParameter("realmId", realm.getId()); + query.setParameter("clientId", client.getId()); + + return loadUserSessionsWithClientSessions(query, offlineStr); + } + + @Override + public Stream loadUserSessionsStream(RealmModel realm, UserModel user, boolean offline, Integer firstResult, Integer maxResults) { + + String offlineStr = offlineToString(offline); + + TypedQuery query = paginateQuery( + em.createNamedQuery("findUserSessionsByUserId", PersistentUserSessionEntity.class), + firstResult, maxResults); + + query.setParameter("offline", offlineStr); + query.setParameter("realmId", realm.getId()); + query.setParameter("userId", user.getId()); + + return loadUserSessionsWithClientSessions(query, offlineStr); + } + public Stream loadUserSessionsStream(Integer firstResult, Integer maxResults, boolean offline, String lastUserSessionId) { String offlineStr = offlineToString(offline); - TypedQuery queryUserSessions = em.createNamedQuery("findUserSessionsOrderedById", PersistentUserSessionEntity.class); - queryUserSessions.setParameter("offline", offlineStr); - queryUserSessions.setParameter("lastSessionId", lastUserSessionId); + TypedQuery query = paginateQuery(em.createNamedQuery("findUserSessionsOrderedById", PersistentUserSessionEntity.class) + .setParameter("offline", offlineStr) + .setParameter("lastSessionId", lastUserSessionId), firstResult, maxResults); + + return loadUserSessionsWithClientSessions(query, offlineStr); + } - List userSessionAdapters = closing(paginateQuery(queryUserSessions, firstResult, maxResults).getResultStream() - .map(this::toAdapter)) - .filter(Objects::nonNull) + private Stream loadUserSessionsWithClientSessions(TypedQuery query, String offlineStr) { + + List userSessionAdapters = closing(query.getResultStream() + .map(this::toAdapter) + .filter(Objects::nonNull)) .collect(Collectors.toList()); Map sessionsById = userSessionAdapters.stream() @@ -272,15 +389,10 @@ public Stream loadUserSessionsStream(Integer firstResult, Inte closing(queryClientSessions.getResultStream()).forEach(clientSession -> { PersistentUserSessionAdapter userSession = sessionsById.get(clientSession.getUserSessionId()); - - PersistentAuthenticatedClientSessionAdapter clientSessAdapter = toAdapter(userSession.getRealm(), userSession, clientSession); - Map currentClientSessions = userSession.getAuthenticatedClientSessions(); - - // Case when client was removed in the meantime - if (clientSessAdapter.getClient() == null) { + boolean added = addClientSessionToAuthenticatedClientSessionsIfPresent(userSession, clientSession); + if (!added) { + // client was removed in the meantime removedClientUUIDs.add(clientSession.getClientId()); - } else { - currentClientSessions.put(clientSession.getClientId(), clientSessAdapter); } }); } @@ -292,6 +404,18 @@ public Stream loadUserSessionsStream(Integer firstResult, Inte return userSessionAdapters.stream().map(UserSessionModel.class::cast); } + private boolean addClientSessionToAuthenticatedClientSessionsIfPresent(PersistentUserSessionAdapter userSession, PersistentClientSessionEntity clientSessionEntity) { + + PersistentAuthenticatedClientSessionAdapter clientSessAdapter = toAdapter(userSession.getRealm(), userSession, clientSessionEntity); + + if (clientSessAdapter.getClient() == null) { + return false; + } + + userSession.getAuthenticatedClientSessions().put(clientSessionEntity.getClientId(), clientSessAdapter); + return true; + } + private PersistentUserSessionAdapter toAdapter(PersistentUserSessionEntity entity) { RealmModel realm = session.realms().getRealm(entity.getRealmId()); if (realm == null) { // Realm has been deleted concurrently, ignore the entity @@ -329,18 +453,33 @@ private PersistentAuthenticatedClientSessionAdapter toAdapter(RealmModel realm, } @Override - public int getUserSessionsCount(boolean offline) { + public int getUserSessionsCount(RealmModel realm, boolean offline) { String offlineStr = offlineToString(offline); + String realmId = realm != null ? realm.getId() : null; Query query = em.createNamedQuery("findUserSessionsCount"); + query.setParameter("realmId", realmId); query.setParameter("offline", offlineStr); Number n = (Number) query.getSingleResult(); return n.intValue(); } @Override - public void close() { + public int getUserSessionsCount(RealmModel realm, ClientModel clientModel, boolean offline) { + String offlineStr = offlineToString(offline); + + Query query = em.createNamedQuery("findClientSessionsCountByClient"); + // Note, that realm is unused here, since the clientModel id already determines the offline user-sessions bound to a owning realm. + query.setParameter("offline", offlineStr); + query.setParameter("clientId", clientModel.getId()); + Number n = (Number) query.getSingleResult(); + return n.intValue(); + } + + @Override + public void close() { + // NOOP } private String offlineToString(boolean offline) { diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java index d8173c41ff3f..51115104caa8 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java @@ -30,7 +30,7 @@ * @author Marek Posolda */ @NamedQueries({ - @NamedQuery(name="deleteClientSessionsByRealm", query="delete from PersistentClientSessionEntity sess where sess.userSessionId IN (select u.userSessionId from PersistentUserSessionEntity u where u.realmId = :realmId)"), + @NamedQuery(name="deleteClientSessionsByRealm", query="delete from PersistentClientSessionEntity sess where sess.userSessionId IN (select u.userSessionId from PersistentUserSessionEntity u where u.realmId = :realmId) and sess.offline = coalesce(:offline, sess.offline) "), @NamedQuery(name="deleteClientSessionsByClient", query="delete from PersistentClientSessionEntity sess where sess.clientId = :clientId"), @NamedQuery(name="deleteClientSessionsByExternalClient", query="delete from PersistentClientSessionEntity sess where sess.clientStorageProvider = :clientStorageProvider and sess.externalClientId = :externalClientId"), @NamedQuery(name="deleteClientSessionsByClientStorageProvider", query="delete from PersistentClientSessionEntity sess where sess.clientStorageProvider = :clientStorageProvider"), @@ -38,7 +38,8 @@ @NamedQuery(name="deleteClientSessionsByUserSession", query="delete from PersistentClientSessionEntity sess where sess.userSessionId = :userSessionId and sess.offline = :offline"), @NamedQuery(name="deleteExpiredClientSessions", query="delete from PersistentClientSessionEntity sess where sess.userSessionId IN (select u.userSessionId from PersistentUserSessionEntity u where u.realmId = :realmId AND u.offline = :offline AND u.lastSessionRefresh < :lastSessionRefresh)"), @NamedQuery(name="findClientSessionsByUserSession", query="select sess from PersistentClientSessionEntity sess where sess.userSessionId=:userSessionId and sess.offline = :offline"), - @NamedQuery(name="findClientSessionsOrderedById", query="select sess from PersistentClientSessionEntity sess where sess.offline = :offline and sess.userSessionId >= :fromSessionId and sess.userSessionId <= :toSessionId order by sess.userSessionId") + @NamedQuery(name="findClientSessionsOrderedById", query="select sess from PersistentClientSessionEntity sess where sess.offline = :offline and sess.userSessionId >= :fromSessionId and sess.userSessionId <= :toSessionId order by sess.userSessionId"), + @NamedQuery(name="findClientSessionsCountByClient", query="select count(sess) from PersistentClientSessionEntity sess where sess.offline = :offline and sess.clientId = :clientId") }) @Table(name="OFFLINE_CLIENT_SESSION") @Entity diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java index e9f873798351..dee22cb87aaf 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java @@ -32,7 +32,7 @@ * @author Marek Posolda */ @NamedQueries({ - @NamedQuery(name="deleteUserSessionsByRealm", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId"), + @NamedQuery(name="deleteUserSessionsByRealm", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId and sess.offline = coalesce(:offline, sess.offline) "), @NamedQuery(name="deleteUserSessionsByUser", query="delete from PersistentUserSessionEntity sess where sess.userId = :userId"), @NamedQuery(name="deleteExpiredUserSessions", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId AND sess.offline = :offline AND sess.lastSessionRefresh < :lastSessionRefresh"), @NamedQuery(name="updateUserSessionLastSessionRefresh", query="update PersistentUserSessionEntity sess set lastSessionRefresh = :lastSessionRefresh where sess.realmId = :realmId" + @@ -40,7 +40,24 @@ @NamedQuery(name="findUserSessionsCount", query="select count(sess) from PersistentUserSessionEntity sess where sess.offline = :offline"), @NamedQuery(name="findUserSessionsOrderedById", query="select sess from PersistentUserSessionEntity sess, RealmEntity realm where realm.id = sess.realmId AND sess.offline = :offline" + " AND sess.userSessionId > :lastSessionId" + - " order by sess.userSessionId") + " order by sess.userSessionId"), + @NamedQuery(name="findUserSession", query="select sess from PersistentUserSessionEntity sess where sess.offline = :offline" + + " AND sess.userSessionId = :userSessionId AND sess.realmId = :realmId"), + @NamedQuery(name="findUserSessionsByUserId", query="select sess from PersistentUserSessionEntity sess where sess.offline = :offline" + + " AND sess.realmId = :realmId AND sess.userId = :userId"), + @NamedQuery(name="findUserSessionsByClientId", query="SELECT sess FROM PersistentUserSessionEntity sess INNER JOIN PersistentClientSessionEntity clientSess " + + " ON sess.userSessionId = clientSess.userSessionId AND clientSess.clientId = :clientId WHERE sess.offline = :offline " + + " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId AND clientSess.clientId = :clientId"), + @NamedQuery(name="findUserSessionsCountsByClientId", query="SELECT clientSess.clientId, count(clientSess) " + + " FROM PersistentUserSessionEntity sess INNER JOIN PersistentClientSessionEntity clientSess " + + " ON sess.userSessionId = clientSess.userSessionId " + + // find all available offline user-session for all or specific clients in a realm + " AND clientSess.clientId = coalesce(:clientId, clientSess.clientId) " + + " WHERE sess.offline = :offline " + + " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId " + + // find all available offline user-session for all or specific clients in a realm + " AND clientSess.clientId = coalesce(:clientId, clientSess.clientId) " + + " GROUP BY clientSess.clientId") }) @Table(name="OFFLINE_USER_SESSION") diff --git a/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java b/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java index 26b12a3d1d31..00ba11dd2a72 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java @@ -27,6 +27,8 @@ import org.keycloak.models.UserSessionModel; import java.util.Collection; +import java.util.Collections; +import java.util.Map; import java.util.stream.Stream; /** @@ -84,6 +86,11 @@ public void removeClientSession(String userSessionId, String clientUUID, boolean } + @Override + public int removeUserSessions(RealmModel realm, Boolean offline) { + return 0; + } + @Override public void onRealmRemoved(RealmModel realm) { @@ -109,6 +116,20 @@ public void removeExpired(RealmModel realm) { } + public UserSessionModel loadUserSession(RealmModel realm, String userSessionId, boolean offline) { + return null; + } + + @Override + public Stream loadUserSessionsStream(RealmModel realm, ClientModel client, boolean offline, Integer firstResult, Integer maxResults) { + return Stream.empty(); + } + + @Override + public Stream loadUserSessionsStream(RealmModel realm, UserModel user, boolean offline, Integer firstResult, Integer maxResults) { + return Stream.empty(); + } + @Override public Stream loadUserSessionsStream(Integer firstResult, Integer maxResults, boolean offline, String lastUserSessionId) { @@ -116,7 +137,17 @@ public Stream loadUserSessionsStream(Integer firstResult, Inte } @Override - public int getUserSessionsCount(boolean offline) { + public int getUserSessionsCount(RealmModel realm, boolean offline) { + return 0; + } + + @Override + public int getUserSessionsCount(RealmModel realm, ClientModel clientModel, boolean offline) { return 0; } + + @Override + public Map getUserSessionsCountsByClients(RealmModel realm, boolean offline) { + return Collections.emptyMap(); + } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java b/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java index 3b7e4e0b44c2..835ee304a6b0 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java @@ -26,6 +26,7 @@ import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -56,6 +57,37 @@ public interface UserSessionPersisterProvider extends Provider { // Remove userSessions and clientSessions, which are expired void removeExpired(RealmModel realm); + /** + * Loads the user session with the given userSessionId. + * @param userSessionId + * @param offline + * @return + */ + UserSessionModel loadUserSession(RealmModel realm, String userSessionId, boolean offline); + + /** + * Loads the user sessions for the given {@link UserModel} in the given {@link RealmModel} if present. + * @param realm + * @param user + * @param offline + * @param firstResult + * @param maxResults + * @return + */ + Stream loadUserSessionsStream(RealmModel realm, UserModel user, boolean offline, Integer firstResult, Integer maxResults); + + /** + * Loads the user sessions for the given {@link ClientModel} in the given {@link RealmModel} if present. + * + * @param realm + * @param client + * @param offline + * @param firstResult + * @param maxResults + * @return + */ + Stream loadUserSessionsStream(RealmModel realm, ClientModel client, boolean offline, Integer firstResult, Integer maxResults); + /** * Called during startup. For each userSession, it loads also clientSessions * @deprecated Use {@link #loadUserSessionsStream(Integer, Integer, boolean, String) loadUserSessionsStream} instead. @@ -77,6 +109,53 @@ default List loadUserSessions(int firstResult, int maxResults, Stream loadUserSessionsStream(Integer firstResult, Integer maxResults, boolean offline, String lastUserSessionId); - int getUserSessionsCount(boolean offline); + /** + * Retrieves the count of user sessions for a given realm. + * + * If {@code realm} is {@literal null}, then all sessions are counted. + * + * @param realm + * @param offline + * @return + */ + int getUserSessionsCount(RealmModel realm, boolean offline); + + /** + * Retrieves the count of user sessions for all realms. + * + * @param offline + * @return + * + * @see #getUserSessionsCount(RealmModel, boolean) + */ + default int getUserSessionsCount(boolean offline) { + return getUserSessionsCount(null, offline); + } + + /** + * Retrieves the count of user client-sessions for the given client + * + * @param realm + * @param clientModel + * @param offline + * @return + */ + int getUserSessionsCount(RealmModel realm, ClientModel clientModel, boolean offline); + /** + * Returns a {@link Map} containing the number of user-sessions aggregated by client id for the given realm. + * @param realm + * @param offline + * @return the count {@link Map} with clientId as key and session count as value + */ + Map getUserSessionsCountsByClients(RealmModel realm, boolean offline); + + + /** + * Removes the sessions from the given realm. + * @param realm + * @param offline + * @return + */ + int removeUserSessions(RealmModel realm, Boolean offline); } From f10ea74cc83bb5d85f9021235d26d9f351c9d355 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 8 Feb 2021 00:23:15 +0100 Subject: [PATCH 2/7] KEYCLOAK-11019 Fix exception message for unsupported brokered offline user-session lookups --- .../sessions/infinispan/InfinispanUserSessionProvider.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 8aaecaacc98d..5f44334e3388 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 @@ -345,7 +345,7 @@ protected Stream getUserSessionsStream(RealmModel realm, UserS // fetch the offline user-sessions from the persistence provider UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class); - UserModel user = session.users().getUserById(realm, predicate.getUser()); + UserModel user = session.users().getUserById(realm, predicate.getUserId()); if (user != null) { return persister.loadUserSessionsStream(realm, user, offline, 0, null); } @@ -353,13 +353,13 @@ protected Stream getUserSessionsStream(RealmModel realm, UserS if (predicate.getBrokerSessionId() != null) { // TODO add support for offline user-session lookup by brokerSessionId // currently it is not possible to access the brokerSessionId in offline user-session in a database agnostic way - throw new ModelException("Dynamic database lookup for offline user-sessions by brokerSessionId is currently not supported."); + throw new ModelException("Dynamic database lookup for offline user-sessions by brokerSessionId is currently only supported for preloaded sessions."); } if (predicate.getBrokerUserId() != null) { // TODO add support for offline user-session lookup by brokerUserId // currently it is not possible to access the brokerUserId in offline user-session in a database agnostic way - throw new ModelException("Dynamic database lookup for offline user-sessions by brokerUserId is currently not supported."); + throw new ModelException("Dynamic database lookup for offline user-sessions by brokerUserId is currently only supported for preloaded sessions."); } } From 892bfc0763971ce087ac1fdef039292d460bc5c7 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 8 Feb 2021 00:23:25 +0100 Subject: [PATCH 3/7] KEYCLOAK-11019 Fix naming in UserSessionPredicate --- .../models/sessions/infinispan/stream/UserSessionPredicate.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java index ece20181684d..e0e7be264b6d 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/stream/UserSessionPredicate.java @@ -112,7 +112,7 @@ public UserSessionPredicate brokerUserId(String id) { * Returns the user id. * @return */ - public String getUser() { + public String getUserId() { return user; } From 3a7f471a016ad092d1ec11bab08cfb14c1650a7f Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 8 Feb 2021 00:23:52 +0100 Subject: [PATCH 4/7] KEYCLOAK-11019 Revise queries for offline user-session loading Removed redundant condition checking. We use the `coalesce` function to make the queries work for a specific realm / client or all, e.g.: ``` clientSess.clientId = coalesce(CAST(:clientId as text), clientSess.clientId) ``` Note that the explicit cast is required to treat null values as text to avoid confusing databases. --- .../jpa/session/PersistentClientSessionEntity.java | 2 +- .../jpa/session/PersistentUserSessionEntity.java | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java index 51115104caa8..2210407dc91f 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java @@ -30,7 +30,7 @@ * @author Marek Posolda */ @NamedQueries({ - @NamedQuery(name="deleteClientSessionsByRealm", query="delete from PersistentClientSessionEntity sess where sess.userSessionId IN (select u.userSessionId from PersistentUserSessionEntity u where u.realmId = :realmId) and sess.offline = coalesce(:offline, sess.offline) "), + @NamedQuery(name="deleteClientSessionsByRealm", query="delete from PersistentClientSessionEntity sess where sess.userSessionId IN (select u.userSessionId from PersistentUserSessionEntity u where u.realmId = :realmId) and sess.offline = coalesce(cast(:offline as text), sess.offline)"), @NamedQuery(name="deleteClientSessionsByClient", query="delete from PersistentClientSessionEntity sess where sess.clientId = :clientId"), @NamedQuery(name="deleteClientSessionsByExternalClient", query="delete from PersistentClientSessionEntity sess where sess.clientStorageProvider = :clientStorageProvider and sess.externalClientId = :externalClientId"), @NamedQuery(name="deleteClientSessionsByClientStorageProvider", query="delete from PersistentClientSessionEntity sess where sess.clientStorageProvider = :clientStorageProvider"), diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java index dee22cb87aaf..9087884fb28e 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java @@ -32,12 +32,12 @@ * @author Marek Posolda */ @NamedQueries({ - @NamedQuery(name="deleteUserSessionsByRealm", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId and sess.offline = coalesce(:offline, sess.offline) "), + @NamedQuery(name="deleteUserSessionsByRealm", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId and sess.offline = coalesce(cast(:offline as text), sess.offline)"), @NamedQuery(name="deleteUserSessionsByUser", query="delete from PersistentUserSessionEntity sess where sess.userId = :userId"), @NamedQuery(name="deleteExpiredUserSessions", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId AND sess.offline = :offline AND sess.lastSessionRefresh < :lastSessionRefresh"), @NamedQuery(name="updateUserSessionLastSessionRefresh", query="update PersistentUserSessionEntity sess set lastSessionRefresh = :lastSessionRefresh where sess.realmId = :realmId" + " AND sess.offline = :offline AND sess.userSessionId IN (:userSessionIds)"), - @NamedQuery(name="findUserSessionsCount", query="select count(sess) from PersistentUserSessionEntity sess where sess.offline = :offline"), + @NamedQuery(name="findUserSessionsCount", query="select count(sess) from PersistentUserSessionEntity sess where sess.offline = :offline and sess.realmId = coalesce(cast(:realmId AS text), sess.realmId)"), @NamedQuery(name="findUserSessionsOrderedById", query="select sess from PersistentUserSessionEntity sess, RealmEntity realm where realm.id = sess.realmId AND sess.offline = :offline" + " AND sess.userSessionId > :lastSessionId" + " order by sess.userSessionId"), @@ -47,16 +47,15 @@ " AND sess.realmId = :realmId AND sess.userId = :userId"), @NamedQuery(name="findUserSessionsByClientId", query="SELECT sess FROM PersistentUserSessionEntity sess INNER JOIN PersistentClientSessionEntity clientSess " + " ON sess.userSessionId = clientSess.userSessionId AND clientSess.clientId = :clientId WHERE sess.offline = :offline " + - " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId AND clientSess.clientId = :clientId"), + " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId"), @NamedQuery(name="findUserSessionsCountsByClientId", query="SELECT clientSess.clientId, count(clientSess) " + " FROM PersistentUserSessionEntity sess INNER JOIN PersistentClientSessionEntity clientSess " + " ON sess.userSessionId = clientSess.userSessionId " + // find all available offline user-session for all or specific clients in a realm - " AND clientSess.clientId = coalesce(:clientId, clientSess.clientId) " + + // cast(... as text) is required here to support null values + " AND clientSess.clientId = coalesce(CAST(:clientId as text), clientSess.clientId) " + " WHERE sess.offline = :offline " + " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId " + - // find all available offline user-session for all or specific clients in a realm - " AND clientSess.clientId = coalesce(:clientId, clientSess.clientId) " + " GROUP BY clientSess.clientId") }) From e5b76b1b842d65ef658f080730d6d7b911957eb8 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 8 Feb 2021 00:26:39 +0100 Subject: [PATCH 5/7] KEYCLOAK-11019 Add indices to speed up lazy offline user-session lookups --- .../META-INF/jpa-changelog-13.0.0.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml index 0868fd1f349f..a815f033d599 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml @@ -97,4 +97,23 @@ + + + + + + + + + + + + + + + + + + + From 10acb78f039b4401f0ab38b6064af6ea424bea97 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Sat, 3 Apr 2021 00:29:08 +0200 Subject: [PATCH 6/7] KEYCLOAK-11019 Revise indices according to analysis from code review --- .../src/main/resources/META-INF/jpa-changelog-13.0.0.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml index a815f033d599..7bf938652044 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml @@ -99,19 +99,19 @@ - + - - + + - + From a65756ae70f380b1c71091d63ae62e79ce10203c Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Mon, 10 May 2021 12:36:45 +0200 Subject: [PATCH 7/7] KEYCLOAK-11019 Add model tests + fix broken cluster tests + move migration changelog to 14 --- .../JpaUserSessionPersisterProvider.java | 18 +---- .../PersistentClientSessionEntity.java | 2 +- .../session/PersistentUserSessionEntity.java | 12 ++- .../META-INF/jpa-changelog-13.0.0.xml | 19 ----- .../META-INF/jpa-changelog-14.0.0.xml | 19 +++++ .../DisabledUserSessionPersisterProvider.java | 7 +- .../session/UserSessionPersisterProvider.java | 26 +----- testsuite/model/pom.xml | 10 +++ .../OfflineSessionPersistenceTest.java | 81 +++++++++++++++++++ .../session/UserSessionInitializerTest.java | 20 ----- 10 files changed, 120 insertions(+), 94 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java index a047fc77ed76..962085389702 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java @@ -127,17 +127,6 @@ public void removeUserSession(String userSessionId, boolean offline) { } } - @Override - public int removeUserSessions(RealmModel realm, Boolean offline) { - - String offlineStr = offline == null ? null : offlineToString(offline); - - return em.createNamedQuery("deleteUserSessionsByRealm") - .setParameter("realmId", realm.getId()) - .setParameter("offline", offlineStr) - .executeUpdate(); - } - @Override public void removeClientSession(String userSessionId, String clientUUID, boolean offline) { String offlineStr = offlineToString(offline); @@ -185,12 +174,10 @@ private List getClientSessionsByUserSession(Strin public void onRealmRemoved(RealmModel realm) { int deletedClientSessions = em.createNamedQuery("deleteClientSessionsByRealm") .setParameter("realmId", realm.getId()) - .setParameter("offline", null) .executeUpdate(); int deletedUserSessions = em.createNamedQuery("deleteUserSessionsByRealm") .setParameter("realmId", realm.getId()) - .setParameter("offline", null) .executeUpdate(); } @@ -270,7 +257,6 @@ public Map getUserSessionsCountsByClients(RealmModel realm, boolea query.setParameter("offline", offlineStr); query.setParameter("realmId", realm.getId()); - query.setParameter("clientId", null); Map offlineSessionsByClient = new HashMap<>(); @@ -453,12 +439,10 @@ private PersistentAuthenticatedClientSessionAdapter toAdapter(RealmModel realm, } @Override - public int getUserSessionsCount(RealmModel realm, boolean offline) { + public int getUserSessionsCount(boolean offline) { String offlineStr = offlineToString(offline); - String realmId = realm != null ? realm.getId() : null; Query query = em.createNamedQuery("findUserSessionsCount"); - query.setParameter("realmId", realmId); query.setParameter("offline", offlineStr); Number n = (Number) query.getSingleResult(); return n.intValue(); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java index 2210407dc91f..beac52b1abd3 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentClientSessionEntity.java @@ -30,7 +30,7 @@ * @author Marek Posolda */ @NamedQueries({ - @NamedQuery(name="deleteClientSessionsByRealm", query="delete from PersistentClientSessionEntity sess where sess.userSessionId IN (select u.userSessionId from PersistentUserSessionEntity u where u.realmId = :realmId) and sess.offline = coalesce(cast(:offline as text), sess.offline)"), + @NamedQuery(name="deleteClientSessionsByRealm", query="delete from PersistentClientSessionEntity sess where sess.userSessionId IN (select u.userSessionId from PersistentUserSessionEntity u where u.realmId = :realmId)"), @NamedQuery(name="deleteClientSessionsByClient", query="delete from PersistentClientSessionEntity sess where sess.clientId = :clientId"), @NamedQuery(name="deleteClientSessionsByExternalClient", query="delete from PersistentClientSessionEntity sess where sess.clientStorageProvider = :clientStorageProvider and sess.externalClientId = :externalClientId"), @NamedQuery(name="deleteClientSessionsByClientStorageProvider", query="delete from PersistentClientSessionEntity sess where sess.clientStorageProvider = :clientStorageProvider"), diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java index 9087884fb28e..6ed3e1bb954b 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java @@ -32,28 +32,26 @@ * @author Marek Posolda */ @NamedQueries({ - @NamedQuery(name="deleteUserSessionsByRealm", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId and sess.offline = coalesce(cast(:offline as text), sess.offline)"), + @NamedQuery(name="deleteUserSessionsByRealm", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId"), @NamedQuery(name="deleteUserSessionsByUser", query="delete from PersistentUserSessionEntity sess where sess.userId = :userId"), @NamedQuery(name="deleteExpiredUserSessions", query="delete from PersistentUserSessionEntity sess where sess.realmId = :realmId AND sess.offline = :offline AND sess.lastSessionRefresh < :lastSessionRefresh"), @NamedQuery(name="updateUserSessionLastSessionRefresh", query="update PersistentUserSessionEntity sess set lastSessionRefresh = :lastSessionRefresh where sess.realmId = :realmId" + " AND sess.offline = :offline AND sess.userSessionId IN (:userSessionIds)"), - @NamedQuery(name="findUserSessionsCount", query="select count(sess) from PersistentUserSessionEntity sess where sess.offline = :offline and sess.realmId = coalesce(cast(:realmId AS text), sess.realmId)"), + @NamedQuery(name="findUserSessionsCount", query="select count(sess) from PersistentUserSessionEntity sess where sess.offline = :offline"), @NamedQuery(name="findUserSessionsOrderedById", query="select sess from PersistentUserSessionEntity sess, RealmEntity realm where realm.id = sess.realmId AND sess.offline = :offline" + " AND sess.userSessionId > :lastSessionId" + " order by sess.userSessionId"), @NamedQuery(name="findUserSession", query="select sess from PersistentUserSessionEntity sess where sess.offline = :offline" + " AND sess.userSessionId = :userSessionId AND sess.realmId = :realmId"), @NamedQuery(name="findUserSessionsByUserId", query="select sess from PersistentUserSessionEntity sess where sess.offline = :offline" + - " AND sess.realmId = :realmId AND sess.userId = :userId"), + " AND sess.realmId = :realmId AND sess.userId = :userId ORDER BY sess.userSessionId"), @NamedQuery(name="findUserSessionsByClientId", query="SELECT sess FROM PersistentUserSessionEntity sess INNER JOIN PersistentClientSessionEntity clientSess " + " ON sess.userSessionId = clientSess.userSessionId AND clientSess.clientId = :clientId WHERE sess.offline = :offline " + - " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId"), + " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId ORDER BY sess.userSessionId"), @NamedQuery(name="findUserSessionsCountsByClientId", query="SELECT clientSess.clientId, count(clientSess) " + " FROM PersistentUserSessionEntity sess INNER JOIN PersistentClientSessionEntity clientSess " + " ON sess.userSessionId = clientSess.userSessionId " + - // find all available offline user-session for all or specific clients in a realm - // cast(... as text) is required here to support null values - " AND clientSess.clientId = coalesce(CAST(:clientId as text), clientSess.clientId) " + + // find all available offline user-session for all clients in a realm " WHERE sess.offline = :offline " + " AND sess.userSessionId = clientSess.userSessionId AND sess.realmId = :realmId " + " GROUP BY clientSess.clientId") diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml index 7bf938652044..0868fd1f349f 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-13.0.0.xml @@ -97,23 +97,4 @@ - - - - - - - - - - - - - - - - - - - diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml index 05f9858c4434..d9f7a3e702fe 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-14.0.0.xml @@ -17,6 +17,25 @@ --> + + + + + + + + + + + + + + + + + + + diff --git a/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java b/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java index 00ba11dd2a72..7293aaabac4a 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/models/session/DisabledUserSessionPersisterProvider.java @@ -86,11 +86,6 @@ public void removeClientSession(String userSessionId, String clientUUID, boolean } - @Override - public int removeUserSessions(RealmModel realm, Boolean offline) { - return 0; - } - @Override public void onRealmRemoved(RealmModel realm) { @@ -137,7 +132,7 @@ public Stream loadUserSessionsStream(Integer firstResult, Inte } @Override - public int getUserSessionsCount(RealmModel realm, boolean offline) { + public int getUserSessionsCount(boolean offline) { return 0; } diff --git a/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java b/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java index 835ee304a6b0..0dac1b9de61a 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/models/session/UserSessionPersisterProvider.java @@ -109,28 +109,14 @@ default List loadUserSessions(int firstResult, int maxResults, Stream loadUserSessionsStream(Integer firstResult, Integer maxResults, boolean offline, String lastUserSessionId); - /** - * Retrieves the count of user sessions for a given realm. - * - * If {@code realm} is {@literal null}, then all sessions are counted. - * - * @param realm - * @param offline - * @return - */ - int getUserSessionsCount(RealmModel realm, boolean offline); - /** * Retrieves the count of user sessions for all realms. * * @param offline * @return - * - * @see #getUserSessionsCount(RealmModel, boolean) + * */ - default int getUserSessionsCount(boolean offline) { - return getUserSessionsCount(null, offline); - } + int getUserSessionsCount(boolean offline); /** * Retrieves the count of user client-sessions for the given client @@ -150,12 +136,4 @@ default int getUserSessionsCount(boolean offline) { */ Map getUserSessionsCountsByClients(RealmModel realm, boolean offline); - - /** - * Removes the sessions from the given realm. - * @param realm - * @param offline - * @return - */ - int removeUserSessions(RealmModel realm, Boolean offline); } diff --git a/testsuite/model/pom.xml b/testsuite/model/pom.xml index 414248a19822..ccaaa11aee64 100644 --- a/testsuite/model/pom.xml +++ b/testsuite/model/pom.xml @@ -26,6 +26,7 @@ file:${project.build.directory}/dependency/log4j.properties true disabled + true @@ -137,6 +138,7 @@ ${keycloak.connectionsJpa.url} file:${project.build.directory}/test-classes/log4j.properties ${keycloak.profile.feature.map_storage} + ${keycloak.userSessions.infinispan.preloadOfflineSessionsFromDatabase} @@ -178,6 +180,14 @@ + + jpa+infinispan-sessions-preloading-disabled + + Infinispan,Jpa + false + + + jpa-federation+infinispan diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/OfflineSessionPersistenceTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/OfflineSessionPersistenceTest.java index b625a31e3c35..84c1efff7d97 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/OfflineSessionPersistenceTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/OfflineSessionPersistenceTest.java @@ -26,6 +26,9 @@ import org.keycloak.models.UserProvider; import org.keycloak.models.UserSessionModel; import org.keycloak.models.UserSessionProvider; +import org.keycloak.models.session.UserSessionPersisterProvider; +import org.keycloak.models.sessions.infinispan.InfinispanUserSessionProvider; +import org.keycloak.models.sessions.infinispan.InfinispanUserSessionProviderFactory; import org.keycloak.services.managers.RealmManager; import org.keycloak.testsuite.model.KeycloakModelTest; import org.keycloak.testsuite.model.RequireProvider; @@ -33,6 +36,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Random; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; @@ -42,6 +46,7 @@ import org.hamcrest.Matchers; import org.junit.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; /** * @@ -190,6 +195,82 @@ public void testPersistenceMultipleNodesClientSessionsAtRandomNode() throws Inte assertOfflineSessionsExist(realmId, offlineSessionIds); } + @Test + @RequireProvider(UserSessionPersisterProvider.class) + @RequireProvider(value = UserSessionProvider.class, only = InfinispanUserSessionProviderFactory.PROVIDER_ID) + public void testOfflineSessionLoadingAfterCacheRemoval() { + List offlineSessionIds = createOfflineSessions(realmId, userIds); + assertOfflineSessionsExist(realmId, offlineSessionIds); + + // Simulate server restart + reinitializeKeycloakSessionFactory(); + assertOfflineSessionsExist(realmId, offlineSessionIds); + + // remove sessions from the cache + withRealm(realmId, (session, realm) -> { + // Delete local user cache (persisted sessions are still kept) + UserSessionProvider provider = session.getProvider(UserSessionProvider.class); + // Remove in-memory representation of the offline sessions + ((InfinispanUserSessionProvider) provider).removeLocalUserSessions(realm.getId(), true); + + return null; + }); + + // assert sessions are lazily loaded from DB + assertOfflineSessionsExist(realmId, offlineSessionIds); + } + + @Test + @RequireProvider(UserSessionPersisterProvider.class) + @RequireProvider(value = UserSessionProvider.class, only = InfinispanUserSessionProviderFactory.PROVIDER_ID) + public void testLazyClientSessionStatsFetching() { + List clientIds = withRealm(realmId, (session, realm) -> IntStream.range(0, 5) + .mapToObj(cid -> session.clients().addClient(realm, "client-" + cid)) + .map(ClientModel::getId) + .collect(Collectors.toList())); + + List offlineSessionIds = createOfflineSessions(realmId, userIds); + assertOfflineSessionsExist(realmId, offlineSessionIds); + + Random r = new Random(); + offlineSessionIds.stream().forEach(offlineSessionId -> createOfflineClientSession(offlineSessionId, clientIds.get(r.nextInt(5)))); + + // Simulate server restart + reinitializeKeycloakSessionFactory(); + + // load active client sessions stats from DB + Map sessionStats = withRealm(realmId, (session, realm) -> session.sessions().getActiveClientSessionStats(realm, true)); + + long client1SessionCount = sessionStats.get(clientIds.get(0)); + int clientSessionsCount = sessionStats.values().stream().reduce(0l, Long::sum).intValue(); + assertThat(clientSessionsCount, Matchers.is(USER_COUNT * OFFLINE_SESSION_COUNT_PER_USER)); + + // Simulate server restart + reinitializeKeycloakSessionFactory(); + + long actualClient1SessionCount = withRealm(realmId, (session, realm) -> { + ClientModel client = realm.getClientById(clientIds.get(0)); + return session.sessions().getOfflineSessionsCount(realm, client); + }); + assertThat(actualClient1SessionCount, Matchers.is(client1SessionCount)); + } + + @Test + @RequireProvider(UserSessionPersisterProvider.class) + @RequireProvider(value = UserSessionProvider.class, only = InfinispanUserSessionProviderFactory.PROVIDER_ID) + public void testLazyOfflineUserSessionFetching() { + List offlineSessionIds = createOfflineSessions(realmId, userIds); + assertOfflineSessionsExist(realmId, offlineSessionIds); + + // Simulate server restart + reinitializeKeycloakSessionFactory(); + + List actualOfflineSessionIds = withRealm(realmId, (session, realm) -> session.users().getUsersStream(realm).flatMap(user -> + session.sessions().getOfflineUserSessionsStream(realm, user)).map(UserSessionModel::getId).collect(Collectors.toList())); + + assertThat(actualOfflineSessionIds, containsInAnyOrder(offlineSessionIds.toArray())); + } + private String createOfflineClientSession(String offlineUserSessionId, String clientId) { return withRealm(realmId, (session, realm) -> { UserSessionModel offlineUserSession = session.sessions().getOfflineUserSession(realm, offlineUserSessionId); diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionInitializerTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionInitializerTest.java index 78eb5da860c7..04db8f860344 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionInitializerTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionInitializerTest.java @@ -20,7 +20,6 @@ import org.junit.Assert; import org.junit.Test; import org.keycloak.common.util.Time; -import org.keycloak.connections.infinispan.InfinispanConnectionProvider; import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; @@ -181,25 +180,6 @@ private String[] createSessionsInPersisterOnly() { if (provider instanceof InfinispanUserSessionProvider) { // Remove in-memory representation of the offline sessions ((InfinispanUserSessionProvider) provider).removeLocalUserSessions(realm.getId(), true); - - // Clear ispn cache to ensure initializerState is removed as well - InfinispanConnectionProvider infinispan = session.getProvider(InfinispanConnectionProvider.class); - if (infinispan != null) { - infinispan.getCache(InfinispanConnectionProvider.WORK_CACHE_NAME).clear(); - } - } - }); - - inComittedTransaction(session -> { - // This is only valid in infinispan provider where the offline session is loaded upon start and never reloaded - UserSessionProvider provider = session.getProvider(UserSessionProvider.class); - if (provider instanceof InfinispanUserSessionProvider) { - RealmModel realm = session.realms().getRealm(realmId); - - ClientModel testApp = realm.getClientByClientId("test-app"); - ClientModel thirdparty = realm.getClientByClientId("third-party"); - assertThat("Count of offline sessions for client 'test-app'", session.sessions().getOfflineSessionsCount(realm, testApp), is((long) 0)); - assertThat("Count of offline sessions for client 'third-party'", session.sessions().getOfflineSessionsCount(realm, thirdparty), is((long) 0)); } });