[KEYCLOAK-6621] - Fixing cache and queries of policies with type scope

This commit is contained in:
pedroigor 2018-02-28 14:47:13 -03:00
parent b0200d462d
commit cb531056a6
12 changed files with 171 additions and 27 deletions

View file

@ -48,8 +48,9 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
@Override
public Policy getDelegateForUpdate() {
if (updated == null) {
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), 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");
}
return updated;
@ -97,7 +98,7 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
@Override
public void setName(String name) {
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);
}
@ -148,6 +149,10 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
@Override
public void setConfig(Map<String, String> config) {
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);
}
@ -155,6 +160,9 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
@Override
public void removeConfig(String name) {
getDelegateForUpdate();
if (name.equals("defaultResourceType")) {
cacheSession.registerPolicyInvalidation(cached.getId(), cached.getName(), cached.getResourcesIds(), cached.getScopesIds(), cached.getConfig().get("defaultResourceType"), cached.getResourceServerId());
}
updated.removeConfig(name);
}
@ -162,6 +170,10 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
@Override
public void putConfig(String name, String value) {
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);
}
@ -207,14 +219,14 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
@Override
public void addScope(Scope scope) {
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);
}
@Override
public void removeScope(Scope scope) {
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);
}
@ -237,7 +249,7 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
getDelegateForUpdate();
HashSet<String> resources = new HashSet<>();
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);
}
@ -247,7 +259,7 @@ public class PolicyAdapter implements Policy, CachedModel<Policy> {
getDelegateForUpdate();
HashSet<String> resources = new HashSet<>();
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);
}

View file

@ -27,6 +27,7 @@ import org.keycloak.models.cache.infinispan.authorization.stream.InScopePredicat
import org.keycloak.models.cache.infinispan.entities.Revisioned;
import org.keycloak.models.cache.infinispan.events.InvalidationEvent;
import java.util.Objects;
import java.util.Set;
/**
@ -112,6 +113,11 @@ public class StoreFactoryCacheManager extends CacheManager {
if (resources != null) {
for (String resource : resources) {
invalidations.add(StoreFactoryCacheSession.getPolicyByResource(resource, serverId));
if (Objects.nonNull(scopes)) {
for (String scope : scopes) {
invalidations.add(StoreFactoryCacheSession.getPolicyByResourceScope(scope, resource, serverId));
}
}
}
}
@ -124,6 +130,7 @@ public class StoreFactoryCacheManager extends CacheManager {
if (scopes != null) {
for (String scope : scopes) {
invalidations.add(StoreFactoryCacheSession.getPolicyByScope(scope, serverId));
invalidations.add(StoreFactoryCacheSession.getPolicyByResourceScope(scope, null, serverId));
}
}
}

View file

@ -271,8 +271,11 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider {
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);
if (Objects.nonNull(defaultResourceType)) {
resourceTypes.add(defaultResourceType);
}
cache.policyUpdated(id, name, resources, resourceTypes, scopes, serverId, invalidations);
PolicyAdapter adapter = managedPolicies.get(id);
if (adapter != null) adapter.invalidateFlag();
@ -369,6 +372,10 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider {
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) {
return "permission.ticket.resource." + resourceId + "." + serverId;
}
@ -664,8 +671,9 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider {
@Override
public Policy create(AbstractPolicyRepresentation representation, ResourceServer resourceServer) {
Policy policy = getPolicyStoreDelegate().create(representation, resourceServer);
registerPolicyInvalidation(policy.getId(), representation.getName(), representation.getResources(), representation.getScopes(), resourceServer.getId());
return policy;
Policy cached = findById(policy.getId(), resourceServer.getId());
registerPolicyInvalidation(policy.getId(), representation.getName(), representation.getResources(), representation.getScopes(), null, resourceServer.getId());
return cached;
}
@Override
@ -678,6 +686,10 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider {
Set<String> resources = policy.getResources().stream().map(resource -> resource.getId()).collect(Collectors.toSet());
ResourceServer resourceServer = policy.getResourceServer();
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());
invalidationEvents.add(PolicyRemovedEvent.create(id, policy.getName(), resources, resourceTypes, scopes, resourceServer.getId()));
cache.policyRemoval(id, policy.getName(), resources, resourceTypes, scopes, resourceServer.getId(), invalidations);
@ -771,6 +783,19 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider {
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
public List<Policy> findByType(String type, String resourceServerId) {
return getPolicyStoreDelegate().findByType(type, resourceServerId);

View file

@ -57,6 +57,8 @@ import org.keycloak.representations.idm.authorization.Logic;
@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="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="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)"),

View file

@ -22,6 +22,7 @@ import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import javax.persistence.EntityManager;
import javax.persistence.FlushModeType;
@ -114,7 +115,10 @@ public class JPAPolicyStore implements PolicyStore {
List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
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;
}
@ -157,7 +161,10 @@ public class JPAPolicyStore implements PolicyStore {
List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
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;
}
@ -173,7 +180,10 @@ public class JPAPolicyStore implements PolicyStore {
List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
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;
}
@ -189,7 +199,10 @@ public class JPAPolicyStore implements PolicyStore {
List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
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;
}
@ -210,7 +223,41 @@ public class JPAPolicyStore implements PolicyStore {
List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
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;
}
@ -226,7 +273,10 @@ public class JPAPolicyStore implements PolicyStore {
List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
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;
}
@ -243,7 +293,10 @@ public class JPAPolicyStore implements PolicyStore {
List<String> result = query.getResultList();
List<Policy> list = new LinkedList<>();
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;
}

View file

@ -385,6 +385,11 @@ public final class AuthorizationProvider implements Provider {
return policyStore.findByScopeIds(scopeIds, resourceServerId);
}
@Override
public List<Policy> findByScopeIds(List<String> scopeIds, String resourceId, String resourceServerId) {
return policyStore.findByScopeIds(scopeIds, resourceId, resourceServerId);
}
@Override
public List<Policy> findByType(String type, String resourceServerId) {
return policyStore.findByType(type, resourceServerId);

View file

@ -79,8 +79,10 @@ public class DefaultPolicyEvaluator implements PolicyEvaluator {
evaluatePolicies(() -> {
List<Policy> policies = policyStore.findByResourceType(resource.getType(), resourceServer.getId());
for (Resource typedResource : resourceStore.findByType(resource.getType(), resourceServer.getId())) {
policies.addAll(policyStore.findByResource(typedResource.getId(), resourceServer.getId()));
if (!resource.getOwner().equals(resourceServer.getId())) {
for (Resource typedResource : resourceStore.findByType(resource.getType(), resourceServer.getId())) {
policies.addAll(policyStore.findByResource(typedResource.getId(), resourceServer.getId()));
}
}
return policies;
@ -89,7 +91,7 @@ public class DefaultPolicyEvaluator implements PolicyEvaluator {
}
if (!scopes.isEmpty()) {
evaluatePolicies(() -> policyStore.findByScopeIds(scopes.stream().map(Scope::getId).collect(Collectors.toList()), resourceServer.getId()), consumer);
evaluatePolicies(() -> policyStore.findByScopeIds(scopes.stream().map(Scope::getId).collect(Collectors.toList()), null, resourceServer.getId()), consumer);
}
if (PolicyEnforcementMode.PERMISSIVE.equals(enforcementMode) && !verified.get()) {

View file

@ -111,6 +111,16 @@ public interface PolicyStore {
*/
List<Policy> findByScopeIds(List<String> scopeIds, String resourceServerId);
/**
* Returns a list of {@link Policy} associated with a {@link org.keycloak.authorization.core.model.Scope} with the given <code>resourceId</code> and <code>scopeIds</code>.
*
* @param scopeIds the id of the scopes
* @param resourceId the id of the resource
* @param resourceServerId the resource server id
* @return a list of policies associated with the given scopes
*/
List<Policy> findByScopeIds(List<String> scopeIds, String resourceId, String resourceServerId);
/**
* Returns a list of {@link Policy} with the given <code>type</code>.
*

View file

@ -234,7 +234,7 @@ public class ResourceSetService {
return representation;
}).collect(Collectors.toList());
if (model.getType() != null) {
if (model.getType() != null && !model.getOwner().equals(resourceServer.getId())) {
ResourceStore resourceStore = authorization.getStoreFactory().getResourceStore();
for (Resource typed : resourceStore.findByType(model.getType(), resourceServer.getId())) {
if (typed.getOwner().equals(resourceServer.getId()) && !typed.getId().equals(model.getId())) {
@ -273,7 +273,8 @@ public class ResourceSetService {
policies.addAll(policyStore.findByResource(model.getId(), resourceServer.getId()));
policies.addAll(policyStore.findByResourceType(model.getType(), resourceServer.getId()));
policies.addAll(policyStore.findByScopeIds(model.getScopes().stream().map(scope -> scope.getId()).collect(Collectors.toList()), resourceServer.getId()));
policies.addAll(policyStore.findByScopeIds(model.getScopes().stream().map(scope -> scope.getId()).collect(Collectors.toList()), id, resourceServer.getId()));
policies.addAll(policyStore.findByScopeIds(model.getScopes().stream().map(scope -> scope.getId()).collect(Collectors.toList()), null, resourceServer.getId()));
List<PolicyRepresentation> representation = new ArrayList<>();

View file

@ -44,6 +44,7 @@ import org.keycloak.testsuite.console.page.fragment.ModalDialog;
import org.keycloak.testsuite.console.page.fragment.MultipleStringSelect2;
import org.keycloak.testsuite.console.page.fragment.SingleStringSelect2;
import org.keycloak.testsuite.page.Form;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;
import org.openqa.selenium.support.ui.Select;
@ -115,6 +116,7 @@ public class ScopePermissionForm extends Form {
resourceSelect.update(resources);
resourceScopeSelect.update(expected.getScopes());
} else {
driver.findElement(By.className("select2-search-choice-close")).click();
scopeSelect.update(expected.getScopes());
}

View file

@ -88,7 +88,7 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
}
@Test
public void testCreateWithoutPolicies() throws InterruptedException {
public void testCreateWithoutPolicies() {
authorizationPage.navigateTo();
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
@ -105,7 +105,7 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
}
@Test
public void testUpdateResourceScope() throws InterruptedException {
public void testUpdateResourceScope() {
authorizationPage.navigateTo();
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
@ -149,7 +149,32 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
}
@Test
public void testUpdateScopeOnly() throws InterruptedException {
public void testUpdateWithoutResource() {
authorizationPage.navigateTo();
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
expected.setName("testUpdateWithoutResource Permission");
expected.setDescription("description");
expected.addResource("Resource A");
expected.addScope("Scope A");
expected.addPolicy("Policy C");
expected = createPermission(expected);
expected.getResources().clear();
expected.addScope("Scope B");
authorizationPage.navigateTo();
authorizationPage.authorizationTabs().permissions().update(expected.getName(), expected);
assertAlertSuccess();
authorizationPage.navigateTo();
ScopePermission actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
assertPolicy(expected, actual);
}
@Test
public void testUpdateScopeOnly() {
authorizationPage.navigateTo();
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
@ -180,7 +205,7 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
}
@Test
public void testDelete() throws InterruptedException {
public void testDelete() {
authorizationPage.navigateTo();
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
@ -198,7 +223,7 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
}
@Test
public void testDeleteFromList() throws InterruptedException {
public void testDeleteFromList() {
authorizationPage.navigateTo();
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();

View file

@ -1292,7 +1292,7 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route
if ($scope.selectedResource != null) {
$scope.policy.resources = [$scope.selectedResource._id];
} else {
delete $scope.policy.resources;
$scope.policy.resources = [];
}
var scopes = [];