From c9c8acd0294f0a9fb7c805f7ce16b266765c3413 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Mon, 19 Dec 2016 20:28:49 -0200 Subject: [PATCH] [KEYCLOAK-4034] - Invalidating policy cache when creating resources and scopes --- .../infinispan/CachedPolicyStore.java | 11 +++++++++- .../infinispan/CachedResourceStore.java | 21 +++++++++++++++++-- .../infinispan/CachedScopeStore.java | 13 ++++++++++++ .../jpa/store/JPAResourceStore.java | 10 +++++++++ .../jpa/store/JPAScopeStore.java | 2 +- .../mongo/store/MongoResourceStore.java | 11 ++++++++++ .../authorization/store/ResourceStore.java | 8 +++++++ .../protection/resource/ResourceService.java | 8 +++---- 8 files changed, 75 insertions(+), 9 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedPolicyStore.java b/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedPolicyStore.java index 3914c68dd9..e332de0fc6 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedPolicyStore.java +++ b/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedPolicyStore.java @@ -77,8 +77,11 @@ public class CachedPolicyStore implements PolicyStore { Policy policy = getDelegate().create(name, type, getStoreFactory().getResourceServerStore().findById(resourceServer.getId())); String id = policy.getId(); - this.transaction.whenCommit(() -> { + this.transaction.whenRollback(() -> { cache.remove(getCacheKeyForPolicy(id)); + }); + + this.transaction.whenCommit(() -> { invalidateCache(resourceServer.getId()); }); @@ -88,6 +91,9 @@ public class CachedPolicyStore implements PolicyStore { @Override public void delete(String id) { Policy policy = findById(id, null); + if (policy == null) { + return; + } ResourceServer resourceServer = policy.getResourceServer(); getDelegate().delete(id); this.transaction.whenCommit(() -> { @@ -385,6 +391,9 @@ public class CachedPolicyStore implements PolicyStore { cache.remove(getCacheKeyForPolicy(getId())); invalidateCache(cached.getResourceServerId()); }); + transaction.whenRollback(() -> { + cache.remove(getCacheKeyForPolicy(getId())); + }); } return this.updated; diff --git a/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedResourceStore.java b/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedResourceStore.java index 6ebac833c5..92adf6ce0d 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedResourceStore.java +++ b/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedResourceStore.java @@ -62,6 +62,7 @@ public class CachedResourceStore implements ResourceStore { this.transaction = transaction; cacheKeys = new ArrayList<>(); cacheKeys.add("findByOwner"); + cacheKeys.add("findByUri"); this.storeFactory = storeFactory; } @@ -71,7 +72,11 @@ public class CachedResourceStore implements ResourceStore { this.transaction.whenRollback(() -> { cache.remove(getCacheKeyForResource(resource.getId())); + }); + + this.transaction.whenCommit(() -> { invalidateCache(resourceServer.getId()); + getCachedStoreFactory().getPolicyStore().notifyChange(resource); }); return createAdapter(new CachedResource(resource)); @@ -80,6 +85,9 @@ public class CachedResourceStore implements ResourceStore { @Override public void delete(String id) { Resource resource = findById(id, null); + if (resource == null) { + return; + } ResourceServer resourceServer = resource.getResourceServer(); getDelegate().delete(id); this.transaction.whenCommit(() -> { @@ -91,6 +99,7 @@ public class CachedResourceStore implements ResourceStore { } invalidateCache(resourceServer.getId()); + getCachedStoreFactory().getPolicyStore().notifyChange(resource); }); } @@ -117,6 +126,11 @@ public class CachedResourceStore implements ResourceStore { return cacheResult(new StringBuilder("findByOwner").append(resourceServerId).append(ownerId).toString(), () -> getDelegate().findByOwner(ownerId, resourceServerId)); } + @Override + public List findByUri(String uri, String resourceServerId) { + return cacheResult(new StringBuilder("findByUri").append(resourceServerId).append(uri).toString(), () -> getDelegate().findByUri(uri, resourceServerId)); + } + @Override public List findByResourceServer(String resourceServerId) { return getDelegate().findByResourceServer(resourceServerId); @@ -268,10 +282,13 @@ public class CachedResourceStore implements ResourceStore { if (this.updated == null) { this.updated = getDelegate().findById(getId(), cached.getResourceServerId()); if (this.updated == null) throw new IllegalStateException("Not found in database"); - transaction.whenRollback(() -> { + transaction.whenCommit(() -> { cache.remove(getCacheKeyForResource(cached.getId())); invalidateCache(cached.getResourceServerId()); - getCachedStoreFactory().getPolicyStore().notifyChange(cached); + getCachedStoreFactory().getPolicyStore().notifyChange(updated); + }); + transaction.whenRollback(() -> { + cache.remove(getCacheKeyForResource(cached.getId())); }); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedScopeStore.java b/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedScopeStore.java index 77a6970936..5be00b4fb6 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedScopeStore.java +++ b/model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedScopeStore.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import org.infinispan.Cache; +import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.store.ScopeStore; @@ -61,12 +62,19 @@ public class CachedScopeStore implements ScopeStore { Scope scope = getDelegate().create(name, getStoreFactory().getResourceServerStore().findById(resourceServer.getId())); this.transaction.whenRollback(() -> cache.remove(getCacheKeyForScope(scope.getId()))); + this.transaction.whenCommit(() -> { + getCachedStoreFactory().getPolicyStore().notifyChange(scope); + }); return createAdapter(new CachedScope(scope)); } @Override public void delete(String id) { + Scope scope = findById(id, null); + if (scope == null) { + return; + } getDelegate().delete(id); this.transaction.whenCommit(() -> { List scopes = cache.remove(getCacheKeyForScope(id)); @@ -75,6 +83,8 @@ public class CachedScopeStore implements ScopeStore { CachedScope entry = scopes.get(0); cache.remove(getCacheKeyForScopeName(entry.getName(), entry.getResourceServerId())); } + + getCachedStoreFactory().getPolicyStore().notifyChange(scope); }); } @@ -190,6 +200,9 @@ public class CachedScopeStore implements ScopeStore { cache.remove(getCacheKeyForScope(getId())); getCachedStoreFactory().getPolicyStore().notifyChange(updated); }); + transaction.whenRollback(() -> { + cache.remove(getCacheKeyForScope(cached.getId())); + }); } return this.updated; diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java index 9f5346da50..0b04388e55 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java @@ -103,6 +103,16 @@ public class JPAResourceStore implements ResourceStore { return query.getResultList(); } + @Override + public List findByUri(String uri, String resourceServerId) { + Query query = entityManager.createQuery("from ResourceEntity where resourceServer.id = :serverId and uri = :uri"); + + query.setParameter("uri", uri); + query.setParameter("serverId", resourceServerId); + + return query.getResultList(); + } + @Override public List findByResourceServer(String resourceServerId) { Query query = entityManager.createQuery("from ResourceEntity where resourceServer.id = :serverId"); diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java index 30869d3d72..031eb4a087 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java @@ -75,7 +75,7 @@ public class JPAScopeStore implements ScopeStore { return null; } - if (true) { + if (resourceServerId == null) { return entityManager.find(ScopeEntity.class, id); } diff --git a/model/mongo/src/main/java/org/keycloak/authorization/mongo/store/MongoResourceStore.java b/model/mongo/src/main/java/org/keycloak/authorization/mongo/store/MongoResourceStore.java index 79f6a9ec71..dcbf3ec2f4 100644 --- a/model/mongo/src/main/java/org/keycloak/authorization/mongo/store/MongoResourceStore.java +++ b/model/mongo/src/main/java/org/keycloak/authorization/mongo/store/MongoResourceStore.java @@ -91,6 +91,17 @@ public class MongoResourceStore implements ResourceStore { .map(scope -> findById(scope.getId(), resourceServerId)).collect(toList()); } + @Override + public List findByUri(String uri, String resourceServerId) { + DBObject query = new QueryBuilder() + .and("resourceServerId").is(resourceServerId) + .and("uri").is(uri) + .get(); + + return getMongoStore().loadEntities(ResourceEntity.class, query, getInvocationContext()).stream() + .map(scope -> findById(scope.getId(), resourceServerId)).collect(toList()); + } + @Override public List findByResourceServer(String resourceServerId) { DBObject query = new QueryBuilder() diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java index e897664669..b55ec746c8 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java @@ -63,6 +63,14 @@ public interface ResourceStore { */ List findByOwner(String ownerId, String resourceServerId); + /** + * Finds all {@link Resource} instances with the given uri. + * + * @param ownerId the identifier of the owner + * @return a list with all resource instances owned by the given owner + */ + List findByUri(String uri, String resourceServerId); + /** * Finds all {@link Resource} instances associated with a given resource server. * diff --git a/services/src/main/java/org/keycloak/authorization/protection/resource/ResourceService.java b/services/src/main/java/org/keycloak/authorization/protection/resource/ResourceService.java index 219bccacc8..3573d3ddf9 100644 --- a/services/src/main/java/org/keycloak/authorization/protection/resource/ResourceService.java +++ b/services/src/main/java/org/keycloak/authorization/protection/resource/ResourceService.java @@ -132,19 +132,17 @@ public class ResourceService { if ("name".equals(filterType)) { - resources.addAll(storeFactory.getResourceStore().findByResourceServer(this.resourceServer.getId()).stream().filter(description -> filterValue == null || filterValue.equals(description.getName())).collect(Collectors.toSet()).stream() - .map(resource -> ModelToRepresentation.toRepresentation(resource, this.resourceServer, authorization)) - .collect(Collectors.toList())); + resources.add(ModelToRepresentation.toRepresentation(storeFactory.getResourceStore().findByName(filterValue, this.resourceServer.getId()), resourceServer, authorization)); } else if ("type".equals(filterType)) { resources.addAll(storeFactory.getResourceStore().findByResourceServer(this.resourceServer.getId()).stream().filter(description -> filterValue == null || filterValue.equals(description.getType())).collect(Collectors.toSet()).stream() .map(resource -> ModelToRepresentation.toRepresentation(resource, this.resourceServer, authorization)) .collect(Collectors.toList())); } else if ("uri".equals(filterType)) { - resources.addAll(storeFactory.getResourceStore().findByResourceServer(this.resourceServer.getId()).stream().filter(description -> filterValue == null || filterValue.equals(description.getUri())).collect(Collectors.toSet()).stream() + resources.addAll(storeFactory.getResourceStore().findByUri(filterValue, this.resourceServer.getId()).stream() .map(resource -> ModelToRepresentation.toRepresentation(resource, this.resourceServer, authorization)) .collect(Collectors.toList())); } else if ("owner".equals(filterType)) { - resources.addAll(storeFactory.getResourceStore().findByResourceServer(this.resourceServer.getId()).stream().filter(description -> filterValue == null || filterValue.equals(description.getOwner())).collect(Collectors.toSet()).stream() + resources.addAll(storeFactory.getResourceStore().findByOwner(filterValue, this.resourceServer.getId()).stream() .map(resource -> ModelToRepresentation.toRepresentation(resource, this.resourceServer, authorization)) .collect(Collectors.toList())); }