From 0f86427dd0f408e772051d308806f4552640d0b6 Mon Sep 17 00:00:00 2001 From: Michal Hajas Date: Wed, 29 Jun 2022 16:45:36 +0200 Subject: [PATCH] Make user->client sessions relationship consistent Closes #12817 --- .../HotRodMapStorageProviderFactory.java | 9 +- .../IckleQueryMapModelCriteriaBuilder.java | 3 +- ...otRodAuthenticatedClientSessionEntity.java | 70 ++------- .../userSession/HotRodUserSessionEntity.java | 45 +++++- .../HotRodUserSessionMapStorage.java | 62 -------- .../src/main/resources/config/cacheConfig.xml | 9 +- .../src/main/resources/config/infinispan.xml | 9 +- ...ncurrentHashMapStorageProviderFactory.java | 17 +-- .../map/storage/chm/MapFieldPredicates.java | 10 +- .../UserSessionCascadeRemovalTransaction.java | 72 ---------- .../UserSessionConcurrentHashMapStorage.java | 56 -------- ...stractAuthenticatedClientSessionModel.java | 4 +- .../MapAuthenticatedClientSessionAdapter.java | 11 +- .../MapAuthenticatedClientSessionEntity.java | 3 - .../userSession/MapUserSessionAdapter.java | 107 ++++++++------ .../map/userSession/MapUserSessionEntity.java | 54 ++++++- .../userSession/MapUserSessionProvider.java | 134 ++++-------------- .../MapUserSessionProviderFactory.java | 62 ++------ .../models/UserSessionProviderFactory.java | 2 +- .../AuthenticatedClientSessionModel.java | 9 -- .../resources/META-INF/keycloak-server.json | 5 +- .../src/main/resources/hotrod/infinispan.xml | 9 +- .../model/parameters/HotRodMapStorage.java | 3 +- .../model/parameters/JpaMapStorage.java | 3 +- .../model/parameters/LdapMapStorage.java | 3 +- .../session/UserSessionProviderModelTest.java | 48 ++++--- 26 files changed, 259 insertions(+), 560 deletions(-) delete mode 100644 model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionMapStorage.java delete mode 100644 model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionCascadeRemovalTransaction.java delete mode 100644 model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/HotRodMapStorageProviderFactory.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/HotRodMapStorageProviderFactory.java index f52e8033af..f0cd23f5bf 100644 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/HotRodMapStorageProviderFactory.java +++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/HotRodMapStorageProviderFactory.java @@ -98,7 +98,6 @@ import org.keycloak.models.map.storage.hotRod.user.HotRodUserEntityDelegate; import org.keycloak.models.map.storage.hotRod.user.HotRodUserFederatedIdentityEntityDelegate; import org.keycloak.models.map.storage.hotRod.userSession.HotRodAuthenticatedClientSessionEntityDelegate; import org.keycloak.models.map.storage.hotRod.userSession.HotRodUserSessionEntityDelegate; -import org.keycloak.models.map.storage.hotRod.userSession.HotRodUserSessionMapStorage; import org.keycloak.models.map.user.MapUserConsentEntity; import org.keycloak.models.map.user.MapUserCredentialEntity; import org.keycloak.models.map.user.MapUserEntity; @@ -113,7 +112,6 @@ import java.util.concurrent.ConcurrentHashMap; public class HotRodMapStorageProviderFactory implements AmphibianProviderFactory, MapStorageProviderFactory, EnvironmentDependentProviderFactory { public static final String PROVIDER_ID = "hotrod"; - private static final Logger LOG = Logger.getLogger(HotRodMapStorageProviderFactory.class); private final Map, HotRodMapStorage> storages = new ConcurrentHashMap<>(); @@ -176,7 +174,6 @@ public class HotRodMapStorageProviderFactory implements AmphibianProviderFactory } public & AbstractEntity, M> HotRodMapStorage getHotRodStorage(KeycloakSession session, Class modelType, MapStorageProviderFactory.Flag... flags) { - if (modelType == UserSessionModel.class) getHotRodStorage(session, AuthenticatedClientSessionModel.class, flags); return storages.computeIfAbsent(modelType, c -> createHotRodStorage(session, modelType, flags)); } @@ -184,11 +181,7 @@ public class HotRodMapStorageProviderFactory implements AmphibianProviderFactory HotRodConnectionProvider connectionProvider = session.getProvider(HotRodConnectionProvider.class); HotRodEntityDescriptor entityDescriptor = (HotRodEntityDescriptor) getEntityDescriptor(modelType); - if (modelType == UserSessionModel.class) { - HotRodMapStorage clientSessionStore = getHotRodStorage(session, AuthenticatedClientSessionModel.class); - return new HotRodUserSessionMapStorage(clientSessionStore, connectionProvider.getRemoteCache(entityDescriptor.getCacheName()), StringKeyConverter.StringKey.INSTANCE, entityDescriptor, CLONER); - - } else if (modelType == ActionTokenValueModel.class) { + if (modelType == ActionTokenValueModel.class) { return new SingleUseObjectHotRodMapStorage(connectionProvider.getRemoteCache(entityDescriptor.getCacheName()), StringKeyConverter.StringKey.INSTANCE, entityDescriptor, CLONER); } return new HotRodMapStorage<>(connectionProvider.getRemoteCache(entityDescriptor.getCacheName()), StringKeyConverter.StringKey.INSTANCE, entityDescriptor, CLONER); diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java index ce523ed089..5295a71762 100644 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java +++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryMapModelCriteriaBuilder.java @@ -80,8 +80,7 @@ public class IckleQueryMapModelCriteriaBuilder> notes; - @ProtoField(number = 12) + @ProtoField(number = 10) public String currentRefreshToken; - @ProtoField(number = 13) + @ProtoField(number = 11) public Integer currentRefreshTokenUseCount; - @ProtoDoc("@Field(index = Index.YES, store = Store.YES)") - @ProtoField(number = 14) + @ProtoField(number = 12) public Boolean offline; - public static abstract class AbstractHotRodAuthenticatedClientSessionEntityDelegate extends UpdatableHotRodEntityDelegateImpl implements MapAuthenticatedClientSessionEntity { - - @Override - public String getId() { - return getHotRodEntity().id; - } - - @Override - public void setId(String id) { - HotRodAuthenticatedClientSessionEntity entity = getHotRodEntity(); - if (entity.id != null) throw new IllegalStateException("Id cannot be changed"); - entity.id = id; - entity.updated |= id != null; - } - } - @Override public boolean equals(Object o) { return HotRodAuthenticatedClientSessionEntityDelegate.entityEquals(this, o); diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionEntity.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionEntity.java index 00050b13d2..60339e745d 100644 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionEntity.java +++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionEntity.java @@ -25,12 +25,17 @@ import org.keycloak.models.UserSessionModel; import org.keycloak.models.map.annotations.GenerateHotRodEntityImplementation; import org.keycloak.models.map.annotations.IgnoreForEntityImplementationGenerator; import org.keycloak.models.map.storage.hotRod.authorization.HotRodResourceServerEntity; +import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.models.map.storage.hotRod.common.AbstractHotRodEntity; import org.keycloak.models.map.storage.hotRod.common.CommonPrimitivesProtoSchemaInitializer; import org.keycloak.models.map.storage.hotRod.common.HotRodStringPair; import org.keycloak.models.map.storage.hotRod.common.UpdatableHotRodEntityDelegateImpl; +import org.keycloak.models.map.userSession.MapAuthenticatedClientSessionEntity; import org.keycloak.models.map.userSession.MapUserSessionEntity; +import java.util.Collections; +import java.util.Objects; +import java.util.Optional; import java.util.Set; @GenerateHotRodEntityImplementation( @@ -48,7 +53,8 @@ public class HotRodUserSessionEntity extends AbstractHotRodEntity { @AutoProtoSchemaBuilder( includeClasses = { - HotRodUserSessionEntity.class + HotRodUserSessionEntity.class, + HotRodAuthenticatedClientSessionEntity.class, }, schemaFilePath = "proto/", schemaPackageName = CommonPrimitivesProtoSchemaInitializer.HOT_ROD_ENTITY_PACKAGE, @@ -113,7 +119,7 @@ public class HotRodUserSessionEntity extends AbstractHotRodEntity { @ProtoDoc("@Field(index = Index.YES, store = Store.YES)") @ProtoField(number = 16) - public Set authenticatedClientSessions; + public Set authenticatedClientSessions; @ProtoDoc("@Field(index = Index.YES, store = Store.YES)") @ProtoField(number = 17) @@ -145,6 +151,41 @@ public class HotRodUserSessionEntity extends AbstractHotRodEntity { throw new IllegalArgumentException("Transient session should not be stored in the HotRod."); } } + + @Override + public boolean isUpdated() { + return getHotRodEntity().updated + || Optional.ofNullable(getAuthenticatedClientSessions()).orElseGet(Collections::emptySet).stream().anyMatch(UpdatableEntity::isUpdated); + } + + @Override + public void clearUpdatedFlag() { + getHotRodEntity().updated = false; + Optional.ofNullable(getAuthenticatedClientSessions()).orElseGet(Collections::emptySet).forEach(UpdatableEntity::clearUpdatedFlag); + } + + @Override + public Optional getAuthenticatedClientSession(String clientUUID) { + Set acss = getHotRodEntity().authenticatedClientSessions; + if (acss == null || acss.isEmpty()) return Optional.empty(); + + return acss.stream().filter(acs -> Objects.equals(acs.clientId, clientUUID)).findFirst().map(HotRodAuthenticatedClientSessionEntityDelegate::new); + } + + @Override + public Boolean removeAuthenticatedClientSession(String clientUUID) { + Set acss = getHotRodEntity().authenticatedClientSessions; + boolean removed = acss != null && acss.removeIf(uc -> Objects.equals(uc.clientId, clientUUID)); + getHotRodEntity().updated |= removed; + return removed; + } + + @Override + public void clearAuthenticatedClientSessions() { + HotRodUserSessionEntity entity = getHotRodEntity(); + entity.updated = entity.authenticatedClientSessions != null; + entity.authenticatedClientSessions = null; + } } @Override diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionMapStorage.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionMapStorage.java deleted file mode 100644 index 7d4cd1d24f..0000000000 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/userSession/HotRodUserSessionMapStorage.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2022 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.map.storage.hotRod.userSession; - -import org.infinispan.client.hotrod.RemoteCache; -import org.keycloak.models.AuthenticatedClientSessionModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.UserSessionModel; -import org.keycloak.models.map.common.DeepCloner; -import org.keycloak.models.map.common.StringKeyConverter; -import org.keycloak.models.map.storage.MapKeycloakTransaction; -import org.keycloak.models.map.storage.chm.MapFieldPredicates; -import org.keycloak.models.map.storage.chm.MapModelCriteriaBuilder; -import org.keycloak.models.map.storage.chm.UserSessionCascadeRemovalTransaction; -import org.keycloak.models.map.storage.hotRod.HotRodMapStorage; -import org.keycloak.models.map.storage.hotRod.common.HotRodEntityDescriptor; -import org.keycloak.storage.SearchableModelField; - -import java.util.Map; - -public class HotRodUserSessionMapStorage extends HotRodMapStorage { - - private final HotRodMapStorage hotRodClientSessionStore; - - public HotRodUserSessionMapStorage(HotRodMapStorage hotRodClientSessionStore, - RemoteCache remoteCache, - StringKeyConverter keyConverter, - HotRodEntityDescriptor storedEntityDescriptor, - DeepCloner cloner - ) { - super(remoteCache, keyConverter, storedEntityDescriptor, cloner); - this.hotRodClientSessionStore = hotRodClientSessionStore; - } - - @Override - public MapKeycloakTransaction createTransaction(KeycloakSession session) { - MapKeycloakTransaction sessionTransaction = session.getAttribute("map-transaction-" + hashCode(), MapKeycloakTransaction.class); - - if (sessionTransaction == null) { - Map, MapModelCriteriaBuilder.UpdatePredicatesFunc> fieldPredicates = MapFieldPredicates.getPredicates((Class) storedEntityDescriptor.getModelTypeClass()); - sessionTransaction = new UserSessionCascadeRemovalTransaction<>(this, hotRodClientSessionStore.createTransaction(session), keyConverter, cloner, fieldPredicates); - session.setAttribute("map-transaction-" + hashCode(), sessionTransaction); - } - - return sessionTransaction; - } -} diff --git a/model/map-hot-rod/src/main/resources/config/cacheConfig.xml b/model/map-hot-rod/src/main/resources/config/cacheConfig.xml index d95ec65f48..71337664cb 100644 --- a/model/map-hot-rod/src/main/resources/config/cacheConfig.xml +++ b/model/map-hot-rod/src/main/resources/config/cacheConfig.xml @@ -75,15 +75,8 @@ kc.HotRodUserSessionEntity - kc.HotRodStringPair - - - - - - - kc.HotRodAuthenticatedClientSessionEntity + kc.HotRodStringPair diff --git a/model/map-hot-rod/src/main/resources/config/infinispan.xml b/model/map-hot-rod/src/main/resources/config/infinispan.xml index cd8dfc0a49..b3dbecf894 100644 --- a/model/map-hot-rod/src/main/resources/config/infinispan.xml +++ b/model/map-hot-rod/src/main/resources/config/infinispan.xml @@ -77,15 +77,8 @@ kc.HotRodUserSessionEntity - kc.HotRodStringPair - - - - - - - kc.HotRodAuthenticatedClientSessionEntity + kc.HotRodStringPair diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java index b97cded696..f8b96932c1 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java @@ -240,15 +240,7 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide LOG.debugf("Initializing new map storage: %s", mapName); ConcurrentHashMapStorage store; - if (modelType == UserSessionModel.class) { - ConcurrentHashMapStorage clientSessionStore = getStorage(AuthenticatedClientSessionModel.class); - store = new UserSessionConcurrentHashMapStorage(clientSessionStore, kc, CLONER) { - @Override - public String toString() { - return "ConcurrentHashMapStorage(" + mapName + suffix + ")"; - } - }; - } else if(modelType == ActionTokenValueModel.class) { + if(modelType == ActionTokenValueModel.class) { store = new SingleUseObjectConcurrentHashMapStorage(kc, CLONER) { @Override public String toString() { @@ -299,14 +291,7 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide /* From ConcurrentHashMapStorage.computeIfAbsent javadoc: * * "... the computation [...] must not attempt to update any other mappings of this map." - * - * For UserSessionModel, there is a separate clientSessionStore in this CHM implementation. Thus - * we cannot guarantee that this won't be the case e.g. for user and client sessions. Hence we need - * to prepare clientSessionStore outside computeIfAbsent, otherwise deadlock occurs. */ - if (modelType == UserSessionModel.class) { - getStorage(AuthenticatedClientSessionModel.class, flags); - } return (ConcurrentHashMapStorage) storages.computeIfAbsent(name, n -> loadMap(name, modelType, f)); } diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java index 33548104ef..d0f12f8bb4 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java @@ -83,7 +83,6 @@ import static org.keycloak.models.UserSessionModel.CORRESPONDING_SESSION_ID; */ public class MapFieldPredicates { - public static final Map, UpdatePredicatesFunc> CLIENT_SESSION_PREDICATES = basePredicates(AuthenticatedClientSessionModel.SearchableFields.ID); public static final Map, UpdatePredicatesFunc> CLIENT_PREDICATES = basePredicates(ClientModel.SearchableFields.ID); public static final Map, UpdatePredicatesFunc> CLIENT_SCOPE_PREDICATES = basePredicates(ClientScopeModel.SearchableFields.ID); public static final Map, UpdatePredicatesFunc> GROUP_PREDICATES = basePredicates(GroupModel.SearchableFields.ID); @@ -201,12 +200,6 @@ public class MapFieldPredicates { put(USER_SESSION_PREDICATES, UserSessionModel.SearchableFields.IS_OFFLINE, MapUserSessionEntity::isOffline); put(USER_SESSION_PREDICATES, UserSessionModel.SearchableFields.LAST_SESSION_REFRESH, MapUserSessionEntity::getLastSessionRefresh); - put(CLIENT_SESSION_PREDICATES, AuthenticatedClientSessionModel.SearchableFields.REALM_ID, MapAuthenticatedClientSessionEntity::getRealmId); - put(CLIENT_SESSION_PREDICATES, AuthenticatedClientSessionModel.SearchableFields.CLIENT_ID, MapAuthenticatedClientSessionEntity::getClientId); - put(CLIENT_SESSION_PREDICATES, AuthenticatedClientSessionModel.SearchableFields.USER_SESSION_ID, MapAuthenticatedClientSessionEntity::getUserSessionId); - put(CLIENT_SESSION_PREDICATES, AuthenticatedClientSessionModel.SearchableFields.IS_OFFLINE, MapAuthenticatedClientSessionEntity::isOffline); - put(CLIENT_SESSION_PREDICATES, AuthenticatedClientSessionModel.SearchableFields.TIMESTAMP, MapAuthenticatedClientSessionEntity::getTimestamp); - put(USER_LOGIN_FAILURE_PREDICATES, UserLoginFailureModel.SearchableFields.REALM_ID, MapUserLoginFailureEntity::getRealmId); put(USER_LOGIN_FAILURE_PREDICATES, UserLoginFailureModel.SearchableFields.USER_ID, MapUserLoginFailureEntity::getUserId); @@ -247,7 +240,6 @@ public class MapFieldPredicates { PREDICATES.put(PermissionTicket.class, AUTHZ_PERMISSION_TICKET_PREDICATES); PREDICATES.put(Policy.class, AUTHZ_POLICY_PREDICATES); PREDICATES.put(UserSessionModel.class, USER_SESSION_PREDICATES); - PREDICATES.put(AuthenticatedClientSessionModel.class, CLIENT_SESSION_PREDICATES); PREDICATES.put(UserLoginFailureModel.class, USER_LOGIN_FAILURE_PREDICATES); PREDICATES.put(Event.class, AUTH_EVENTS_PREDICATES); PREDICATES.put(AdminEvent.class, ADMIN_EVENTS_PREDICATES); @@ -539,7 +531,7 @@ public class MapFieldPredicates { private static MapModelCriteriaBuilder checkUserSessionContainsAuthenticatedClientSession(MapModelCriteriaBuilder mcb, Operator op, Object[] values) { String clientId = ensureEqSingleValue(UserSessionModel.SearchableFields.CLIENT_ID, "client_id", op, values); - Function getter = use -> (use.getAuthenticatedClientSession(clientId) != null); + Function getter = use -> (use.getAuthenticatedClientSession(clientId).isPresent()); return mcb.fieldCompare(Boolean.TRUE::equals, getter); } diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionCascadeRemovalTransaction.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionCascadeRemovalTransaction.java deleted file mode 100644 index dd11026b86..0000000000 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionCascadeRemovalTransaction.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2022 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.map.storage.chm; - -import org.keycloak.models.AuthenticatedClientSessionModel; -import org.keycloak.models.UserSessionModel; -import org.keycloak.models.map.common.AbstractEntity; -import org.keycloak.models.map.common.DeepCloner; -import org.keycloak.models.map.common.StringKeyConverter; -import org.keycloak.models.map.storage.MapKeycloakTransaction; -import org.keycloak.models.map.storage.ModelCriteriaBuilder; -import org.keycloak.models.map.storage.QueryParameters; -import org.keycloak.models.map.storage.criteria.DefaultModelCriteria; -import org.keycloak.models.map.userSession.MapAuthenticatedClientSessionEntity; -import org.keycloak.models.map.userSession.MapUserSessionEntity; -import org.keycloak.storage.SearchableModelField; - -import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; - -import static org.keycloak.models.map.storage.QueryParameters.withCriteria; - -public class UserSessionCascadeRemovalTransaction extends ConcurrentHashMapKeycloakTransaction { - - private final MapKeycloakTransaction clientSessionTr; - - public UserSessionCascadeRemovalTransaction(ConcurrentHashMapCrudOperations userSessionConcurrentHashMapStorage, - MapKeycloakTransaction clientSessionTr, - StringKeyConverter keyConverter, - DeepCloner cloner, - Map, - MapModelCriteriaBuilder.UpdatePredicatesFunc> fieldPredicates) { - super(userSessionConcurrentHashMapStorage, keyConverter, cloner, fieldPredicates); - this.clientSessionTr = clientSessionTr; - } - - @Override - public long delete(QueryParameters queryParameters) { - Set ids = read(queryParameters).map(AbstractEntity::getId).collect(Collectors.toSet()); - DefaultModelCriteria csMcb = DefaultModelCriteria.criteria() - .compare(AuthenticatedClientSessionModel.SearchableFields.USER_SESSION_ID, ModelCriteriaBuilder.Operator.IN, ids); - clientSessionTr.delete(withCriteria(csMcb)); - return super.delete(queryParameters); - } - - @Override - public boolean delete(String key) { - DefaultModelCriteria csMcb = DefaultModelCriteria.criteria() - .compare(AuthenticatedClientSessionModel.SearchableFields.USER_SESSION_ID, ModelCriteriaBuilder.Operator.EQ, key); - clientSessionTr.delete(withCriteria(csMcb)); - return super.delete(key); - } - -} diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java deleted file mode 100644 index 233247ac9a..0000000000 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2020 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.map.storage.chm; - -import org.keycloak.models.map.common.StringKeyConverter; -import org.keycloak.models.AuthenticatedClientSessionModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.UserSessionModel; -import org.keycloak.models.map.common.DeepCloner; -import org.keycloak.models.map.storage.MapKeycloakTransaction; -import org.keycloak.models.map.userSession.MapAuthenticatedClientSessionEntity; -import org.keycloak.models.map.userSession.MapUserSessionEntity; - -/** - * User session storage with a naive implementation of referential integrity in client to user session relation, restricted to - * ON DELETE CASCADE functionality. - * - * @author hmlnarik - */ -public class UserSessionConcurrentHashMapStorage extends ConcurrentHashMapStorage { - - private final ConcurrentHashMapStorage clientSessionStore; - - @SuppressWarnings("unchecked") - public UserSessionConcurrentHashMapStorage(ConcurrentHashMapStorage clientSessionStore, - StringKeyConverter keyConverter, DeepCloner cloner) { - super(UserSessionModel.class, keyConverter, cloner); - this.clientSessionStore = clientSessionStore; - } - - @Override - @SuppressWarnings("unchecked") - public MapKeycloakTransaction createTransaction(KeycloakSession session) { - MapKeycloakTransaction sessionTransaction = session.getAttribute("map-transaction-" + hashCode(), MapKeycloakTransaction.class); - - if (sessionTransaction == null) { - sessionTransaction = new UserSessionCascadeRemovalTransaction<>(this, clientSessionStore.createTransaction(session), clientSessionStore.getKeyConverter(), cloner, fieldPredicates); - session.setAttribute("map-transaction-" + hashCode(), sessionTransaction); - } - return sessionTransaction; - } -} diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/AbstractAuthenticatedClientSessionModel.java b/model/map/src/main/java/org/keycloak/models/map/userSession/AbstractAuthenticatedClientSessionModel.java index 367bcb6377..cc9dc1d0d3 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/AbstractAuthenticatedClientSessionModel.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/AbstractAuthenticatedClientSessionModel.java @@ -35,16 +35,14 @@ public abstract class AbstractAuthenticatedClientSessionModel implements Authent protected UserSessionModel userSession; protected final MapAuthenticatedClientSessionEntity entity; - public AbstractAuthenticatedClientSessionModel(KeycloakSession session, RealmModel realm, ClientModel client, + public AbstractAuthenticatedClientSessionModel(KeycloakSession session, RealmModel realm, UserSessionModel userSession, MapAuthenticatedClientSessionEntity entity) { Objects.requireNonNull(entity, "entity"); Objects.requireNonNull(realm, "realm"); - Objects.requireNonNull(client, "client"); Objects.requireNonNull(userSession, "userSession"); this.session = session; this.realm = realm; - this.client = client; this.userSession = userSession; this.entity = entity; } diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionAdapter.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionAdapter.java index b71447f733..ef437a9010 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionAdapter.java @@ -25,14 +25,16 @@ import org.keycloak.models.map.common.TimeAdapter; import java.util.Collections; import java.util.Map; +import static org.keycloak.models.map.userSession.SessionExpiration.setClientSessionExpiration; + /** * @author Martin Kanis */ public abstract class MapAuthenticatedClientSessionAdapter extends AbstractAuthenticatedClientSessionModel { - public MapAuthenticatedClientSessionAdapter(KeycloakSession session, RealmModel realm, ClientModel client, + public MapAuthenticatedClientSessionAdapter(KeycloakSession session, RealmModel realm, UserSessionModel userSession, MapAuthenticatedClientSessionEntity entity) { - super(session, realm, client, userSession, entity); + super(session, realm, userSession, entity); } @Override @@ -49,6 +51,9 @@ public abstract class MapAuthenticatedClientSessionAdapter extends AbstractAuthe @Override public void setTimestamp(int timestamp) { entity.setTimestamp(TimeAdapter.fromSecondsToMilliseconds(timestamp)); + + // whenever the timestamp is changed recompute the expiration time + setClientSessionExpiration(entity, realm, getClient()); } @Override @@ -123,7 +128,7 @@ public abstract class MapAuthenticatedClientSessionAdapter extends AbstractAuthe @Override public ClientModel getClient() { - return client; + return realm.getClientById(entity.getClientId()); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java index 8e747a419a..5a855ca7ff 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java @@ -69,9 +69,6 @@ public interface MapAuthenticatedClientSessionEntity extends AbstractEntity, Upd String getClientId(); void setClientId(String clientId); - String getUserSessionId(); - void setUserSessionId(String userSessionId); - String getAuthMethod(); void setAuthMethod(String authMethod); diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionAdapter.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionAdapter.java index 98728e84de..d3b729d875 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionAdapter.java @@ -26,18 +26,21 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.models.map.common.TimeAdapter; +import java.util.Collection; import java.util.Collections; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; + +import static org.keycloak.models.map.common.ExpirationUtils.isExpired; +import static org.keycloak.models.map.userSession.SessionExpiration.setUserSessionExpiration; /** * @author Martin Kanis */ -public abstract class MapUserSessionAdapter extends AbstractUserSessionModel { +public class MapUserSessionAdapter extends AbstractUserSessionModel { public MapUserSessionAdapter(KeycloakSession session, RealmModel realm, MapUserSessionEntity entity) { super(session, realm, entity); @@ -104,6 +107,9 @@ public abstract class MapUserSessionAdapter extends AbstractUserSessionModel { @Override public void setLastSessionRefresh(int seconds) { entity.setLastSessionRefresh(TimeAdapter.fromSecondsToMilliseconds(seconds)); + + // whenever the lastSessionRefresh is changed recompute the expiration time + setUserSessionExpiration(entity, realm); } @Override @@ -114,55 +120,74 @@ public abstract class MapUserSessionAdapter extends AbstractUserSessionModel { @Override public Map getAuthenticatedClientSessions() { - Map result = new HashMap<>(); - List removedClientUUIDS = new LinkedList<>(); - - Map authenticatedClientSessions = entity.getAuthenticatedClientSessions(); + Set authenticatedClientSessions = entity.getAuthenticatedClientSessions(); if (authenticatedClientSessions == null) { return Collections.emptyMap(); - } else { - // to avoid concurrentModificationException - authenticatedClientSessions = new HashMap<>(authenticatedClientSessions); } - authenticatedClientSessions.forEach((clientUUID, clientSessionId) -> { - ClientModel client = realm.getClientById(clientUUID); + return authenticatedClientSessions + .stream() + .filter(this::filterAndRemoveExpiredClientSessions) + .filter(this::matchingOfflineFlag) + .filter(this::filterAndRemoveClientSessionWithoutClient) + .collect(Collectors.toMap(MapAuthenticatedClientSessionEntity::getClientId, this::clientEntityToModel)); + } - if (client != null) { - AuthenticatedClientSessionModel clientSession = session.sessions() - .getClientSession(this, client, clientSessionId, isOffline()); - if (clientSession != null) { - result.put(clientUUID, clientSession); - } - } else { - removedClientUUIDS.add(clientUUID); + private AuthenticatedClientSessionModel clientEntityToModel(MapAuthenticatedClientSessionEntity clientSessionEntity) { + return new MapAuthenticatedClientSessionAdapter(session, realm, this, clientSessionEntity) { + @Override + public void detachFromUserSession() { + MapUserSessionAdapter.this.entity.removeAuthenticatedClientSession(entity.getClientId()); + this.userSession = null; } - }); + }; + } - removeAuthenticatedClientSessions(removedClientUUIDS); + public boolean filterAndRemoveExpiredClientSessions(MapAuthenticatedClientSessionEntity clientSession) { + if (isExpired(clientSession, false)) { + entity.removeAuthenticatedClientSession(clientSession.getClientId()); + return false; + } - return Collections.unmodifiableMap(result); + return true; + } + + public boolean filterAndRemoveClientSessionWithoutClient(MapAuthenticatedClientSessionEntity clientSession) { + ClientModel client = realm.getClientById(clientSession.getClientId()); + + if (client == null) { + entity.removeAuthenticatedClientSession(clientSession.getId()); + + // Filter out entities that doesn't have client + return false; + } + + // client session has client so we do not filter it out + return true; + } + + public boolean matchingOfflineFlag(MapAuthenticatedClientSessionEntity clientSession) { + Boolean isClientSessionOffline = clientSession.isOffline(); + + // If client session doesn't have offline flag default to false + if (isClientSessionOffline == null) return !isOffline(); + + return isOffline() == isClientSessionOffline; } @Override public AuthenticatedClientSessionModel getAuthenticatedClientSessionByClient(String clientUUID) { - String clientSessionId = entity.getAuthenticatedClientSession(clientUUID); - - if (clientSessionId == null) { - return null; - } - - ClientModel client = realm.getClientById(clientUUID); - - if (client != null) { - return session.sessions().getClientSession(this, client, clientSessionId, isOffline()); - } - - removeAuthenticatedClientSessions(Collections.singleton(clientUUID)); - - return null; + return entity.getAuthenticatedClientSession(clientUUID) + .filter(this::filterAndRemoveExpiredClientSessions) + .filter(this::matchingOfflineFlag) + .filter(this::filterAndRemoveClientSessionWithoutClient) + .map(this::clientEntityToModel) + .orElse(null); + } + @Override + public void removeAuthenticatedClientSessions(Collection removedClientUKS) { + removedClientUKS.forEach(entity::removeAuthenticatedClientSession); } - @Override public String getNote(String name) { @@ -226,7 +251,7 @@ public abstract class MapUserSessionAdapter extends AbstractUserSessionModel { if (correspondingSessionId != null) entity.setNote(CORRESPONDING_SESSION_ID, correspondingSessionId); - entity.setAuthenticatedClientSessions(null); + entity.clearAuthenticatedClientSessions(); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java index c37d11f53f..918e066219 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java @@ -18,12 +18,19 @@ package org.keycloak.models.map.userSession; import org.keycloak.models.UserSessionModel; import org.keycloak.models.map.annotations.GenerateEntityImplementations; +import org.keycloak.models.map.annotations.IgnoreForEntityImplementationGenerator; +import org.keycloak.models.map.client.MapProtocolMapperEntity; import org.keycloak.models.map.common.AbstractEntity; import org.keycloak.models.map.common.DeepCloner; import org.keycloak.models.map.common.ExpirableEntity; import org.keycloak.models.map.common.UpdatableEntity; +import java.util.Collections; import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; /** * @author Martin Kanis @@ -49,6 +56,44 @@ public interface MapUserSessionEntity extends AbstractEntity, UpdatableEntity, E this.id = id; this.updated |= id != null; } + + @Override + public boolean isUpdated() { + return this.updated + || Optional.ofNullable(getAuthenticatedClientSessions()).orElseGet(Collections::emptySet).stream().anyMatch(UpdatableEntity::isUpdated); + } + + @Override + public void clearUpdatedFlag() { + this.updated = false; + Optional.ofNullable(getAuthenticatedClientSessions()).orElseGet(Collections::emptySet).forEach(UpdatableEntity::clearUpdatedFlag); + } + + @Override + public Optional getAuthenticatedClientSession(String clientUUID) { + Set acss = getAuthenticatedClientSessions(); + if (acss == null || acss.isEmpty()) return Optional.empty(); + + return acss.stream().filter(acs -> Objects.equals(acs.getClientId(), clientUUID)).findFirst(); + } + + @Override + public Boolean removeAuthenticatedClientSession(String clientUUID) { + Set acss = getAuthenticatedClientSessions(); + boolean removed = acss != null && acss.removeIf(uc -> Objects.equals(uc.getClientId(), clientUUID)); + this.updated |= removed; + return removed; + } + + @Override + public void clearAuthenticatedClientSessions() { + Set acss = getAuthenticatedClientSessions(); + if (acss != null) { + acss.stream().map(MapAuthenticatedClientSessionEntity::getClientId) + .collect(Collectors.toSet()) + .forEach(this::removeAuthenticatedClientSession); + } + } } String getRealmId(); @@ -110,11 +155,12 @@ public interface MapUserSessionEntity extends AbstractEntity, UpdatableEntity, E UserSessionModel.State getState(); void setState(UserSessionModel.State state); - Map getAuthenticatedClientSessions(); - void setAuthenticatedClientSessions(Map authenticatedClientSessions); - String getAuthenticatedClientSession(String clientUUID); - void setAuthenticatedClientSession(String clientUUID, String clientSessionId); + Set getAuthenticatedClientSessions(); + Optional getAuthenticatedClientSession(String clientUUID); + void addAuthenticatedClientSession(MapAuthenticatedClientSessionEntity clientSession); Boolean removeAuthenticatedClientSession(String clientUUID); + @IgnoreForEntityImplementationGenerator + void clearAuthenticatedClientSessions(); Boolean isOffline(); void setOffline(Boolean offline); diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java index 50ee635859..a93400865e 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java @@ -32,6 +32,7 @@ import org.keycloak.models.map.storage.MapKeycloakTransaction; import org.keycloak.models.map.storage.MapStorage; import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator; import org.keycloak.models.map.storage.criteria.DefaultModelCriteria; +import org.keycloak.models.utils.KeycloakModelUtils; import java.util.Arrays; import java.util.Collection; @@ -63,25 +64,17 @@ public class MapUserSessionProvider implements UserSessionProvider { private static final Logger LOG = Logger.getLogger(MapUserSessionProvider.class); private final KeycloakSession session; protected final MapKeycloakTransaction userSessionTx; - protected final MapKeycloakTransaction clientSessionTx; /** * Storage for transient user sessions which lifespan is limited to one request. */ private final Map transientUserSessions = new HashMap<>(); - /** - * Storage for client sessions where parent is transient user session. Lifespan is limited to one request. - */ - private final Map transientClientSessions = new HashMap<>(); - public MapUserSessionProvider(KeycloakSession session, MapStorage userSessionStore, - MapStorage clientSessionStore) { + public MapUserSessionProvider(KeycloakSession session, MapStorage userSessionStore) { this.session = session; userSessionTx = userSessionStore.createTransaction(session); - clientSessionTx = clientSessionStore.createTransaction(session); session.getTransactionManager().enlistAfterCompletion(userSessionTx); - session.getTransactionManager().enlistAfterCompletion(clientSessionTx); } private Function userEntityToAdapterFunc(RealmModel realm) { @@ -96,55 +89,7 @@ public class MapUserSessionProvider implements UserSessionProvider { } return null; } else { - return new MapUserSessionAdapter(session, realm, origEntity) { - @Override - public void removeAuthenticatedClientSessions(Collection removedClientUKS) { - removedClientUKS.forEach(entity::removeAuthenticatedClientSession); - } - - @Override - public void setLastSessionRefresh(int lastSessionRefresh) { - entity.setLastSessionRefresh(TimeAdapter.fromSecondsToMilliseconds(lastSessionRefresh)); - // whenever the lastSessionRefresh is changed recompute the expiration time - setUserSessionExpiration(entity, realm); - } - }; - } - }; - } - - private Function clientEntityToAdapterFunc(RealmModel realm, - ClientModel client, - UserSessionModel userSession) { - // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return origEntity -> { - if (origEntity == null) return null; - if (isExpired(origEntity, false)) { - userSession.removeAuthenticatedClientSessions(Arrays.asList(origEntity.getClientId())); - // if a client session is found among transient ones we can skip call to store - if (transientClientSessions.remove(origEntity.getId()) == null) { - clientSessionTx.delete(origEntity.getId()); - } - return null; - } else { - return new MapAuthenticatedClientSessionAdapter(session, realm, client, userSession, origEntity) { - @Override - public void detachFromUserSession() { - this.userSession = null; - - // if a client session is found among transient ones we can skip call to store - if (transientClientSessions.remove(entity.getId()) == null) { - clientSessionTx.delete(entity.getId()); - } - } - - @Override - public void setTimestamp(int timestamp) { - entity.setTimestamp(TimeAdapter.fromSecondsToMilliseconds(timestamp)); - // whenever the timestamp is changed recompute the expiration time - setClientSessionExpiration(entity, realm, client); - } - }; + return new MapUserSessionAdapter(session, realm, origEntity); } }; } @@ -164,24 +109,22 @@ public class MapUserSessionProvider implements UserSessionProvider { throw new IllegalStateException("User session entity does not exist: " + userSession.getId()); } + if (userSessionEntity.getAuthenticatedClientSession(client.getId()).isPresent()) { + userSessionEntity.removeAuthenticatedClientSession(client.getId()); + } + MapAuthenticatedClientSessionEntity entity = createAuthenticatedClientSessionEntityInstance(null, userSession.getId(), realm.getId(), client.getId(), false); String started = entity.getTimestamp() != null ? String.valueOf(TimeAdapter.fromMilliSecondsToSeconds(entity.getTimestamp())) : String.valueOf(0); entity.setNote(AuthenticatedClientSessionModel.STARTED_AT_NOTE, started); setClientSessionExpiration(entity, realm, client); - if (TRANSIENT == userSessionEntity.getPersistenceState()) { - if (entity.getId() == null) { - entity.setId(UUID.randomUUID().toString()); - } - transientClientSessions.put(entity.getId(), entity); - } else { - entity = clientSessionTx.create(entity); - } + userSessionEntity.addAuthenticatedClientSession(entity); - userSessionEntity.setAuthenticatedClientSession(client.getId(), entity.getId()); - - return clientEntityToAdapterFunc(realm, client, userSession).apply(entity); + // We need to load the clientSession through userModel so we return an entity that is included within the + // transaction and also, so we not avoid all the checks present in the adapter, for example expiration + UserSessionModel userSessionModel = userEntityToAdapterFunc(realm).apply(userSessionEntity); + return userSessionModel == null ? null : userSessionModel.getAuthenticatedClientSessionByClient(client.getId()); } @Override @@ -190,29 +133,7 @@ public class MapUserSessionProvider implements UserSessionProvider { LOG.tracef("getClientSession(%s, %s, %s, %s)%s", userSession, client, clientSessionId, offline, getShortStackTrace()); - Objects.requireNonNull(userSession, "The provided user session cannot be null!"); - Objects.requireNonNull(client, "The provided client cannot be null!"); - if (clientSessionId == null) { - return null; - } - - MapAuthenticatedClientSessionEntity entity = transientClientSessions.get(clientSessionId); - - if (entity != null) { - return clientEntityToAdapterFunc(client.getRealm(), client, userSession).apply(entity); - } - - DefaultModelCriteria mcb = criteria(); - mcb = mcb.compare(AuthenticatedClientSessionModel.SearchableFields.ID, Operator.EQ, clientSessionId) - .compare(AuthenticatedClientSessionModel.SearchableFields.USER_SESSION_ID, Operator.EQ, userSession.getId()) - .compare(AuthenticatedClientSessionModel.SearchableFields.REALM_ID, Operator.EQ, userSession.getRealm().getId()) - .compare(AuthenticatedClientSessionModel.SearchableFields.CLIENT_ID, Operator.EQ, client.getId()) - .compare(AuthenticatedClientSessionModel.SearchableFields.IS_OFFLINE, Operator.EQ, offline); - - return clientSessionTx.read(withCriteria(mcb)) - .findFirst() - .map(clientEntityToAdapterFunc(client.getRealm(), client, userSession)) - .orElse(null); + return userSession.getAuthenticatedClientSessionByClient(client.getId()); } @Override @@ -492,16 +413,24 @@ public class MapUserSessionProvider implements UserSessionProvider { int currentTime = Time.currentTime(); clientSessionEntity.setNote(AuthenticatedClientSessionModel.STARTED_AT_NOTE, String.valueOf(currentTime)); clientSessionEntity.setTimestamp(Time.currentTimeMillis()); - setClientSessionExpiration(clientSessionEntity, clientSession.getRealm(), clientSession.getClient()); - clientSessionEntity = clientSessionTx.create(clientSessionEntity); + RealmModel realm = clientSession.getRealm(); + setClientSessionExpiration(clientSessionEntity, realm, clientSession.getClient()); - Optional userSessionEntity = getOfflineUserSessionEntityStream(clientSession.getRealm(), offlineUserSession.getId()).findFirst(); + Optional userSessionEntity = getOfflineUserSessionEntityStream(realm, offlineUserSession.getId()).findFirst(); if (userSessionEntity.isPresent()) { - userSessionEntity.get().setAuthenticatedClientSession(clientSession.getClient().getId(), clientSessionEntity.getId()); + MapUserSessionEntity userSession = userSessionEntity.get(); + String clientId = clientSession.getClient().getId(); + if (userSession.getAuthenticatedClientSession(clientId).isPresent()) { + userSession.removeAuthenticatedClientSession(clientId); + } + + userSession.addAuthenticatedClientSession(clientSessionEntity); + + UserSessionModel userSessionModel = userEntityToAdapterFunc(realm).apply(userSession); + return userSessionModel == null ? null : userSessionModel.getAuthenticatedClientSessionByClient(clientId); } - return clientEntityToAdapterFunc(clientSession.getRealm(), - clientSession.getClient(), offlineUserSession).apply(clientSessionEntity); + return null; } @Override @@ -582,14 +511,12 @@ public class MapUserSessionProvider implements UserSessionProvider { // Update timestamp to same value as userSession. LastSessionRefresh of userSession from DB will have correct value clientSession.setTimestamp(userSessionEntity.getLastSessionRefresh()); - - clientSession = clientSessionTx.create(clientSession); - userSessionEntity.setAuthenticatedClientSession(entry.getKey(), clientSession.getId()); + userSessionEntity.addAuthenticatedClientSession(clientSession); } return userSessionEntity; }) - .forEach(use -> userSessionTx.create(use)); + .forEach(userSessionTx::create); } @Override @@ -698,8 +625,7 @@ public class MapUserSessionProvider implements UserSessionProvider { private MapAuthenticatedClientSessionEntity createAuthenticatedClientSessionEntityInstance(String id, String userSessionId, String realmId, String clientId, boolean offline) { MapAuthenticatedClientSessionEntityImpl clientSessionEntity = new MapAuthenticatedClientSessionEntityImpl(); - clientSessionEntity.setId(id); - clientSessionEntity.setUserSessionId(userSessionId); + clientSessionEntity.setId(id == null ? KeycloakModelUtils.generateId() : id); clientSessionEntity.setRealmId(realmId); clientSessionEntity.setClientId(clientId); clientSessionEntity.setOffline(offline); diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java index fabb12e0b5..f37d3bdea3 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java @@ -20,6 +20,7 @@ import org.keycloak.Config.Scope; import org.keycloak.common.Profile; import org.keycloak.component.AmphibianProviderFactory; import org.keycloak.models.AuthenticatedClientSessionModel; +import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.RealmModel; @@ -27,6 +28,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.models.UserSessionProvider; import org.keycloak.models.UserSessionProviderFactory; +import org.keycloak.models.map.client.MapClientProvider; import org.keycloak.models.map.common.AbstractMapProviderFactory; import org.keycloak.models.map.storage.MapStorage; import org.keycloak.models.map.storage.MapStorageProvider; @@ -42,61 +44,15 @@ import static org.keycloak.models.utils.KeycloakModelUtils.getComponentFactory; /** * @author Martin Kanis */ -public class MapUserSessionProviderFactory implements AmphibianProviderFactory, UserSessionProviderFactory, EnvironmentDependentProviderFactory, InvalidationHandler { +public class MapUserSessionProviderFactory extends AbstractMapProviderFactory implements UserSessionProviderFactory, InvalidationHandler { - public static final String CONFIG_STORAGE_USER_SESSIONS = "storage-user-sessions"; - public static final String CONFIG_STORAGE_CLIENT_SESSIONS = "storage-client-sessions"; - public static final String PROVIDER_ID = AbstractMapProviderFactory.PROVIDER_ID; - - private final String uniqueKey = getClass().getName() + uniqueCounter.incrementAndGet(); - - private Scope storageConfigScopeUserSessions; - private Scope storageConfigScopeClientSessions; - - @Override - public String getId() { - return PROVIDER_ID; + public MapUserSessionProviderFactory() { + super(UserSessionModel.class, MapUserSessionProvider.class); } @Override - public void init(Scope config) { - storageConfigScopeUserSessions = config.scope(AbstractMapProviderFactory.CONFIG_STORAGE + "-user-sessions"); - storageConfigScopeClientSessions = config.scope(AbstractMapProviderFactory.CONFIG_STORAGE + "-client-sessions"); - } - - @Override - public void postInit(KeycloakSessionFactory factory) { - } - - @Override - public void close() { - AmphibianProviderFactory.super.close(); - } - - @Override - public void loadPersistentSessions(KeycloakSessionFactory sessionFactory, int maxErrors, int sessionsPerSegment) { - } - - @Override - @SuppressWarnings("unchecked") - public MapUserSessionProvider create(KeycloakSession session) { - MapUserSessionProvider provider = session.getAttribute(uniqueKey, MapUserSessionProvider.class); - - if (provider != null) return provider; - - MapStorageProviderFactory storageProviderFactoryUs = (MapStorageProviderFactory) getComponentFactory(session.getKeycloakSessionFactory(), - MapStorageProvider.class, storageConfigScopeUserSessions, MapStorageSpi.NAME); - final MapStorageProvider factoryUs = storageProviderFactoryUs.create(session); - MapStorage userSessionStore = factoryUs.getStorage(UserSessionModel.class); - - MapStorageProviderFactory storageProviderFactoryCs = (MapStorageProviderFactory) getComponentFactory(session.getKeycloakSessionFactory(), - MapStorageProvider.class, storageConfigScopeClientSessions, MapStorageSpi.NAME); - final MapStorageProvider factoryCs = storageProviderFactoryCs.create(session); - MapStorage clientSessionStore = factoryCs.getStorage(AuthenticatedClientSessionModel.class); - - provider = new MapUserSessionProvider(session, userSessionStore, clientSessionStore); - session.setAttribute(uniqueKey, provider); - return provider; + public MapUserSessionProvider createNew(KeycloakSession session) { + return new MapUserSessionProvider(session, getStorage(session)); } @Override @@ -112,7 +68,7 @@ public class MapUserSessionProviderFactory implements AmphibianProviderF } @Override - public boolean isSupported() { - return Profile.isFeatureEnabled(Profile.Feature.MAP_STORAGE); + public void loadPersistentSessions(KeycloakSessionFactory sessionFactory, int maxErrors, int sessionsPerSegment) { + } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/UserSessionProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/models/UserSessionProviderFactory.java index fb64e7b0b4..f05b5d2e6d 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/UserSessionProviderFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/models/UserSessionProviderFactory.java @@ -23,7 +23,7 @@ import org.keycloak.provider.ProviderFactory; * @author Bill Burke * @version $Revision: 1 $ */ -public interface UserSessionProviderFactory extends ProviderFactory { +public interface UserSessionProviderFactory extends ProviderFactory { // This is supposed to prefill all userSessions and clientSessions from userSessionPersister to the userSession infinispan/memory storage void loadPersistentSessions(KeycloakSessionFactory sessionFactory, final int maxErrors, final int sessionsPerSegment); diff --git a/server-spi/src/main/java/org/keycloak/models/AuthenticatedClientSessionModel.java b/server-spi/src/main/java/org/keycloak/models/AuthenticatedClientSessionModel.java index 4a7b3b2ae0..d069f93c19 100644 --- a/server-spi/src/main/java/org/keycloak/models/AuthenticatedClientSessionModel.java +++ b/server-spi/src/main/java/org/keycloak/models/AuthenticatedClientSessionModel.java @@ -28,15 +28,6 @@ import org.keycloak.storage.SearchableModelField; */ public interface AuthenticatedClientSessionModel extends CommonClientSessionModel { - class SearchableFields { - public static final SearchableModelField ID = new SearchableModelField<>("id", String.class); - public static final SearchableModelField REALM_ID = new SearchableModelField<>("realmId", String.class); - public static final SearchableModelField CLIENT_ID = new SearchableModelField<>("clientId", String.class); - public static final SearchableModelField USER_SESSION_ID = new SearchableModelField<>("userSessionId", String.class); - public static final SearchableModelField IS_OFFLINE = new SearchableModelField<>("isOffline", Boolean.class); - public static final SearchableModelField TIMESTAMP = new SearchableModelField<>("timestamp", Long.class); - } - String STARTED_AT_NOTE = "startedAt"; String getId(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json b/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json index 01fee7dee2..994914ec0f 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json @@ -124,10 +124,7 @@ "userSessions": { "provider": "${keycloak.userSession.provider:infinispan}", "map": { - "storage-user-sessions": { - "provider": "${keycloak.userSession.map.storage.provider:concurrenthashmap}" - }, - "storage-client-sessions": { + "storage": { "provider": "${keycloak.userSession.map.storage.provider:concurrenthashmap}" } } diff --git a/testsuite/model/src/main/resources/hotrod/infinispan.xml b/testsuite/model/src/main/resources/hotrod/infinispan.xml index 7e1e619716..0b9037d2ca 100644 --- a/testsuite/model/src/main/resources/hotrod/infinispan.xml +++ b/testsuite/model/src/main/resources/hotrod/infinispan.xml @@ -73,15 +73,8 @@ kc.HotRodUserSessionEntity - kc.HotRodStringPair - - - - - - - kc.HotRodAuthenticatedClientSessionEntity + kc.HotRodStringPair diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/HotRodMapStorage.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/HotRodMapStorage.java index a4509c362c..b9b496847e 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/HotRodMapStorage.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/HotRodMapStorage.java @@ -88,8 +88,7 @@ public class HotRodMapStorage extends KeycloakModelParameters { .spi(DeploymentStateSpi.NAME).provider(MapDeploymentStateProviderFactory.PROVIDER_ID).config(STORAGE_CONFIG, ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi(StoreFactorySpi.NAME).provider(MapAuthorizationStoreFactory.PROVIDER_ID).config(STORAGE_CONFIG, HotRodMapStorageProviderFactory.PROVIDER_ID) .spi("user").provider(MapUserProviderFactory.PROVIDER_ID).config(STORAGE_CONFIG, HotRodMapStorageProviderFactory.PROVIDER_ID) - .spi(UserSessionSpi.NAME).provider(MapUserSessionProviderFactory.PROVIDER_ID).config("storage-user-sessions.provider", HotRodMapStorageProviderFactory.PROVIDER_ID) - .config("storage-client-sessions.provider", HotRodMapStorageProviderFactory.PROVIDER_ID) + .spi(UserSessionSpi.NAME).provider(MapUserSessionProviderFactory.PROVIDER_ID).config(STORAGE_CONFIG, HotRodMapStorageProviderFactory.PROVIDER_ID) .spi(UserLoginFailureSpi.NAME).provider(MapUserLoginFailureProviderFactory.PROVIDER_ID).config(STORAGE_CONFIG, HotRodMapStorageProviderFactory.PROVIDER_ID) .spi("dblock").provider(NoLockingDBLockProviderFactory.PROVIDER_ID).config(STORAGE_CONFIG, ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi(EventStoreSpi.NAME).provider(MapUserSessionProviderFactory.PROVIDER_ID).config("storage-admin-events.provider", HotRodMapStorageProviderFactory.PROVIDER_ID) diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/JpaMapStorage.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/JpaMapStorage.java index 2a236ef6ef..619b5b9a5d 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/JpaMapStorage.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/JpaMapStorage.java @@ -101,8 +101,7 @@ public class JpaMapStorage extends KeycloakModelParameters { .spi(ActionTokenStoreSpi.NAME).provider(MapSingleUseObjectProviderFactory.PROVIDER_ID) .config(STORAGE_CONFIG, JpaMapStorageProviderFactory.PROVIDER_ID) .spi(SingleUseObjectSpi.NAME).provider(MapSingleUseObjectProviderFactory.PROVIDER_ID) .config(STORAGE_CONFIG, JpaMapStorageProviderFactory.PROVIDER_ID) .spi("publicKeyStorage").provider(MapPublicKeyStorageProviderFactory.PROVIDER_ID) .config(STORAGE_CONFIG, ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) - .spi(UserSessionSpi.NAME).provider(MapUserSessionProviderFactory.PROVIDER_ID) .config("storage-user-sessions.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) - .config("storage-client-sessions.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) + .spi(UserSessionSpi.NAME).provider(MapUserSessionProviderFactory.PROVIDER_ID) .config(STORAGE_CONFIG, ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi(EventStoreSpi.NAME).provider(MapUserSessionProviderFactory.PROVIDER_ID) .config("storage-admin-events.provider", JpaMapStorageProviderFactory.PROVIDER_ID) .config("storage-auth-events.provider", JpaMapStorageProviderFactory.PROVIDER_ID); } diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LdapMapStorage.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LdapMapStorage.java index 3a27110f40..26f101f509 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LdapMapStorage.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/parameters/LdapMapStorage.java @@ -97,8 +97,7 @@ public class LdapMapStorage extends KeycloakModelParameters { .spi(DeploymentStateSpi.NAME).config("map.storage.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi(StoreFactorySpi.NAME).config("map.storage.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi("user").config("map.storage.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) - .spi(UserSessionSpi.NAME).config("map.storage-user-sessions.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) - .spi(UserSessionSpi.NAME).config("map.storage-client-sessions.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) + .spi(UserSessionSpi.NAME).config("map.storage.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi(UserLoginFailureSpi.NAME).config("map.storage.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi("authorizationPersister").config("map.storage.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) .spi("authenticationSessions").config("map.storage.provider", ConcurrentHashMapStorageProviderFactory.PROVIDER_ID) diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionProviderModelTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionProviderModelTest.java index e522ddc9dd..64abe12708 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionProviderModelTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionProviderModelTest.java @@ -32,7 +32,9 @@ import org.keycloak.models.UserSessionModel; import org.keycloak.models.UserSessionProvider; import org.keycloak.models.map.userSession.MapUserSessionProvider; import org.keycloak.models.session.UserSessionPersisterProvider; +import org.keycloak.models.sessions.infinispan.InfinispanUserSessionProviderFactory; import org.keycloak.models.sessions.infinispan.changes.sessions.PersisterLastSessionRefreshStoreFactory; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ResetTimeOffsetEvent; import org.keycloak.testsuite.model.KeycloakModelTest; import org.keycloak.testsuite.model.RequireProvider; @@ -208,30 +210,37 @@ public class UserSessionProviderModelTest extends KeycloakModelTest { } @Test - public void testCascadeRemovalOfClientSessionOnUserSessionRemoval() { - UserSessionModel[] origSessions = inComittedTransaction(session -> { return createSessions(session, realmId); }); - - String testAppClientSessionId = withRealm(realmId, (session, realm) -> { - ClientModel testApp = realm.getClientByClientId("test-app"); - UserSessionModel userSessionToBeRemoved = session.sessions().getUserSession(realm, origSessions[0].getId()); - String returnValue = userSessionToBeRemoved.getAuthenticatedClientSessions().get(testApp.getId()).getId(); - - session.sessions().removeUserSession(realm, userSessionToBeRemoved); - return returnValue; - }); - - assertThat(withRealm(realmId, (session, realm) -> session.sessions().getClientSession(origSessions[0], realm.getClientByClientId("test-app"), testAppClientSessionId, false)), nullValue()); - } - - @Test - public void testClientSessionIsNotPersistedForTransientUserSession() { - Object[] transientUserSessionWithClientSessionId = inComittedTransaction(session -> { + public void testTransientUserSessionIsNotPersisted() { + String id = inComittedTransaction(session -> { RealmModel realm = session.realms().getRealm(realmId); - UserSessionModel userSession = session.sessions().createUserSession(null, realm, session.users().getUserByUsername(realm, "user1"), "user1", "127.0.0.1", "form", false, null, null, UserSessionModel.SessionPersistenceState.TRANSIENT); + UserSessionModel userSession = session.sessions().createUserSession(KeycloakModelUtils.generateId(), realm, session.users().getUserByUsername(realm, "user1"), "user1", "127.0.0.1", "form", false, null, null, UserSessionModel.SessionPersistenceState.TRANSIENT); ClientModel testApp = realm.getClientByClientId("test-app"); AuthenticatedClientSessionModel clientSession = session.sessions().createClientSession(realm, testApp, userSession); + // assert the client sessions are present + assertThat(session.sessions().getClientSession(userSession, testApp, clientSession.getId(), false), notNullValue()); + return userSession.getId(); + }); + + inComittedTransaction(session -> { + RealmModel realm = session.realms().getRealm(realmId); + UserSessionModel userSession = session.sessions().getUserSession(realm, id); + + // in new transaction transient session should not be present + assertThat(userSession, nullValue()); + }); + } + + @Test + @RequireProvider(value = UserSessionProvider.class, only = InfinispanUserSessionProviderFactory.PROVIDER_ID) + public void testClientSessionIsNotPersistedForTransientUserSession() { + Object[] transientUserSessionWithClientSessionId = inComittedTransaction(session -> { + RealmModel realm = session.realms().getRealm(realmId); + UserSessionModel userSession = session.sessions().createUserSession(null, realm, session.users().getUserByUsername(realm, "user1"), "user1", "127.0.0.1", "form", false, null, null, UserSessionModel.SessionPersistenceState.TRANSIENT); + ClientModel testApp = realm.getClientByClientId("test-app"); + AuthenticatedClientSessionModel clientSession = session.sessions().createClientSession(realm, testApp, userSession); + // assert the client sessions are present assertThat(session.sessions().getClientSession(userSession, testApp, clientSession.getId(), false), notNullValue()); Object[] result = new Object[2]; @@ -239,7 +248,6 @@ public class UserSessionProviderModelTest extends KeycloakModelTest { result[1] = clientSession.getId(); return result; }); - inComittedTransaction(session -> { RealmModel realm = session.realms().getRealm(realmId); ClientModel testApp = realm.getClientByClientId("test-app");