From df7ddbf9b31c5f65560a793c7d4c76526779c33c Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Mon, 24 Jan 2022 14:59:34 +0100 Subject: [PATCH] Added ModelIllegalStateException to handle lazy loading exception. Closes #9645 --- .../map/storage/jpa/JpaDelegateProvider.java | 52 +++++++++++++++++ .../models/map/storage/jpa/JpaRootEntity.java | 28 ++++++++++ .../JpaClientMapKeycloakTransaction.java | 5 +- .../delegate/JpaClientDelegateProvider.java | 20 +++---- .../jpa/client/entity/JpaClientEntity.java | 8 ++- .../role/JpaRoleMapKeycloakTransaction.java | 3 +- .../delegate/JpaRoleDelegateProvider.java | 20 +++---- .../jpa/role/entity/JpaRoleEntity.java | 8 ++- .../delegate/ClientModelLazyDelegate.java | 3 +- .../models/utils/ModelToRepresentation.java | 23 ++++++++ .../models/ModelIllegalStateException.java | 49 ++++++++++++++++ .../exportimport/util/ExportUtils.java | 56 +++++++++---------- .../managers/ResourceAdminManager.java | 16 ++++-- .../resources/admin/ClientsResource.java | 15 +++-- 14 files changed, 236 insertions(+), 70 deletions(-) create mode 100644 model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaDelegateProvider.java create mode 100644 model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaRootEntity.java create mode 100644 server-spi/src/main/java/org/keycloak/models/ModelIllegalStateException.java diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaDelegateProvider.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaDelegateProvider.java new file mode 100644 index 0000000000..7d0e8557f3 --- /dev/null +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaDelegateProvider.java @@ -0,0 +1,52 @@ +/* + * 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.jpa; + +import org.keycloak.models.ModelIllegalStateException; +import org.keycloak.models.map.common.AbstractEntity; + +/** + * Base class for all delegate providers for the JPA storage. + * + * Wraps the delegate so that it can be safely updated during lazy loading. + */ +public abstract class JpaDelegateProvider { + private T delegate; + + protected JpaDelegateProvider(T delegate) { + this.delegate = delegate; + } + + protected T getDelegate() { + return delegate; + } + + /** + * Validates the new entity. + * + * Will throw {@link ModelIllegalStateException} if the entity has been deleted or changed in the meantime. + */ + protected void setDelegate(T newDelegate) { + if (newDelegate == null) { + throw new ModelIllegalStateException("Unable to retrieve entity: " + delegate.getClass().getName() + "#" + delegate.getId()); + } + if (newDelegate.getVersion() != delegate.getVersion()) { + throw new ModelIllegalStateException("Version of entity changed between two loads: " + delegate.getClass().getName() + "#" + delegate.getId()); + } + this.delegate = newDelegate; + } +} diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaRootEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaRootEntity.java new file mode 100644 index 0000000000..948e933aee --- /dev/null +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaRootEntity.java @@ -0,0 +1,28 @@ +/* + * 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.jpa; + +/** + * Interface for all root entities in the JPA storage. + */ +public interface JpaRootEntity { + + /** + * Version of the JPA entity used for optimistic locking + */ + int getVersion(); +} diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/JpaClientMapKeycloakTransaction.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/JpaClientMapKeycloakTransaction.java index bd776b5ca6..19c35914fc 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/JpaClientMapKeycloakTransaction.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/JpaClientMapKeycloakTransaction.java @@ -74,8 +74,9 @@ public class JpaClientMapKeycloakTransaction extends JpaKeycloakTransaction impl CriteriaBuilder cb = em.getCriteriaBuilder(); CriteriaQuery query = cb.createQuery(JpaClientEntity.class); Root root = query.from(JpaClientEntity.class); - query.select(cb.construct(JpaClientEntity.class, - root.get("id"), + query.select(cb.construct(JpaClientEntity.class, + root.get("id"), + root.get("version"), root.get("entityVersion"), root.get("realmId"), root.get("clientId"), diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/delegate/JpaClientDelegateProvider.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/delegate/JpaClientDelegateProvider.java index 7c400e4ec3..bab3792d6e 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/delegate/JpaClientDelegateProvider.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/delegate/JpaClientDelegateProvider.java @@ -28,21 +28,21 @@ import org.keycloak.models.map.client.MapClientEntity; import org.keycloak.models.map.client.MapClientEntityFields; import org.keycloak.models.map.common.EntityField; import org.keycloak.models.map.common.delegate.DelegateProvider; +import org.keycloak.models.map.storage.jpa.JpaDelegateProvider; import org.keycloak.models.map.storage.jpa.client.entity.JpaClientEntity; -public class JpaClientDelegateProvider implements DelegateProvider { +public class JpaClientDelegateProvider extends JpaDelegateProvider implements DelegateProvider { - private JpaClientEntity delegate; private final EntityManager em; public JpaClientDelegateProvider(JpaClientEntity delegate, EntityManager em) { - this.delegate = delegate; + super(delegate); this.em = em; } @Override public MapClientEntity getDelegate(boolean isRead, Enum> field, Object... parameters) { - if (delegate.isMetadataInitialized()) return delegate; + if (getDelegate().isMetadataInitialized()) return getDelegate(); if (isRead) { if (field instanceof MapClientEntityFields) { switch ((MapClientEntityFields) field) { @@ -51,26 +51,26 @@ public class JpaClientDelegateProvider implements DelegateProvider query = cb.createQuery(JpaClientEntity.class); Root root = query.from(JpaClientEntity.class); root.fetch("attributes", JoinType.INNER); - query.select(root).where(cb.equal(root.get("id"), UUID.fromString(delegate.getId()))); + query.select(root).where(cb.equal(root.get("id"), UUID.fromString(getDelegate().getId()))); - delegate = em.createQuery(query).getSingleResult(); + setDelegate(em.createQuery(query).getSingleResult()); break; default: - delegate = em.find(JpaClientEntity.class, UUID.fromString(delegate.getId())); + setDelegate(em.find(JpaClientEntity.class, UUID.fromString(getDelegate().getId()))); } } else throw new IllegalStateException("Not a valid client field: " + field); } else { - delegate = em.find(JpaClientEntity.class, UUID.fromString(delegate.getId())); + setDelegate(em.find(JpaClientEntity.class, UUID.fromString(getDelegate().getId()))); } - return delegate; + return getDelegate(); } } diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java index 34e665574a..db22900e65 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java @@ -45,6 +45,8 @@ import org.keycloak.models.map.client.MapClientEntity.AbstractClientEntity; import org.keycloak.models.map.client.MapProtocolMapperEntity; import org.keycloak.models.map.common.DeepCloner; import static org.keycloak.models.map.storage.jpa.Constants.SUPPORTED_VERSION_CLIENT; + +import org.keycloak.models.map.storage.jpa.JpaRootEntity; import org.keycloak.models.map.storage.jpa.hibernate.jsonb.JsonbType; /** @@ -60,7 +62,7 @@ import org.keycloak.models.map.storage.jpa.hibernate.jsonb.JsonbType; ) }) @TypeDefs({@TypeDef(name = "jsonb", typeClass = JsonbType.class)}) -public class JpaClientEntity extends AbstractClientEntity implements Serializable { +public class JpaClientEntity extends AbstractClientEntity implements Serializable, JpaRootEntity { @Id @Column @@ -113,9 +115,10 @@ public class JpaClientEntity extends AbstractClientEntity implements Serializabl * Used by hibernate when calling cb.construct from read(QueryParameters) method. * It is used to select client without metadata(json) field. */ - public JpaClientEntity(UUID id, Integer entityVersion, String realmId, String clientId, + public JpaClientEntity(UUID id, int version, Integer entityVersion, String realmId, String clientId, String protocol, Boolean enabled) { this.id = id; + this.version = version; this.entityVersion = entityVersion; this.realmId = realmId; this.clientId = clientId; @@ -148,6 +151,7 @@ public class JpaClientEntity extends AbstractClientEntity implements Serializabl metadata.setEntityVersion(entityVersion); } + @Override public int getVersion() { return version; } diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/JpaRoleMapKeycloakTransaction.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/JpaRoleMapKeycloakTransaction.java index 5029cf4fa3..c14b740f0a 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/JpaRoleMapKeycloakTransaction.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/JpaRoleMapKeycloakTransaction.java @@ -75,7 +75,8 @@ public class JpaRoleMapKeycloakTransaction extends JpaKeycloakTransaction implem CriteriaQuery query = cb.createQuery(JpaRoleEntity.class); Root root = query.from(JpaRoleEntity.class); query.select(cb.construct(JpaRoleEntity.class, - root.get("id"), + root.get("id"), + root.get("version"), root.get("entityVersion"), root.get("realmId"), root.get("clientId"), diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java index 41d5f6604c..dfb41abe10 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/delegate/JpaRoleDelegateProvider.java @@ -26,21 +26,21 @@ import org.keycloak.models.map.common.EntityField; import org.keycloak.models.map.common.delegate.DelegateProvider; import org.keycloak.models.map.role.MapRoleEntity; import org.keycloak.models.map.role.MapRoleEntityFields; +import org.keycloak.models.map.storage.jpa.JpaDelegateProvider; import org.keycloak.models.map.storage.jpa.role.entity.JpaRoleEntity; -public class JpaRoleDelegateProvider implements DelegateProvider { +public class JpaRoleDelegateProvider extends JpaDelegateProvider implements DelegateProvider { - private JpaRoleEntity delegate; private final EntityManager em; public JpaRoleDelegateProvider(JpaRoleEntity delegate, EntityManager em) { - this.delegate = delegate; + super(delegate); this.em = em; } @Override public JpaRoleEntity getDelegate(boolean isRead, Enum> field, Object... parameters) { - if (delegate.isMetadataInitialized()) return delegate; + if (getDelegate().isMetadataInitialized()) return getDelegate(); if (isRead) { if (field instanceof MapRoleEntityFields) { switch ((MapRoleEntityFields) field) { @@ -49,26 +49,26 @@ public class JpaRoleDelegateProvider implements DelegateProvider case CLIENT_ID: case NAME: case DESCRIPTION: - return delegate; + return getDelegate(); case ATTRIBUTES: CriteriaBuilder cb = em.getCriteriaBuilder(); CriteriaQuery query = cb.createQuery(JpaRoleEntity.class); Root root = query.from(JpaRoleEntity.class); root.fetch("attributes", JoinType.INNER); - query.select(root).where(cb.equal(root.get("id"), UUID.fromString(delegate.getId()))); + query.select(root).where(cb.equal(root.get("id"), UUID.fromString(getDelegate().getId()))); - delegate = em.createQuery(query).getSingleResult(); + setDelegate(em.createQuery(query).getSingleResult()); break; default: - delegate = em.find(JpaRoleEntity.class, UUID.fromString(delegate.getId())); + setDelegate(em.find(JpaRoleEntity.class, UUID.fromString(getDelegate().getId()))); } } else throw new IllegalStateException("Not a valid role field: " + field); } else { - delegate = em.find(JpaRoleEntity.class, UUID.fromString(delegate.getId())); + setDelegate(em.find(JpaRoleEntity.class, UUID.fromString(getDelegate().getId()))); } - return delegate; + return getDelegate(); } } diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java index 9f1812cec4..4a61f45568 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java @@ -43,6 +43,8 @@ import org.hibernate.annotations.TypeDefs; import org.keycloak.models.map.common.DeepCloner; import org.keycloak.models.map.role.MapRoleEntity.AbstractRoleEntity; import static org.keycloak.models.map.storage.jpa.Constants.SUPPORTED_VERSION_ROLE; + +import org.keycloak.models.map.storage.jpa.JpaRootEntity; import org.keycloak.models.map.storage.jpa.hibernate.jsonb.JsonbType; /** @@ -53,7 +55,7 @@ import org.keycloak.models.map.storage.jpa.hibernate.jsonb.JsonbType; @Entity @Table(name = "role", uniqueConstraints = {@UniqueConstraint(columnNames = {"realmId", "clientId", "name"})}) @TypeDefs({@TypeDef(name = "jsonb", typeClass = JsonbType.class)}) -public class JpaRoleEntity extends AbstractRoleEntity implements Serializable { +public class JpaRoleEntity extends AbstractRoleEntity implements Serializable, JpaRootEntity { @Id @Column @@ -106,8 +108,9 @@ public class JpaRoleEntity extends AbstractRoleEntity implements Serializable { * Used by hibernate when calling cb.construct from read(QueryParameters) method. * It is used to select role without metadata(json) field. */ - public JpaRoleEntity(UUID id, Integer entityVersion, String realmId, String clientId, String name, String description) { + public JpaRoleEntity(UUID id, int version, Integer entityVersion, String realmId, String clientId, String name, String description) { this.id = id; + this.version = version; this.entityVersion = entityVersion; this.realmId = realmId; this.clientId = clientId; @@ -140,6 +143,7 @@ public class JpaRoleEntity extends AbstractRoleEntity implements Serializable { metadata.setEntityVersion(entityVersion); } + @Override public int getVersion() { return version; } diff --git a/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java b/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java index 2edf4737e9..c03480990e 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java +++ b/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java @@ -19,6 +19,7 @@ package org.keycloak.models.delegate; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientScopeModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelIllegalStateException; import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; @@ -83,7 +84,7 @@ public class ClientModelLazyDelegate implements ClientModel { } ClientModel ref = delegate.getReference(); if (ref == null) { - throw new IllegalStateException("Invalid delegate obtained"); + throw new ModelIllegalStateException("Invalid delegate obtained"); } return ref; } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 151aeb1f72..282eedf40f 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -17,6 +17,7 @@ package org.keycloak.models.utils; +import org.jboss.logging.Logger; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.PermissionTicket; import org.keycloak.authorization.model.Policy; @@ -46,8 +47,10 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -101,6 +104,7 @@ public class ModelToRepresentation { REALM_EXCLUDED_ATTRIBUTES.add(Constants.CLIENT_PROFILES); } + private static final Logger LOG = Logger.getLogger(ModelToRepresentation.class); public static void buildGroupPath(StringBuilder sb, GroupModel group) { if (group.getParent() != null) { @@ -554,6 +558,25 @@ public class ModelToRepresentation { return rep; } + /** + * Handles exceptions that occur when transforming the model to a representation and will remove + * all null objects from the stream. + * + * Entities that have been removed from the store or where a lazy loading exception occurs will not show up + * in the output stream. + */ + public static Stream filterValidRepresentations(Stream models, Function transformer) { + return models.map(m -> { + try { + return transformer.apply(m); + } catch (ModelIllegalStateException e) { + LOG.warn("unable to retrieve model information, skipping entity", e); + return null; + } + }) + .filter(Objects::nonNull); + } + public static CredentialRepresentation toRepresentation(UserCredentialModel cred) { CredentialRepresentation rep = new CredentialRepresentation(); rep.setType(CredentialRepresentation.SECRET); diff --git a/server-spi/src/main/java/org/keycloak/models/ModelIllegalStateException.java b/server-spi/src/main/java/org/keycloak/models/ModelIllegalStateException.java new file mode 100644 index 0000000000..8780727aa4 --- /dev/null +++ b/server-spi/src/main/java/org/keycloak/models/ModelIllegalStateException.java @@ -0,0 +1,49 @@ +/* + * 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; + +import java.util.function.Function; +import java.util.stream.Stream; + +/** + * Thrown when data can't be retrieved for the model. + * + * This occurs when an entity has been removed or updated in the meantime. This might wrap an optimistic lock exception + * depending on the store. + * + * Callers might use this exception to filter out entities that are in an illegal state, see + * org.keycloak.models.utils.ModelToRepresentation#toRepresentation(Stream, Function) + * + * @author Alexander Schwartz + */ +public class ModelIllegalStateException extends ModelException { + + public ModelIllegalStateException() { + } + + public ModelIllegalStateException(String message) { + super(message); + } + + public ModelIllegalStateException(String message, Throwable cause) { + super(message, cause); + } + + public ModelIllegalStateException(Throwable cause) { + super(cause); + } +} diff --git a/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java b/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java index 86c621f8f2..e4eb3ba7b1 100755 --- a/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java +++ b/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java @@ -17,22 +17,11 @@ package org.keycloak.exportimport.util; -import static org.keycloak.models.utils.ModelToRepresentation.toRepresentation; - -import java.io.IOException; -import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; - +import com.fasterxml.jackson.core.JsonEncoding; +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.AuthorizationProviderFactory; import org.keycloak.authorization.model.Policy; @@ -71,11 +60,20 @@ import org.keycloak.representations.idm.authorization.ResourceServerRepresentati import org.keycloak.representations.idm.authorization.ScopeRepresentation; import org.keycloak.util.JsonSerialization; -import com.fasterxml.jackson.core.JsonEncoding; -import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.SerializationFeature; +import java.io.IOException; +import java.io.OutputStream; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.keycloak.models.utils.ModelToRepresentation.toRepresentation; /** * @author Marek Posolda @@ -103,15 +101,17 @@ public class ExportUtils { .map(ClientScopeModel::getName).collect(Collectors.toList())); // Clients - List clients = Collections.emptyList(); + List clients = new LinkedList<>(); if (options.isClientsIncluded()) { - clients = realm.getClientsStream() - .filter(c -> { try { c.getClientId(); return true; } catch (Exception ex) { return false; } } ) - .collect(Collectors.toList()); - List clientReps = clients.stream() - .map(app -> exportClient(session, app)) - .collect(Collectors.toList()); + // we iterate over all clients in the stream. + // only those client models that can be translated into a valid client representation will be added to the client list + // that is later used to retrieve related information about groups and roles + List clientReps = ModelToRepresentation.filterValidRepresentations(realm.getClientsStream(), app -> { + ClientRepresentation clientRepresentation = exportClient(session, app); + clients.add(app); + return clientRepresentation; + }).collect(Collectors.toList()); rep.setClients(clientReps); } diff --git a/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java b/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java index bfce618de8..97ea009379 100755 --- a/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java +++ b/services/src/main/java/org/keycloak/services/managers/ResourceAdminManager.java @@ -33,6 +33,7 @@ import org.keycloak.constants.AdapterConstants; import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelIllegalStateException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.protocol.LoginProtocol; @@ -241,15 +242,18 @@ public class ResourceAdminManager { public GlobalRequestResult logoutAll(RealmModel realm) { realm.setNotBefore(Time.currentTime()); - Stream resources = realm.getClientsStream() - .filter(c -> { try { c.getClientId(); return true; } catch (Exception ex) { return false; } } ); GlobalRequestResult finalResult = new GlobalRequestResult(); AtomicInteger counter = new AtomicInteger(0); - resources.forEach(r -> { - counter.getAndIncrement(); - GlobalRequestResult currentResult = logoutClient(realm, r, realm.getNotBefore()); - finalResult.addAll(currentResult); + realm.getClientsStream().forEach(c -> { + try { + counter.getAndIncrement(); + GlobalRequestResult currentResult = logoutClient(realm, c, realm.getNotBefore()); + finalResult.addAll(currentResult); + } catch (ModelIllegalStateException ex) { + // currently, GlobalRequestResult doesn't allow for information about clients that we were unable to retrieve. + logger.warn("unable to retrieve client information for logout, skipping resource", ex); + } }); logger.debugv("logging out {0} resources ", counter); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java index 95601891d5..fafb39d44d 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java @@ -57,7 +57,6 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.util.Map; -import java.util.Objects; import java.util.stream.Stream; import static java.lang.Boolean.TRUE; @@ -87,9 +86,11 @@ public class ClientsResource { } /** - * Get clients belonging to the realm + * Get clients belonging to the realm. * - * Returns a list of clients belonging to the realm + * If a client can't be retrieved from the storage due to a problem with the underlying storage, + * it is silently removed from the returned list. + * This ensures that concurrent modifications to the list don't prevent callers from retrieving this list. * * @param clientId filter by clientId * @param viewableOnly filter clients that cannot be viewed in full by admin @@ -131,9 +132,8 @@ public class ClientsResource { } } - Stream s = clientModels - .filter(c -> { try { c.getClientId(); return true; } catch (Exception ex) { return false; } } ) - .map(c -> { + Stream s = ModelToRepresentation.filterValidRepresentations(clientModels, + c -> { ClientRepresentation representation = null; if (canView || auth.clients().canView(c)) { representation = ModelToRepresentation.toRepresentation(c, session); @@ -146,8 +146,7 @@ public class ClientsResource { } return representation; - }) - .filter(Objects::nonNull); + }); if (!canView) { s = paginatedStream(s, firstResult, maxResults);