Skip to content

Commit

Permalink
[KEYCLOAK-6621] - Fixing cache and queries of policies with type scope
Browse files Browse the repository at this point in the history
  • Loading branch information
@pedroigor
pedroigor committed Feb 28, 2018
1 parent b0200d4 commit cb53105
Show file tree
Hide file tree
Showing 12 changed files with 171 additions and 27 deletions.
24 changes: 18 additions & 6 deletions ...ispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/PolicyAdapter.java
View file
Expand Up @@ -48,8 +48,9 @@ public PolicyAdapter(CachedPolicy cached, StoreFactoryCacheSession cacheSession)
@Override @Override
public Policy getDelegateForUpdate() { public Policy getDelegateForUpdate() {
if (updated == null) { if (updated == null) {
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), cached.getResourceServerId());
updated = cacheSession.getPolicyStoreDelegate().findById(cached.getId(), cached.getResourceServerId()); updated = cacheSession.getPolicyStoreDelegate().findById(cached.getId(), cached.getResourceServerId());
String defaultResourceType = updated.getConfig().get("defaultResourceType");
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), defaultResourceType, cached.getResourceServerId());
if (updated == null) throw new IllegalStateException("Not found in database"); if (updated == null) throw new IllegalStateException("Not found in database");
} }
return updated; return updated;
Expand Down Expand Up @@ -97,7 +98,7 @@ public String getName() {
@Override @Override
public void setName(String name) { public void setName(String name) {
getDelegateForUpdate(); getDelegateForUpdate();
cacheSession.registerPolicyInvalidation(cached.getId(), name, cached.getResourcesIds(), cached.getScopesIds(), cached.getResourceServerId()); cacheSession.registerPolicyInvalidation(cached.getId(), name, cached.getResourcesIds(), cached.getScopesIds(), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
updated.setName(name); updated.setName(name);


} }
Expand Down Expand Up @@ -148,20 +149,31 @@ public Map<String, String> getConfig() {
@Override @Override
public void setConfig(Map<String, String> config) { public void setConfig(Map<String, String> config) {
getDelegateForUpdate(); getDelegateForUpdate();
if (config.containsKey("defaultResourceType") || cached.getConfig().containsKey("defaultResourceType")) {
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), config.get("defaultResourceType"), cached.getResourceServerId());
}
updated.setConfig(config); updated.setConfig(config);


} }


@Override @Override
public void removeConfig(String name) { public void removeConfig(String name) {
getDelegateForUpdate(); getDelegateForUpdate();
if (name.equals("defaultResourceType")) {
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
}
updated.removeConfig(name); updated.removeConfig(name);


} }


@Override @Override
public void putConfig(String name, String value) { public void putConfig(String name, String value) {
getDelegateForUpdate(); getDelegateForUpdate();
if (name.equals("defaultResourceType")) {
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), value, cached.getResourceServerId());
}
updated.putConfig(name, value); updated.putConfig(name, value);
} }


Expand Down Expand Up @@ -207,14 +219,14 @@ public Set<Resource> getResources() {
@Override @Override
public void addScope(Scope scope) { public void addScope(Scope scope) {
getDelegateForUpdate(); getDelegateForUpdate();
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), new HashSet<>(Arrays.asList(scope.getId())), cached.getResourceServerId()); cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), new HashSet<>(Arrays.asList(scope.getId())), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
updated.addScope(scope); updated.addScope(scope);
} }


@Override @Override
public void removeScope(Scope scope) { public void removeScope(Scope scope) {
getDelegateForUpdate(); getDelegateForUpdate();
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), new HashSet<>(Arrays.asList(scope.getId())), cached.getResourceServerId()); cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), new HashSet<>(Arrays.asList(scope.getId())), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
updated.removeScope(scope); updated.removeScope(scope);
} }


Expand All @@ -237,7 +249,7 @@ public void addResource(Resource resource) {
getDelegateForUpdate(); getDelegateForUpdate();
HashSet<String> resources = new HashSet<>(); HashSet<String> resources = new HashSet<>();
resources.add(resource.getId()); resources.add(resource.getId());
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), resources, cached.getScopesIds(), cached.getResourceServerId()); cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), resources, cached.getScopesIds(), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
updated.addResource(resource); updated.addResource(resource);


} }
Expand All @@ -247,7 +259,7 @@ public void removeResource(Resource resource) {
getDelegateForUpdate(); getDelegateForUpdate();
HashSet<String> resources = new HashSet<>(); HashSet<String> resources = new HashSet<>();
resources.add(resource.getId()); resources.add(resource.getId());
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), resources, cached.getScopesIds(), cached.getResourceServerId()); cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), resources, cached.getScopesIds(), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
updated.removeResource(resource); updated.removeResource(resource);


} }
Expand Down
7 changes: 7 additions & 0 deletions ...ain/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheManager.java
View file
Expand Up @@ -27,6 +27,7 @@
import org.keycloak.models.cache.infinispan.entities.Revisioned; import org.keycloak.models.cache.infinispan.entities.Revisioned;
import org.keycloak.models.cache.infinispan.events.InvalidationEvent; import org.keycloak.models.cache.infinispan.events.InvalidationEvent;


import java.util.Objects;
import java.util.Set; import java.util.Set;


/** /**
Expand Down Expand Up @@ -112,6 +113,11 @@ public void policyUpdated(String id, String name, Set<String> resources, Set<Str
if (resources != null) { if (resources != null) {
for (String resource : resources) { for (String resource : resources) {
invalidations.add(StoreFactoryCacheSession.getPolicyByResource(resource, serverId)); invalidations.add(StoreFactoryCacheSession.getPolicyByResource(resource, serverId));
if (Objects.nonNull(scopes)) {
for (String scope : scopes) {
invalidations.add(StoreFactoryCacheSession.getPolicyByResourceScope(scope, resource, serverId));
}
}
} }
} }


Expand All @@ -124,6 +130,7 @@ public void policyUpdated(String id, String name, Set<String> resources, Set<Str
if (scopes != null) { if (scopes != null) {
for (String scope : scopes) { for (String scope : scopes) {
invalidations.add(StoreFactoryCacheSession.getPolicyByScope(scope, serverId)); invalidations.add(StoreFactoryCacheSession.getPolicyByScope(scope, serverId));
invalidations.add(StoreFactoryCacheSession.getPolicyByResourceScope(scope, null, serverId));
} }
} }
} }
Expand Down
31 changes: 28 additions & 3 deletions ...ain/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java
View file
Expand Up @@ -271,8 +271,11 @@ public void registerResourceInvalidation(String id, String name, String type, St
invalidationEvents.add(ResourceUpdatedEvent.create(id, name, type, uri, scopes, serverId, owner)); invalidationEvents.add(ResourceUpdatedEvent.create(id, name, type, uri, scopes, serverId, owner));
} }


public void registerPolicyInvalidation(String id, String name, Set<String> resources, Set<String> scopes, String serverId) { public void registerPolicyInvalidation(String id, String name, Set<String> resources, Set<String> scopes, String defaultResourceType, String serverId) {
Set<String> resourceTypes = getResourceTypes(resources, serverId); Set<String> resourceTypes = getResourceTypes(resources, serverId);
if (Objects.nonNull(defaultResourceType)) {
resourceTypes.add(defaultResourceType);
}
cache.policyUpdated(id, name, resources, resourceTypes, scopes, serverId, invalidations); cache.policyUpdated(id, name, resources, resourceTypes, scopes, serverId, invalidations);
PolicyAdapter adapter = managedPolicies.get(id); PolicyAdapter adapter = managedPolicies.get(id);
if (adapter != null) adapter.invalidateFlag(); if (adapter != null) adapter.invalidateFlag();
Expand Down Expand Up @@ -369,6 +372,10 @@ public static String getPolicyByScope(String scope, String serverId) {
return "policy.scope." + scope + "." + serverId; return "policy.scope." + scope + "." + serverId;
} }


public static String getPolicyByResourceScope(String scope, String resourceId, String serverId) {
return "policy.resource. " + resourceId + ".scope." + scope + "." + serverId;
}

public static String getPermissionTicketByResource(String resourceId, String serverId) { public static String getPermissionTicketByResource(String resourceId, String serverId) {
return "permission.ticket.resource." + resourceId + "." + serverId; return "permission.ticket.resource." + resourceId + "." + serverId;
} }
Expand Down Expand Up @@ -664,8 +671,9 @@ protected class PolicyCache implements PolicyStore {
@Override @Override
public Policy create(AbstractPolicyRepresentation representation, ResourceServer resourceServer) { public Policy create(AbstractPolicyRepresentation representation, ResourceServer resourceServer) {
Policy policy = getPolicyStoreDelegate().create(representation, resourceServer); Policy policy = getPolicyStoreDelegate().create(representation, resourceServer);
registerPolicyInvalidation(policy.getId(), representation.getName(), representation.getResources(), representation.getScopes(), resourceServer.getId()); Policy cached = findById(policy.getId(), resourceServer.getId());
return policy; registerPolicyInvalidation(policy.getId(), representation.getName(), representation.getResources(), representation.getScopes(), null, resourceServer.getId());
return cached;
} }


@Override @Override
Expand All @@ -678,6 +686,10 @@ public void delete(String id) {
Set<String> resources = policy.getResources().stream().map(resource -> resource.getId()).collect(Collectors.toSet()); Set<String> resources = policy.getResources().stream().map(resource -> resource.getId()).collect(Collectors.toSet());
ResourceServer resourceServer = policy.getResourceServer(); ResourceServer resourceServer = policy.getResourceServer();
Set<String> resourceTypes = getResourceTypes(resources, resourceServer.getId()); Set<String> resourceTypes = getResourceTypes(resources, resourceServer.getId());
String defaultResourceType = policy.getConfig().get("defaultResourceType");
if (Objects.nonNull(defaultResourceType)) {
resourceTypes.add(defaultResourceType);
}
Set<String> scopes = policy.getScopes().stream().map(scope -> scope.getId()).collect(Collectors.toSet()); Set<String> scopes = policy.getScopes().stream().map(scope -> scope.getId()).collect(Collectors.toSet());
invalidationEvents.add(PolicyRemovedEvent.create(id, policy.getName(), resources, resourceTypes, scopes, resourceServer.getId())); invalidationEvents.add(PolicyRemovedEvent.create(id, policy.getName(), resources, resourceTypes, scopes, resourceServer.getId()));
cache.policyRemoval(id, policy.getName(), resources, resourceTypes, scopes, resourceServer.getId(), invalidations); cache.policyRemoval(id, policy.getName(), resources, resourceTypes, scopes, resourceServer.getId(), invalidations);
Expand Down Expand Up @@ -771,6 +783,19 @@ public List<Policy> findByScopeIds(List<String> scopeIds, String resourceServerI
return result; return result;
} }


@Override
public List<Policy> findByScopeIds(List<String> scopeIds, String resourceId, String resourceServerId) {
if (scopeIds == null) return null;
List<Policy> result = new ArrayList<>();

for (String id : scopeIds) {
String cacheKey = getPolicyByResourceScope(id, resourceId, resourceServerId);
result.addAll(cacheQuery(cacheKey, PolicyScopeListQuery.class, () -> getPolicyStoreDelegate().findByScopeIds(Arrays.asList(id), resourceId, resourceServerId), (revision, resources) -> new PolicyScopeListQuery(revision, cacheKey, id, resources.stream().map(resource -> resource.getId()).collect(Collectors.toSet()), resourceServerId), resourceServerId));
}

return result;
}

@Override @Override
public List<Policy> findByType(String type, String resourceServerId) { public List<Policy> findByType(String type, String resourceServerId) {
return getPolicyStoreDelegate().findByType(type, resourceServerId); return getPolicyStoreDelegate().findByType(type, resourceServerId);
Expand Down
2 changes: 2 additions & 0 deletions model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/PolicyEntity.java
View file
Expand Up @@ -57,6 +57,8 @@
@NamedQuery(name="findPolicyIdByName", query="select p.id from PolicyEntity p where p.resourceServer.id = :serverId and p.name = :name"), @NamedQuery(name="findPolicyIdByName", query="select p.id from PolicyEntity p where p.resourceServer.id = :serverId and p.name = :name"),
@NamedQuery(name="findPolicyIdByResource", query="select p.id from PolicyEntity p inner join p.resources r where p.resourceServer.id = :serverId and (r.resourceServer.id = :serverId and r.id = :resourceId)"), @NamedQuery(name="findPolicyIdByResource", query="select p.id from PolicyEntity p inner join p.resources r where p.resourceServer.id = :serverId and (r.resourceServer.id = :serverId and r.id = :resourceId)"),
@NamedQuery(name="findPolicyIdByScope", query="select pe.id from PolicyEntity pe where pe.resourceServer.id = :serverId and pe.id IN (select p.id from ScopeEntity s inner join s.policies p where s.resourceServer.id = :serverId and (p.resourceServer.id = :serverId and p.type = 'scope' and s.id in (:scopeIds)))"), @NamedQuery(name="findPolicyIdByScope", query="select pe.id from PolicyEntity pe where pe.resourceServer.id = :serverId and pe.id IN (select p.id from ScopeEntity s inner join s.policies p where s.resourceServer.id = :serverId and (p.resourceServer.id = :serverId and p.type = 'scope' and s.id in (:scopeIds)))"),
@NamedQuery(name="findPolicyIdByResourceScope", query="select pe.id from PolicyEntity pe where pe.resourceServer.id = :serverId and pe.id IN (select p.id from ScopeEntity s inner join s.policies p where s.resourceServer.id = :serverId and (p.resourceServer.id = :serverId and p.type = 'scope' and s.id in (:scopeIds))) and pe.id IN (select p.id from ResourceEntity r inner join r.policies p where r.resourceServer.id = :serverId and (p.resourceServer.id = :serverId and p.type = 'scope' and r.id in (:resourceId))))"),
@NamedQuery(name="findPolicyIdByNullResourceScope", query="select pe.id from PolicyEntity pe where pe.resourceServer.id = :serverId and pe.id IN (select p.id from ScopeEntity s inner join s.policies p where s.resourceServer.id = :serverId and (p.resourceServer.id = :serverId and p.type = 'scope' and s.id in (:scopeIds))) and pe.resources is empty"),
@NamedQuery(name="findPolicyIdByType", query="select p.id from PolicyEntity p where p.resourceServer.id = :serverId and p.type = :type"), @NamedQuery(name="findPolicyIdByType", query="select p.id from PolicyEntity p where p.resourceServer.id = :serverId and p.type = :type"),
@NamedQuery(name="findPolicyIdByResourceType", query="select p.id from PolicyEntity p inner join p.config c where p.resourceServer.id = :serverId and KEY(c) = 'defaultResourceType' and c like :type"), @NamedQuery(name="findPolicyIdByResourceType", query="select p.id from PolicyEntity p inner join p.config c where p.resourceServer.id = :serverId and KEY(c) = 'defaultResourceType' and c like :type"),
@NamedQuery(name="findPolicyIdByDependentPolices", query="select p.id from PolicyEntity p inner join p.associatedPolicies ap where p.resourceServer.id = :serverId and (ap.resourceServer.id = :serverId and ap.id = :policyId)"), @NamedQuery(name="findPolicyIdByDependentPolices", query="select p.id from PolicyEntity p inner join p.associatedPolicies ap where p.resourceServer.id = :serverId and (ap.resourceServer.id = :serverId and ap.id = :policyId)"),
Expand Down
67 changes: 60 additions & 7 deletions model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java
View file
Expand Up @@ -22,6 +22,7 @@
import java.util.LinkedList; import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects;


import javax.persistence.EntityManager; import javax.persistence.EntityManager;
import javax.persistence.FlushModeType; import javax.persistence.FlushModeType;
Expand Down Expand Up @@ -114,7 +115,10 @@ public List<Policy> findByResourceServer(final String resourceServerId) {
List<String> result = query.getResultList(); List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>(); List<Policy> list = new LinkedList<>();
for (String id : result) { for (String id : result) {
list.add(provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId)); Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
} }
return list; return list;
} }
Expand Down Expand Up @@ -157,7 +161,10 @@ public List<Policy> findByResourceServer(Map<String, String[]> attributes, Strin
List<String> result = query.getResultList(); List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>(); List<Policy> list = new LinkedList<>();
for (String id : result) { for (String id : result) {
list.add(provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId)); Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
} }
return list; return list;
} }
Expand All @@ -173,7 +180,10 @@ public List<Policy> findByResource(final String resourceId, String resourceServe
List<String> result = query.getResultList(); List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>(); List<Policy> list = new LinkedList<>();
for (String id : result) { for (String id : result) {
list.add(provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId)); Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
} }
return list; return list;
} }
Expand All @@ -189,7 +199,10 @@ public List<Policy> findByResourceType(final String resourceType, String resourc
List<String> result = query.getResultList(); List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>(); List<Policy> list = new LinkedList<>();
for (String id : result) { for (String id : result) {
list.add(provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId)); Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
} }
return list; return list;
} }
Expand All @@ -210,7 +223,41 @@ public List<Policy> findByScopeIds(List<String> scopeIds, String resourceServerI
List<String> result = query.getResultList(); List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>(); List<Policy> list = new LinkedList<>();
for (String id : result) { for (String id : result) {
list.add(provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId)); Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
}
return list;
}

@Override
public List<Policy> findByScopeIds(List<String> scopeIds, String resourceId, String resourceServerId) {
if (scopeIds==null || scopeIds.isEmpty()) {
return Collections.emptyList();
}

// Use separate subquery to handle DB2 and MSSSQL
TypedQuery<String> query;

if (resourceId == null) {
query = entityManager.createNamedQuery("findPolicyIdByNullResourceScope", String.class);
} else {
query = entityManager.createNamedQuery("findPolicyIdByResourceScope", String.class);
query.setParameter("resourceId", resourceId);
}

query.setFlushMode(FlushModeType.COMMIT);
query.setParameter("scopeIds", scopeIds);
query.setParameter("serverId", resourceServerId);

List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
for (String id : result) {
Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
} }
return list; return list;
} }
Expand All @@ -226,7 +273,10 @@ public List<Policy> findByType(String type, String resourceServerId) {
List<String> result = query.getResultList(); List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>(); List<Policy> list = new LinkedList<>();
for (String id : result) { for (String id : result) {
list.add(provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId)); Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
} }
return list; return list;
} }
Expand All @@ -243,7 +293,10 @@ public List<Policy> findDependentPolicies(String policyId, String resourceServer
List<String> result = query.getResultList(); List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>(); List<Policy> list = new LinkedList<>();
for (String id : result) { for (String id : result) {
list.add(provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId)); Policy policy = provider.getStoreFactory().getPolicyStore().findById(id, resourceServerId);
if (Objects.nonNull(policy)) {
list.add(policy);
}
} }
return list; return list;
} }
Expand Down
5 changes: 5 additions & 0 deletions server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java
View file
Expand Up @@ -385,6 +385,11 @@ public List<Policy> findByScopeIds(List<String> scopeIds, String resourceServerI
return policyStore.findByScopeIds(scopeIds, resourceServerId); return policyStore.findByScopeIds(scopeIds, resourceServerId);
} }


@Override
public List<Policy> findByScopeIds(List<String> scopeIds, String resourceId, String resourceServerId) {
return policyStore.findByScopeIds(scopeIds, resourceId, resourceServerId);
}

@Override @Override
public List<Policy> findByType(String type, String resourceServerId) { public List<Policy> findByType(String type, String resourceServerId) {
return policyStore.findByType(type, resourceServerId); return policyStore.findByType(type, resourceServerId);
Expand Down

0 comments on commit cb53105

Please sign in to comment.