From 54ebc1918c0249384877a9d6b2c3f12919cbff36 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Tue, 11 Apr 2017 16:47:42 -0300 Subject: [PATCH] [KEYCLOAK-3135] - Using abstract policy representation when creating policies and updating tests --- .../ResourcePolicyProviderFactory.java | 27 ++- .../scope/ScopePolicyProviderFactory.java | 27 +-- .../AbstractPolicyRepresentation.java | 2 +- .../ResourcePermissionRepresentation.java | 5 + .../ScopePermissionRepresentation.java | 5 + .../infinispan/CachedPolicyStore.java | 5 +- .../jpa/store/JPAPolicyStore.java | 17 +- .../jpa/store/JPAStoreFactory.java | 2 +- .../provider/PolicyProviderAdminService.java | 8 - .../provider/PolicyProviderFactory.java | 17 +- .../authorization/store/PolicyStore.java | 12 +- .../models/utils/ModelToRepresentation.java | 33 ++- .../models/utils/RepresentationToModel.java | 198 ++++++++---------- .../admin/PolicyResourceService.java | 21 +- .../authorization/admin/PolicyService.java | 50 ++--- .../admin/PolicyTypeResourceService.java | 26 +-- .../admin/PolicyTypeService.java | 15 +- .../admin/ResourceSetService.java | 1 - .../exportimport/util/ExportUtils.java | 2 +- .../testsuite/admin/AuthzCleanupTest.java | 15 +- .../admin/FineGrainAdminLocalTest.java | 40 ++-- .../ScopePermissionManagementTest.java | 12 +- .../authz/ConflictingScopePermissionTest.java | 2 +- .../PolicyEvaluationCompositeRoleTest.java | 43 ++-- .../AbstractPhotozAdminTest.java | 48 +++-- .../ResourcePermissionManagementTest.java | 12 +- .../resources/js/authz/authz-controller.js | 86 +++++--- .../resource-server-policy-scope-detail.html | 12 +- 28 files changed, 385 insertions(+), 358 deletions(-) diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/resource/ResourcePolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/resource/ResourcePolicyProviderFactory.java index eb18ea405e..57cb5caae7 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/resource/ResourcePolicyProviderFactory.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/resource/ResourcePolicyProviderFactory.java @@ -17,7 +17,7 @@ import org.keycloak.representations.idm.authorization.ResourcePermissionRepresen /** * @author Pedro Igor */ -public class ResourcePolicyProviderFactory implements PolicyProviderFactory { +public class ResourcePolicyProviderFactory implements PolicyProviderFactory { private ResourcePolicyProvider provider = new ResourcePolicyProvider(); @@ -36,6 +36,17 @@ public class ResourcePolicyProviderFactory implements PolicyProviderFactory { return provider; } + @Override + public Class getRepresentationType() { + return ResourcePermissionRepresentation.class; + } + + @Override + public ResourcePermissionRepresentation toRepresentation(Policy policy, ResourcePermissionRepresentation representation) { + representation.setResourceType(policy.getConfig().get("defaultResourceType")); + return representation; + } + @Override public PolicyProviderAdminService getAdminResource(ResourceServer resourceServer, AuthorizationProvider authorization) { return new PolicyProviderAdminService() { @@ -71,20 +82,6 @@ public class ResourcePolicyProviderFactory implements PolicyProviderFactory { public void onRemove(Policy policy) { } - - @Override - public Class getRepresentationType() { - return ResourcePermissionRepresentation.class; - } - - @Override - public ResourcePermissionRepresentation toRepresentation(Policy policy) { - ResourcePermissionRepresentation representation = new ResourcePermissionRepresentation(); - - representation.setResourceType(policy.getConfig().get("defaultResourceType")); - - return representation; - } }; } diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/scope/ScopePolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/scope/ScopePolicyProviderFactory.java index d85a73e514..e677a9e5e9 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/scope/ScopePolicyProviderFactory.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/scope/ScopePolicyProviderFactory.java @@ -3,9 +3,7 @@ package org.keycloak.authorization.policy.provider.scope; import org.keycloak.Config; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.Policy; -import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.policy.provider.PolicyProvider; -import org.keycloak.authorization.policy.provider.PolicyProviderAdminService; import org.keycloak.authorization.policy.provider.PolicyProviderFactory; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; @@ -14,7 +12,7 @@ import org.keycloak.representations.idm.authorization.ScopePermissionRepresentat /** * @author Pedro Igor */ -public class ScopePolicyProviderFactory implements PolicyProviderFactory { +public class ScopePolicyProviderFactory implements PolicyProviderFactory { private ScopePolicyProvider provider = new ScopePolicyProvider(); @@ -34,23 +32,18 @@ public class ScopePolicyProviderFactory implements PolicyProviderFactory { } @Override - public PolicyProviderAdminService getAdminResource(ResourceServer resourceServer, AuthorizationProvider authorization) { - return new PolicyProviderAdminService() { - @Override - public Class getRepresentationType() { - return ScopePermissionRepresentation.class; - } - - @Override - public ScopePermissionRepresentation toRepresentation(Policy policy) { - return new ScopePermissionRepresentation(); - } - }; + public PolicyProvider create(KeycloakSession session) { + return null; } @Override - public PolicyProvider create(KeycloakSession session) { - return null; + public Class getRepresentationType() { + return ScopePermissionRepresentation.class; + } + + @Override + public ScopePermissionRepresentation toRepresentation(Policy policy, ScopePermissionRepresentation representation) { + return representation; } @Override diff --git a/core/src/main/java/org/keycloak/representations/idm/authorization/AbstractPolicyRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/authorization/AbstractPolicyRepresentation.java index c4e5c99ebd..e0be8006a3 100644 --- a/core/src/main/java/org/keycloak/representations/idm/authorization/AbstractPolicyRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/authorization/AbstractPolicyRepresentation.java @@ -110,7 +110,7 @@ public class AbstractPolicyRepresentation { return scopes; } - public void addScopes(String... id) { + public void addScope(String... id) { if (this.scopes == null) { this.scopes = new HashSet<>(); } diff --git a/core/src/main/java/org/keycloak/representations/idm/authorization/ResourcePermissionRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/authorization/ResourcePermissionRepresentation.java index ebc4cec324..80f7106752 100644 --- a/core/src/main/java/org/keycloak/representations/idm/authorization/ResourcePermissionRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/authorization/ResourcePermissionRepresentation.java @@ -23,6 +23,11 @@ public class ResourcePermissionRepresentation extends AbstractPolicyRepresentati private String resourceType; + @Override + public String getType() { + return "resource"; + } + public void setResourceType(String resourceType) { this.resourceType = resourceType; } diff --git a/core/src/main/java/org/keycloak/representations/idm/authorization/ScopePermissionRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/authorization/ScopePermissionRepresentation.java index f9d9767c42..b6a02b414d 100644 --- a/core/src/main/java/org/keycloak/representations/idm/authorization/ScopePermissionRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/authorization/ScopePermissionRepresentation.java @@ -20,4 +20,9 @@ package org.keycloak.representations.idm.authorization; * @author Pedro Igor */ public class ScopePermissionRepresentation extends AbstractPolicyRepresentation { + + @Override + public String getType() { + return "scope"; + } } 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 fba7eb58dd..e8a64eb044 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 @@ -42,6 +42,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.authorization.infinispan.InfinispanStoreFactoryProvider.CacheTransaction; import org.keycloak.models.authorization.infinispan.entities.CachedPolicy; import org.keycloak.models.cache.authorization.CachedStoreFactoryProvider; +import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.Logic; @@ -74,8 +75,8 @@ public class CachedPolicyStore implements PolicyStore { } @Override - public Policy create(String name, String type, ResourceServer resourceServer) { - Policy policy = getDelegate().create(name, type, getStoreFactory().getResourceServerStore().findById(resourceServer.getId())); + public Policy create(AbstractPolicyRepresentation representation, ResourceServer resourceServer) { + Policy policy = getDelegate().create(representation, getStoreFactory().getResourceServerStore().findById(resourceServer.getId())); String id = policy.getId(); this.transaction.whenRollback(() -> { diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java index 22fb951940..527671f655 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java @@ -19,12 +19,13 @@ package org.keycloak.authorization.jpa.store; import org.keycloak.authorization.jpa.entities.PolicyEntity; import org.keycloak.authorization.jpa.entities.ResourceServerEntity; -import org.keycloak.authorization.jpa.entities.ScopeEntity; import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.ResourceServer; -import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.store.PolicyStore; +import org.keycloak.authorization.store.StoreFactory; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.models.utils.RepresentationToModel; +import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; import javax.persistence.EntityManager; import javax.persistence.NoResultException; @@ -34,7 +35,6 @@ import javax.persistence.criteria.CriteriaQuery; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -45,19 +45,22 @@ import java.util.Map; public class JPAPolicyStore implements PolicyStore { private final EntityManager entityManager; + private final StoreFactory storeFactory; - public JPAPolicyStore(EntityManager entityManager) { + public JPAPolicyStore(EntityManager entityManager, StoreFactory storeFactory) { this.entityManager = entityManager; + this.storeFactory = storeFactory; } @Override - public Policy create(String name, String type, ResourceServer resourceServer) { + public Policy create(AbstractPolicyRepresentation representation, ResourceServer resourceServer) { PolicyEntity entity = new PolicyEntity(); entity.setId(KeycloakModelUtils.generateId()); - entity.setName(name); - entity.setType(type); entity.setResourceServer((ResourceServerEntity) resourceServer); + entity.setType(representation.getType()); + + entity = (PolicyEntity) RepresentationToModel.toModel(representation, storeFactory, entity); this.entityManager.persist(entity); diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java index 5dad6af60b..1a534bbd08 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAStoreFactory.java @@ -39,7 +39,7 @@ public class JPAStoreFactory implements StoreFactory { @Override public PolicyStore getPolicyStore() { - return new JPAPolicyStore(this.entityManager); + return new JPAPolicyStore(this.entityManager, this); } @Override diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderAdminService.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderAdminService.java index fe6c8a759d..44263a71d9 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderAdminService.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderAdminService.java @@ -37,12 +37,4 @@ public interface PolicyProviderAdminService getRepresentationType() { - return null; - } } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderFactory.java index 8385964ca8..72ffb568a8 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderFactory.java @@ -19,13 +19,16 @@ package org.keycloak.authorization.policy.provider; import org.keycloak.authorization.AuthorizationProvider; +import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.provider.ProviderFactory; +import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; +import org.keycloak.representations.idm.authorization.PolicyRepresentation; /** * @author Pedro Igor */ -public interface PolicyProviderFactory extends ProviderFactory { +public interface PolicyProviderFactory extends ProviderFactory { String getName(); @@ -33,5 +36,15 @@ public interface PolicyProviderFactory extends ProviderFactory { PolicyProvider create(AuthorizationProvider authorization); - PolicyProviderAdminService getAdminResource(ResourceServer resourceServer, AuthorizationProvider authorization); + default R toRepresentation(Policy policy, R representation) { + return representation; + } + + default Class getRepresentationType() { + return (Class) PolicyRepresentation.class; + } + + default PolicyProviderAdminService getAdminResource(ResourceServer resourceServer, AuthorizationProvider authorization) { + return null; + } } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java index 626e31704b..793185e713 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java @@ -18,12 +18,13 @@ package org.keycloak.authorization.store; -import org.keycloak.authorization.model.Policy; -import org.keycloak.authorization.model.ResourceServer; - import java.util.List; import java.util.Map; +import org.keycloak.authorization.model.Policy; +import org.keycloak.authorization.model.ResourceServer; +import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; + /** * A {@link PolicyStore} is responsible to manage the persistence of {@link Policy} instances. * @@ -35,12 +36,11 @@ public interface PolicyStore { * Creates a new {@link Policy} instance. The new instance is not necessarily persisted though, which may require * a call to the {#save} method to actually make it persistent. * - * @param name the name of the policy - * @param type the type of the policy + * @param representation the policy representation * @param resourceServer the resource server to which this policy belongs * @return a new instance of {@link Policy} */ - Policy create(String name, String type, ResourceServer resourceServer); + Policy create(AbstractPolicyRepresentation representation, ResourceServer resourceServer); /** * Deletes a policy from the underlying persistence mechanism. 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 3090c6e13f..360f287e96 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 @@ -33,6 +33,7 @@ import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; +import org.keycloak.authorization.policy.provider.PolicyProviderFactory; import org.keycloak.authorization.store.ResourceStore; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.common.util.Time; @@ -87,6 +88,7 @@ import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserConsentRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.UserSessionRepresentation; +import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.representations.idm.authorization.ResourceOwnerRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; @@ -793,16 +795,29 @@ public class ModelToRepresentation { return server; } - public static PolicyRepresentation toRepresentation(Policy model) { - PolicyRepresentation representation = new PolicyRepresentation(); + public static R toRepresentation(Policy policy, Class representationType, AuthorizationProvider authorization) { + R representation; - representation.setId(model.getId()); - representation.setName(model.getName()); - representation.setDescription(model.getDescription()); - representation.setType(model.getType()); - representation.setDecisionStrategy(model.getDecisionStrategy()); - representation.setLogic(model.getLogic()); - representation.setConfig(new HashMap<>(model.getConfig())); + try { + representation = representationType.newInstance(); + } catch (Exception cause) { + throw new RuntimeException("Could not create policy [" + policy.getType() + "] representation", cause); + } + + PolicyProviderFactory providerFactory = authorization.getProviderFactory(policy.getType()); + + representation.setId(policy.getId()); + representation.setName(policy.getName()); + representation.setDescription(policy.getDescription()); + representation.setType(policy.getType()); + representation.setDecisionStrategy(policy.getDecisionStrategy()); + representation.setLogic(policy.getLogic()); + + if (representation instanceof PolicyRepresentation) { + PolicyRepresentation.class.cast(representation).setConfig(policy.getConfig()); + } else { + representation = (R) providerFactory.toRepresentation(policy, representation); + } return representation; } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 39d7380cc5..f0576ebe45 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -36,7 +36,6 @@ import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; -import org.keycloak.authorization.policy.provider.PolicyProvider; import org.keycloak.authorization.store.PolicyStore; import org.keycloak.authorization.store.ResourceServerStore; import org.keycloak.authorization.store.ResourceStore; @@ -2037,7 +2036,7 @@ public class RepresentationToModel { return newScope.getId(); }).collect(Collectors.toList()))); } catch (Exception e) { - throw new RuntimeException("Error while exporting policy [" + policyRepresentation.getName() + "].", e); + throw new RuntimeException("Error while importing policy [" + policyRepresentation.getName() + "].", e); } } @@ -2061,7 +2060,7 @@ public class RepresentationToModel { return resource.getId(); }).collect(Collectors.toList()))); } catch (Exception e) { - throw new RuntimeException("Error while exporting policy [" + policyRepresentation.getName() + "].", e); + throw new RuntimeException("Error while importing policy [" + policyRepresentation.getName() + "].", e); } } @@ -2088,118 +2087,93 @@ public class RepresentationToModel { return policy.getId(); }).collect(Collectors.toList()))); } catch (Exception e) { - throw new RuntimeException("Error while exporting policy [" + policyRepresentation.getName() + "].", e); + throw new RuntimeException("Error while importing policy [" + policyRepresentation.getName() + "].", e); } } - if (parentPolicyName == null) { - toModel(policyRepresentation, resourceServer, authorization); - } else if (parentPolicyName.equals(policyRepresentation.getName())) { - return toModel(policyRepresentation, resourceServer, authorization); + PolicyStore policyStore = storeFactory.getPolicyStore(); + Policy policy = policyStore.findById(policyRepresentation.getId(), resourceServer.getId()); + + if (policy == null) { + policy = policyStore.findByName(policyRepresentation.getName(), resourceServer.getId()); + } + + if (policy == null) { + policy = policyStore.create(policyRepresentation, resourceServer); + } else { + toModel(policyRepresentation, storeFactory, policy); + } + + if (parentPolicyName != null && parentPolicyName.equals(policyRepresentation.getName())) { + return policy; } } return null; } - public static Policy toModel(AbstractPolicyRepresentation representation, ResourceServer resourceServer, AuthorizationProvider authorization) { - String type = representation.getType(); - PolicyProvider provider = authorization.getProvider(type); + public static Policy toModel(AbstractPolicyRepresentation representation, StoreFactory storeFactory, Policy model) { + model.setName(representation.getName()); + model.setDescription(representation.getDescription()); + model.setDecisionStrategy(representation.getDecisionStrategy()); + model.setLogic(representation.getLogic()); - if (provider == null) { - //TODO: temporary, remove this check on future versions as drools type is now deprecated - if ("drools".equalsIgnoreCase(type)) { - type = "rules"; + Set resources = representation.getResources(); + Set scopes = representation.getScopes(); + Set policies = representation.getPolicies(); + + if (representation instanceof PolicyRepresentation) { + PolicyRepresentation policy = PolicyRepresentation.class.cast(representation); + String resourcesConfig = policy.getConfig().get("resources"); + + if (resourcesConfig != null) { + try { + resources = JsonSerialization.readValue(resourcesConfig, Set.class); + } catch (IOException e) { + throw new RuntimeException(e); + } } - if (authorization.getProvider(type) == null) { - throw new RuntimeException("Unknown policy type [" + type + "]. Could not find a provider for this type."); + + String scopesConfig = policy.getConfig().get("scopes"); + + if (scopesConfig != null) { + try { + scopes = JsonSerialization.readValue(scopesConfig, Set.class); + } catch (IOException e) { + throw new RuntimeException(e); + } } + + String policiesConfig = policy.getConfig().get("applyPolicies"); + + if (policiesConfig != null) { + try { + policies = JsonSerialization.readValue(policiesConfig, Set.class); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + model.setConfig(policy.getConfig()); } - PolicyStore policyStore = authorization.getStoreFactory().getPolicyStore(); - Policy model; - - if (representation.getId() != null) { - model = policyStore.findById(representation.getId(), resourceServer.getId()); - } else { - model = policyStore.findByName(representation.getName(), resourceServer.getId()); - } - - if (model != null) { - model.setName(representation.getName()); - model.setDescription(representation.getDescription()); - model.setDecisionStrategy(representation.getDecisionStrategy()); - model.setLogic(representation.getLogic()); - } else { - model = policyStore.create(representation.getName(), type, resourceServer); - model.setDescription(representation.getDescription()); - model.setDecisionStrategy(representation.getDecisionStrategy()); - model.setLogic(representation.getLogic()); - } - - updateResources(representation.getResources(), model, authorization); - updateScopes(representation.getScopes(), model, authorization); - updateAssociatedPolicies(representation.getPolicies(), model, authorization); + updateResources(resources, model, storeFactory); + updateScopes(scopes, model, storeFactory); + updateAssociatedPolicies(policies, model, storeFactory); representation.setId(model.getId()); return model; } - public static Policy toModel(PolicyRepresentation policy, ResourceServer resourceServer, AuthorizationProvider authorization) { - Policy model = toModel(AbstractPolicyRepresentation.class.cast(policy), resourceServer, authorization); - - String resources = policy.getConfig().get("resources"); - - if (resources != null) { - Set resourceIds; - - try { - resourceIds = JsonSerialization.readValue(resources, Set.class); - } catch (IOException e) { - throw new RuntimeException(e); - } - - updateResources(resourceIds, model, authorization); - } - - String scopes = policy.getConfig().get("scopes"); - - if (scopes != null) { - Set scopeIds; - - try { - scopeIds = JsonSerialization.readValue(scopes, Set.class); - } catch (IOException e) { - throw new RuntimeException(e); - } - - updateScopes(scopeIds, model, authorization); - } - - String policies = policy.getConfig().get("applyPolicies"); - - if (policies != null) { - Set policyIds; - - try { - policyIds = JsonSerialization.readValue(policies, Set.class); - } catch (IOException e) { - throw new RuntimeException(e); - } - - updateAssociatedPolicies(policyIds, model, authorization); - } - - model.setConfig(policy.getConfig()); - - return model; - } - - private static void updateScopes(Set scopeIds, Policy policy, AuthorizationProvider authorization) { + private static void updateScopes(Set scopeIds, Policy policy, StoreFactory storeFactory) { if (scopeIds != null) { - StoreFactory storeFactory = authorization.getStoreFactory(); - + if (scopeIds.isEmpty()) { + for (Scope scope : new HashSet(policy.getScopes())) { + policy.removeScope(scope); + } + return; + } for (String scopeId : scopeIds) { boolean hasScope = false; @@ -2235,16 +2209,22 @@ public class RepresentationToModel { policy.removeScope(scopeModel); } } - - policy.getConfig().remove("scopes"); } + + policy.getConfig().remove("scopes"); } - private static void updateAssociatedPolicies(Set policyIds, Policy policy, AuthorizationProvider authorization) { + private static void updateAssociatedPolicies(Set policyIds, Policy policy, StoreFactory storeFactory) { ResourceServer resourceServer = policy.getResourceServer(); if (policyIds != null) { - StoreFactory storeFactory = authorization.getStoreFactory(); + if (policyIds.isEmpty()) { + for (Policy associated: new HashSet(policy.getAssociatedPolicies())) { + policy.removeAssociatedPolicy(associated); + } + return; + } + PolicyStore policyStore = storeFactory.getPolicyStore(); for (String policyId : policyIds) { @@ -2280,18 +2260,20 @@ public class RepresentationToModel { } if (!hasPolicy) { policy.removeAssociatedPolicy(policyModel); - ; } } - - policy.getConfig().remove("applyPolicies"); } + + policy.getConfig().remove("applyPolicies"); } - private static void updateResources(Set resourceIds, Policy policy, AuthorizationProvider authorization) { + private static void updateResources(Set resourceIds, Policy policy, StoreFactory storeFactory) { if (resourceIds != null) { - StoreFactory storeFactory = authorization.getStoreFactory(); - + if (resourceIds.isEmpty()) { + for (Scope scope : new HashSet(policy.getScopes())) { + policy.removeScope(scope); + } + } for (String resourceId : resourceIds) { boolean hasResource = false; for (Resource resourceModel : new HashSet(policy.getResources())) { @@ -2326,9 +2308,13 @@ public class RepresentationToModel { policy.removeResource(resourceModel); } } - - policy.getConfig().remove("resources"); + } else { + for (Resource resourceModel : new HashSet(policy.getResources())) { + policy.removeResource(resourceModel); + } } + + policy.getConfig().remove("resources"); } public static Resource toModel(ResourceRepresentation resource, ResourceServer resourceServer, AuthorizationProvider authorization) { diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java index fe45dfab0e..5908dcfd28 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java @@ -34,7 +34,6 @@ import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.policy.provider.PolicyProviderAdminService; -import org.keycloak.authorization.policy.provider.PolicyProviderFactory; import org.keycloak.authorization.store.PolicyStore; import org.keycloak.authorization.store.StoreFactory; import org.keycloak.models.utils.ModelToRepresentation; @@ -78,7 +77,7 @@ public class PolicyResourceService { representation.setId(policy.getId()); - Policy updated = toModel(representation); + Policy updated = RepresentationToModel.toModel(representation, authorization.getStoreFactory(), policy); PolicyProviderAdminService resource = getPolicyProviderAdminResource(updated.getType()); @@ -93,10 +92,6 @@ public class PolicyResourceService { return Response.status(Status.CREATED).build(); } - protected Policy toModel(AbstractPolicyRepresentation representation) { - return RepresentationToModel.toModel(PolicyRepresentation.class.cast(representation), resourceServer, authorization); - } - @DELETE public Response delete() { this.auth.requireManage(); @@ -140,11 +135,11 @@ public class PolicyResourceService { return Response.status(Status.NOT_FOUND).build(); } - return Response.ok(toRepresentation(policy)).build(); + return Response.ok(toRepresentation(policy, authorization)).build(); } - protected Object toRepresentation(Policy model) { - return ModelToRepresentation.toRepresentation(model); + protected AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { + return ModelToRepresentation.toRepresentation(policy, PolicyRepresentation.class, authorization); } @Path("/dependentPolicies") @@ -248,13 +243,7 @@ public class PolicyResourceService { } protected PolicyProviderAdminService getPolicyProviderAdminResource(String policyType) { - PolicyProviderFactory providerFactory = authorization.getProviderFactory(policyType); - - if (providerFactory != null) { - return providerFactory.getAdminResource(resourceServer, authorization); - } - - return null; + return authorization.getProviderFactory(policyType).getAdminResource(resourceServer, authorization); } protected Policy getPolicy() { diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java index 6a9a32f0ab..4c8677f882 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java @@ -17,8 +17,6 @@ */ package org.keycloak.authorization.admin; -import static org.keycloak.models.utils.RepresentationToModel.toModel; - import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -53,7 +51,7 @@ import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; import org.keycloak.representations.idm.authorization.PolicyProviderRepresentation; import org.keycloak.representations.idm.authorization.PolicyRepresentation; -import org.keycloak.services.ErrorResponse; +import org.keycloak.services.ErrorResponseException; import org.keycloak.services.resources.admin.RealmAuth; import org.keycloak.util.JsonSerialization; @@ -97,14 +95,7 @@ public class PolicyService { this.auth.requireManage(); AbstractPolicyRepresentation representation = doCreateRepresentation(payload); - - Policy existing = authorization.getStoreFactory().getPolicyStore().findByName(representation.getName(), resourceServer.getId()); - - if (existing != null) { - return ErrorResponse.exists("Policy with name [" + representation.getName() + "] already exists"); - } - - Policy policy = doCreate(representation); + Policy policy = create(representation); PolicyProviderAdminService provider = getPolicyProviderAdminResource(representation.getType()); if (provider != null) { @@ -121,10 +112,6 @@ public class PolicyService { return Response.status(Status.CREATED).entity(representation).build(); } - protected Policy doCreate(AbstractPolicyRepresentation representation) { - return create(PolicyRepresentation.class.cast(representation)); - } - protected AbstractPolicyRepresentation doCreateRepresentation(String payload) { PolicyRepresentation representation; @@ -137,8 +124,15 @@ public class PolicyService { return representation; } - public Policy create(PolicyRepresentation representation) { - Policy policy = toModel(representation, this.resourceServer, authorization); + public Policy create(AbstractPolicyRepresentation representation) { + PolicyStore policyStore = authorization.getStoreFactory().getPolicyStore(); + Policy existing = policyStore.findByName(representation.getName(), resourceServer.getId()); + + if (existing != null) { + throw new ErrorResponseException("Policy with name [" + representation.getName() + "] already exists", "Conflicting policy", Status.CONFLICT); + } + + Policy policy = policyStore.create(representation, resourceServer); PolicyProviderAdminService resource = getPolicyProviderAdminResource(policy.getType()); if (resource != null) { @@ -152,10 +146,6 @@ public class PolicyService { return policy; } - protected Object toRepresentation(Policy model) { - return ModelToRepresentation.toRepresentation(model); - } - @Path("/search") @GET @Produces(MediaType.APPLICATION_JSON) @@ -174,7 +164,7 @@ public class PolicyService { return Response.status(Status.OK).build(); } - return Response.ok(toRepresentation(model)).build(); + return Response.ok(toRepresentation(model, authorization)).build(); } @GET @@ -251,10 +241,14 @@ public class PolicyService { .build(); } + protected AbstractPolicyRepresentation toRepresentation(Policy model, AuthorizationProvider authorization) { + return ModelToRepresentation.toRepresentation(model, PolicyRepresentation.class, authorization); + } + protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { PolicyStore policyStore = authorization.getStoreFactory().getPolicyStore(); return policyStore.findByResourceServer(filters, resourceServer.getId(), firstResult != null ? firstResult : -1, maxResult != null ? maxResult : Constants.DEFAULT_MAX_RESULTS).stream() - .map(policy -> toRepresentation(policy)) + .map(policy -> toRepresentation(policy, authorization)) .collect(Collectors.toList()); } @@ -290,16 +284,10 @@ public class PolicyService { } protected PolicyProviderAdminService getPolicyProviderAdminResource(String policyType) { - PolicyProviderFactory providerFactory = getPolicyProviderFactory(policyType); - - if (providerFactory != null) { - return providerFactory.getAdminResource(resourceServer, authorization); - } - - return null; + return getPolicyProviderFactory(policyType).getAdminResource(resourceServer, authorization); } - private PolicyProviderFactory getPolicyProviderFactory(String policyType) { + protected PolicyProviderFactory getPolicyProviderFactory(String policyType) { return authorization.getProviderFactory(policyType); } diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeResourceService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeResourceService.java index 026d7fba89..8756721e9d 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeResourceService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeResourceService.java @@ -21,8 +21,8 @@ import java.io.IOException; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.ResourceServer; -import org.keycloak.authorization.policy.provider.PolicyProviderAdminService; -import org.keycloak.models.utils.RepresentationToModel; +import org.keycloak.authorization.policy.provider.PolicyProviderFactory; +import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; import org.keycloak.services.resources.admin.RealmAuth; import org.keycloak.util.JsonSerialization; @@ -39,7 +39,7 @@ public class PolicyTypeResourceService extends PolicyResourceService { @Override protected AbstractPolicyRepresentation doCreateRepresentation(String payload) { String type = getPolicy().getType(); - Class representationType = getPolicyProviderAdminResource(type).getRepresentationType(); + Class representationType = authorization.getProviderFactory(type).getRepresentationType(); if (representationType == null) { throw new RuntimeException("Policy provider for type [" + type + "] returned a null representation type."); @@ -59,22 +59,8 @@ public class PolicyTypeResourceService extends PolicyResourceService { } @Override - protected Policy toModel(AbstractPolicyRepresentation representation) { - return RepresentationToModel.toModel(representation, resourceServer, authorization); - } - - @Override - protected Object toRepresentation(Policy policy) { - PolicyProviderAdminService provider = getPolicyProviderAdminResource(policy.getType()); - AbstractPolicyRepresentation representation = provider.toRepresentation(policy); - - representation.setId(policy.getId()); - representation.setName(policy.getName()); - representation.setDescription(policy.getDescription()); - representation.setType(policy.getType()); - representation.setDecisionStrategy(policy.getDecisionStrategy()); - representation.setLogic(policy.getLogic()); - - return representation; + protected AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { + PolicyProviderFactory providerFactory = authorization.getProviderFactory(policy.getType()); + return ModelToRepresentation.toRepresentation(policy, providerFactory.getRepresentationType(), authorization); } } diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java index 865e8db9b4..c2e4db5c55 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java @@ -16,8 +16,6 @@ */ package org.keycloak.authorization.admin; -import static org.keycloak.models.utils.RepresentationToModel.toModel; - import java.io.IOException; import javax.ws.rs.Path; @@ -27,6 +25,8 @@ import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.policy.provider.PolicyProviderAdminService; +import org.keycloak.authorization.policy.provider.PolicyProviderFactory; +import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentation; import org.keycloak.services.resources.admin.RealmAuth; import org.keycloak.util.JsonSerialization; @@ -47,6 +47,10 @@ public class PolicyTypeService extends PolicyService { public Object getPolicyAdminResourceProvider() { PolicyProviderAdminService resource = getPolicyProviderAdminResource(type); + if (resource == null) { + return null; + } + ResteasyProviderFactory.getInstance().injectProperties(resource); return resource; @@ -59,7 +63,7 @@ public class PolicyTypeService extends PolicyService { @Override protected AbstractPolicyRepresentation doCreateRepresentation(String payload) { - PolicyProviderAdminService provider = getPolicyProviderAdminResource(type); + PolicyProviderFactory provider = getPolicyProviderFactory(type); Class representationType = provider.getRepresentationType(); if (representationType == null) { @@ -80,7 +84,8 @@ public class PolicyTypeService extends PolicyService { } @Override - protected Policy doCreate(AbstractPolicyRepresentation representation) { - return toModel(representation, resourceServer, authorization); + protected AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { + PolicyProviderFactory providerFactory = authorization.getProviderFactory(policy.getType()); + return ModelToRepresentation.toRepresentation(policy, providerFactory.getRepresentationType(), authorization); } } diff --git a/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java b/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java index bb241d8805..5d6592366d 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java @@ -58,7 +58,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Function; import java.util.stream.Collectors; import static org.keycloak.models.utils.ModelToRepresentation.toRepresentation; 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 403551fc3b..51ed405e10 100755 --- a/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java +++ b/services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java @@ -337,7 +337,7 @@ public class ExportUtils { RealmModel realm = authorizationProvider.getRealm(); StoreFactory storeFactory = authorizationProvider.getStoreFactory(); try { - PolicyRepresentation rep = toRepresentation(policy); + PolicyRepresentation rep = toRepresentation(policy, PolicyRepresentation.class, authorizationProvider); rep.setId(null); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AuthzCleanupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AuthzCleanupTest.java index 6f68ae1642..de16e41634 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AuthzCleanupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AuthzCleanupTest.java @@ -29,6 +29,7 @@ import org.keycloak.models.RoleModel; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.Logic; +import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.runonserver.RunOnServerDeployment; @@ -73,15 +74,19 @@ public class AuthzCleanupTest extends AbstractKeycloakTest { } private static Policy createRolePolicy(AuthorizationProvider authz, ResourceServer resourceServer, RoleModel role) { - Policy policy = authz.getStoreFactory().getPolicyStore().create(role.getName(), "role", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); + + representation.setName(role.getName()); + representation.setType("role"); + representation.setDecisionStrategy(DecisionStrategy.UNANIMOUS); + representation.setLogic(Logic.POSITIVE); String roleValues = "[{\"id\":\"" + role.getId() + "\",\"required\": true}]"; - policy.setDecisionStrategy(DecisionStrategy.UNANIMOUS); - policy.setLogic(Logic.POSITIVE); Map config = new HashMap<>(); config.put("roles", roleValues); - policy.setConfig(config); - return policy; + representation.setConfig(config); + + return authz.getStoreFactory().getPolicyStore().create(representation, resourceServer); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminLocalTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminLocalTest.java index a533c41037..ee28edd458 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminLocalTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminLocalTest.java @@ -39,6 +39,8 @@ import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.Logic; import org.keycloak.representations.idm.authorization.PolicyEvaluationRequest; import org.keycloak.representations.idm.authorization.PolicyEvaluationResponse; +import org.keycloak.representations.idm.authorization.PolicyRepresentation; +import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import java.util.HashMap; @@ -102,21 +104,16 @@ public class FineGrainAdminLocalTest extends AbstractKeycloakTest { } private static Policy addScopePermission(AuthorizationProvider authz, ResourceServer resourceServer, String name, Resource resource, Scope scope, Policy policy) { - Policy permission = authz.getStoreFactory().getPolicyStore().create(name, "scope", resourceServer); - String resources = "[\"" + resource.getId() + "\"]"; - String scopes = "[\"" + scope.getId() + "\"]"; - String applyPolicies = "[\"" + policy.getId() + "\"]"; - Map config = new HashMap<>(); - config.put("resources", resources); - config.put("scopes", scopes); - config.put("applyPolicies", applyPolicies); - permission.setConfig(config); - permission.setDecisionStrategy(DecisionStrategy.UNANIMOUS); - permission.setLogic(Logic.POSITIVE); - permission.addResource(resource); - permission.addScope(scope); - permission.addAssociatedPolicy(policy); - return permission; + ScopePermissionRepresentation representation = new ScopePermissionRepresentation(); + + representation.setName(name); + representation.setDecisionStrategy(DecisionStrategy.UNANIMOUS); + representation.setLogic(Logic.POSITIVE); + representation.addResource(resource.getName()); + representation.addScope(scope.getName()); + representation.addPolicy(policy.getName()); + + return authz.getStoreFactory().getPolicyStore().create(representation, resourceServer); } private static Resource createRoleResource(AuthorizationProvider authz, ResourceServer resourceServer, RoleModel role) { @@ -144,15 +141,18 @@ public class FineGrainAdminLocalTest extends AbstractKeycloakTest { roleName = client.getClientId() ; } roleName = "role.policy." + roleName + "." + role.getName(); - Policy policy = authz.getStoreFactory().getPolicyStore().create(roleName, "role", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); + representation.setName(roleName); + representation.setType("role"); + representation.setDecisionStrategy(DecisionStrategy.UNANIMOUS); + representation.setLogic(Logic.POSITIVE); String roleValues = "[{\"id\":\"" + role.getId() + "\",\"required\": true}]"; - policy.setDecisionStrategy(DecisionStrategy.UNANIMOUS); - policy.setLogic(Logic.POSITIVE); Map config = new HashMap<>(); config.put("roles", roleValues); - policy.setConfig(config); - return policy; + representation.setConfig(config); + + return authz.getStoreFactory().getPolicyStore().create(representation, resourceServer); } public static void setupUsers(KeycloakSession session) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopePermissionManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopePermissionManagementTest.java index 8759a0a9f2..a833668ec2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopePermissionManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopePermissionManagementTest.java @@ -19,8 +19,6 @@ package org.keycloak.testsuite.admin.client.authorization; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; -import java.util.Collections; - import javax.ws.rs.NotFoundException; import javax.ws.rs.core.Response; @@ -47,7 +45,7 @@ public class ScopePermissionManagementTest extends AbstractPermissionManagementT representation.setDecisionStrategy(DecisionStrategy.CONSENSUS); representation.setLogic(Logic.NEGATIVE); representation.addResource("Resource A"); - representation.addScopes("read", "execute"); + representation.addScope("read", "execute"); representation.addPolicy("Only Marta Policy", "Only Kolo Policy"); assertCreated(authorization, representation); @@ -62,7 +60,7 @@ public class ScopePermissionManagementTest extends AbstractPermissionManagementT representation.setDescription("description"); representation.setDecisionStrategy(DecisionStrategy.CONSENSUS); representation.setLogic(Logic.NEGATIVE); - representation.addScopes("read", "write"); + representation.addScope("read", "write"); representation.addPolicy("Only Marta Policy"); assertCreated(authorization, representation); @@ -78,7 +76,7 @@ public class ScopePermissionManagementTest extends AbstractPermissionManagementT representation.setDecisionStrategy(DecisionStrategy.CONSENSUS); representation.setLogic(Logic.NEGATIVE); representation.addResource("Resource A"); - representation.addScopes("read", "execute"); + representation.addScope("read", "execute"); representation.addPolicy("Only Marta Policy", "Only Kolo Policy"); assertCreated(authorization, representation); @@ -106,7 +104,7 @@ public class ScopePermissionManagementTest extends AbstractPermissionManagementT ScopePermissionRepresentation representation = new ScopePermissionRepresentation(); representation.setName("Test Delete Permission"); - representation.addScopes("execute"); + representation.addScope("execute"); representation.addPolicy("Only Marta Policy"); assertCreated(authorization, representation); @@ -131,7 +129,7 @@ public class ScopePermissionManagementTest extends AbstractPermissionManagementT ScopePermissionRepresentation permission1 = new ScopePermissionRepresentation(); permission1.setName("Conflicting Name Permission"); - permission1.addScopes("read"); + permission1.addScope("read"); permission1.addPolicy("Only Marta Policy"); ScopePermissionsResource permissions = authorization.permissions().scope(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/ConflictingScopePermissionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/ConflictingScopePermissionTest.java index 576a472fc6..6db4891238 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/ConflictingScopePermissionTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/ConflictingScopePermissionTest.java @@ -213,7 +213,7 @@ public class ConflictingScopePermissionTest extends AbstractKeycloakTest { representation.addResource(resourceName); } - representation.addScopes(scopes.toArray(new String[scopes.size()])); + representation.addScope(scopes.toArray(new String[scopes.size()])); representation.addPolicy(scopes.toArray(new String[policies.size()])); authorization.permissions().scope().create(representation); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationCompositeRoleTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationCompositeRoleTest.java index 98d33663a7..74fcb7bd0b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationCompositeRoleTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationCompositeRoleTest.java @@ -24,9 +24,7 @@ import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; -import org.keycloak.models.AdminRoles; import org.keycloak.models.ClientModel; -import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; @@ -38,6 +36,8 @@ import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.Logic; import org.keycloak.representations.idm.authorization.PolicyEvaluationRequest; import org.keycloak.representations.idm.authorization.PolicyEvaluationResponse; +import org.keycloak.representations.idm.authorization.PolicyRepresentation; +import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import java.util.HashMap; @@ -82,34 +82,33 @@ public class PolicyEvaluationCompositeRoleTest extends AbstractKeycloakTest { } private static Policy addScopePermission(AuthorizationProvider authz, ResourceServer resourceServer, String name, Resource resource, Scope scope, Policy policy) { - Policy permission = authz.getStoreFactory().getPolicyStore().create(name, "scope", resourceServer); - String resources = "[\"" + resource.getId() + "\"]"; - String scopes = "[\"" + scope.getId() + "\"]"; - String applyPolicies = "[\"" + policy.getId() + "\"]"; - Map config = new HashMap<>(); - config.put("resources", resources); - config.put("scopes", scopes); - config.put("applyPolicies", applyPolicies); - permission.setConfig(config); - permission.setDecisionStrategy(DecisionStrategy.UNANIMOUS); - permission.setLogic(Logic.POSITIVE); - permission.addResource(resource); - permission.addScope(scope); - permission.addAssociatedPolicy(policy); - return permission; + ScopePermissionRepresentation representation = new ScopePermissionRepresentation(); + + representation.setName(name); + representation.setType("scope"); + representation.addResource(resource.getName()); + representation.addScope(scope.getName()); + representation.addPolicy(policy.getName()); + representation.setDecisionStrategy(DecisionStrategy.UNANIMOUS); + representation.setLogic(Logic.POSITIVE); + + return authz.getStoreFactory().getPolicyStore().create(representation, resourceServer); } private static Policy createRolePolicy(AuthorizationProvider authz, ResourceServer resourceServer, RoleModel role) { - Policy policy = authz.getStoreFactory().getPolicyStore().create(role.getName(), "role", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); + representation.setName(role.getName()); + representation.setType("role"); + representation.setDecisionStrategy(DecisionStrategy.UNANIMOUS); + representation.setLogic(Logic.POSITIVE); String roleValues = "[{\"id\":\"" + role.getId() + "\",\"required\": true}]"; - policy.setDecisionStrategy(DecisionStrategy.UNANIMOUS); - policy.setLogic(Logic.POSITIVE); Map config = new HashMap<>(); config.put("roles", roleValues); - policy.setConfig(config); - return policy; + representation.setConfig(config); + + return authz.getStoreFactory().getPolicyStore().create(representation, resourceServer); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/AbstractPhotozAdminTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/AbstractPhotozAdminTest.java index 642e116fd9..f3c432ba15 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/AbstractPhotozAdminTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/AbstractPhotozAdminTest.java @@ -38,6 +38,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.representations.AccessToken; +import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.representations.idm.authorization.ScopeRepresentation; import org.keycloak.util.JsonSerialization; @@ -246,10 +247,14 @@ public abstract class AbstractPhotozAdminTest extends AbstractAuthorizationTest return onAuthorizationSession(authorizationProvider -> { StoreFactory storeFactory = authorizationProvider.getStoreFactory(); PolicyStore policyStore = storeFactory.getPolicyStore(); - Policy policy = policyStore.create("Administration Policy", "aggregate", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); - policy.addAssociatedPolicy(anyAdminPolicy); - policy.addAssociatedPolicy(onlyFromSpecificAddressPolicy); + representation.setName("Administration Policy"); + representation.setType("aggregate"); + representation.addPolicy(anyAdminPolicy.getName()); + representation.addPolicy(onlyFromSpecificAddressPolicy.getName()); + + Policy policy = policyStore.create(representation, resourceServer); return policy; }); @@ -259,19 +264,22 @@ public abstract class AbstractPhotozAdminTest extends AbstractAuthorizationTest return onAuthorizationSession(authorizationProvider -> { StoreFactory storeFactory = authorizationProvider.getStoreFactory(); PolicyStore policyStore = storeFactory.getPolicyStore(); - Policy policy = policyStore.create("Only From a Specific Client Address", "js", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); + + representation.setName("Only From a Specific Client Address"); + representation.setType("js"); HashedMap config = new HashedMap(); config.put("code", "var contextAttributes = $evaluation.getContext().getAttributes();" + - "var networkAddress = contextAttributes.getValue('kc.client.network.ip_address');" + - "if ('127.0.0.1'.equals(networkAddress.asInetAddress(0).getHostAddress())) {" + - "$evaluation.grant();" + - "}"); + "var networkAddress = contextAttributes.getValue('kc.client.network.ip_address');" + + "if ('127.0.0.1'.equals(networkAddress.asInetAddress(0).getHostAddress())) {" + + "$evaluation.grant();" + + "}"); - policy.setConfig(config); + representation.setConfig(config); - return policy; + return policyStore.create(representation, resourceServer); }); } @@ -279,7 +287,11 @@ public abstract class AbstractPhotozAdminTest extends AbstractAuthorizationTest return onAuthorizationSession(authorizationProvider -> { StoreFactory storeFactory = authorizationProvider.getStoreFactory(); PolicyStore policyStore = storeFactory.getPolicyStore(); - Policy policy = policyStore.create("Any Admin Policy", "role", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); + + representation.setName("Any Admin Policy"); + representation.setType("role"); + HashedMap config = new HashedMap(); RealmModel realm = authorizationProvider.getKeycloakSession().realms().getRealmByName(TEST_REALM_NAME); RoleModel adminRole = realm.getRole("admin"); @@ -294,9 +306,9 @@ public abstract class AbstractPhotozAdminTest extends AbstractAuthorizationTest throw new RuntimeException(e); } - policy.setConfig(config); + representation.setConfig(config); - return policy; + return policyStore.create(representation, resourceServer); }); } @@ -358,7 +370,11 @@ public abstract class AbstractPhotozAdminTest extends AbstractAuthorizationTest return onAuthorizationSession(authorizationProvider -> { StoreFactory storeFactory = authorizationProvider.getStoreFactory(); PolicyStore policyStore = storeFactory.getPolicyStore(); - Policy policy = policyStore.create("Any User Policy", "role", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); + + representation.setName("Any User Policy"); + representation.setType("role"); + HashedMap config = new HashedMap(); RealmModel realm = authorizationProvider.getKeycloakSession().realms().getRealmByName(TEST_REALM_NAME); RoleModel userRole = realm.getRole("user"); @@ -373,7 +389,9 @@ public abstract class AbstractPhotozAdminTest extends AbstractAuthorizationTest throw new RuntimeException(e); } - policy.setConfig(config); + representation.setConfig(config); + + Policy policy = policyStore.create(representation, resourceServer); return policy; }); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/ResourcePermissionManagementTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/ResourcePermissionManagementTest.java index 4708a2a2c8..be17a80ae3 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/ResourcePermissionManagementTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/ResourcePermissionManagementTest.java @@ -46,12 +46,9 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Function; -import java.util.function.Predicate; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -392,7 +389,10 @@ public class ResourcePermissionManagementTest extends AbstractPhotozAdminTest { return onAuthorizationSession(authorizationProvider -> { StoreFactory storeFactory = authorizationProvider.getStoreFactory(); PolicyStore policyStore = storeFactory.getPolicyStore(); - Policy policy = policyStore.create("Client-Based Policy", "client", resourceServer); + PolicyRepresentation representation = new PolicyRepresentation(); + + representation.setName("Client-Based Policy"); + representation.setType("client"); List clientIds = new ArrayList<>(); for (ClientModel client : allowedClients) { @@ -408,9 +408,9 @@ public class ResourcePermissionManagementTest extends AbstractPhotozAdminTest { throw new RuntimeException(e); } - policy.setConfig(config); + representation.setConfig(config); - return policy; + return policyStore.create(representation, resourceServer); }); } diff --git a/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js b/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js index 5cb9f1fb1c..bad0bb2e89 100644 --- a/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js @@ -1078,11 +1078,11 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route $scope.selectResource = function() { $scope.selectedScopes = null; - if ($scope.policy.resources) { + if ($scope.selectedResource) { ResourceServerResource.scopes({ realm: $route.current.params.realm, client: client.id, - rsrid: $scope.policy.resources._id + rsrid: $scope.selectedResource._id }, function (data) { $scope.resourceScopes = data; }); @@ -1091,7 +1091,6 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route }, onInitUpdate : function(policy) { - $scope.selectedScopes = []; ResourceServerPolicy.resources({ realm : $route.current.params.realm, client : client.id, @@ -1111,29 +1110,48 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route deep: false }, function (resource) { resource[0].text = resource[0].name; - $scope.policy.resources = resource[0]; + $scope.selectedResource = resource[0]; + var copy = angular.copy($scope.selectedResource); + $scope.$watch('selectedResource', function() { + if (!angular.equals($scope.selectedResource, copy)) { + $scope.changed = true; + } + }, true); ResourceServerResource.scopes({ realm: $route.current.params.realm, client: client.id, rsrid: resource[0]._id }, function (scopes) { $scope.resourceScopes = scopes; - }); - ResourceServerPolicy.scopes({ - realm: $route.current.params.realm, - client: client.id, - id: policy.id - }, function (scopes) { - $scope.selectedScopes = []; - for (i = 0; i < scopes.length; i++) { - $scope.selectedScopes.push(scopes[i].id); - } + ResourceServerPolicy.scopes({ + realm : $route.current.params.realm, + client : client.id, + id : policy.id + }, function(scopes) { + $scope.selectedScopes = []; + for (i = 0; i < scopes.length; i++) { + scopes[i].text = scopes[i].name; + $scope.selectedScopes.push(scopes[i].id); + } + var copy = angular.copy($scope.selectedScopes); + $scope.$watch('selectedScopes', function() { + if (!angular.equals($scope.selectedScopes, copy)) { + $scope.changed = true; + } + }, true); + }); }); }); }); } } else { - $scope.policy.resources = null; + $scope.selectedResource = null; + var copy = angular.copy($scope.selectedResource); + $scope.$watch('selectedResource', function() { + if (!angular.equals($scope.selectedResource, copy)) { + $scope.changed = true; + } + }, true); ResourceServerPolicy.scopes({ realm : $route.current.params.realm, client : client.id, @@ -1144,26 +1162,38 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route scopes[i].text = scopes[i].name; $scope.selectedScopes.push(scopes[i]); } + var copy = angular.copy($scope.selectedScopes); + $scope.$watch('selectedScopes', function() { + if (!angular.equals($scope.selectedScopes, copy)) { + $scope.changed = true; + } + }, true); }); } }); - policy.policies = []; ResourceServerPolicy.associatedPolicies({ realm : $route.current.params.realm, client : client.id, id : policy.id }, function(policies) { + $scope.selectedPolicies = []; for (i = 0; i < policies.length; i++) { policies[i].text = policies[i].name; - $scope.policy.policies.push(policies[i]); + $scope.selectedPolicies.push(policies[i]); } + var copy = angular.copy($scope.selectedPolicies); + $scope.$watch('selectedPolicies', function() { + if (!angular.equals($scope.selectedPolicies, copy)) { + $scope.changed = true; + } + }, true); }); }, onUpdate : function() { - if ($scope.policy.resources != null) { - $scope.policy.resources = [$scope.policy.resources._id]; + if ($scope.selectedResource != null) { + $scope.policy.resources = [$scope.selectedResource._id]; } else { delete $scope.policy.resources; } @@ -1182,16 +1212,16 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route var policies = []; - for (i = 0; i < $scope.policy.policies.length; i++) { - policies.push($scope.policy.policies[i].id); + for (i = 0; i < $scope.selectedPolicies.length; i++) { + policies.push($scope.selectedPolicies[i].id); } $scope.policy.policies = policies; + delete $scope.policy.config; }, onInitCreate : function(newPolicy) { newPolicy.decisionStrategy = 'UNANIMOUS'; - newPolicy.resources = null; var scopeId = $location.search()['scpid']; @@ -1203,16 +1233,16 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route }, function (data) { data.text = data.name; if (!$scope.policy.scopes) { - $scope.policy.scopes = []; + $scope.selectedScopes = []; } - $scope.policy.scopes.push(data); + $scope.selectedScopes.push(data); }); } }, onCreate : function() { - if ($scope.policy.resources != null) { - $scope.policy.resources = [$scope.policy.resources._id]; + if ($scope.selectedResource != null) { + $scope.policy.resources = [$scope.selectedResource._id]; } var scopes = []; @@ -1229,8 +1259,8 @@ module.controller('ResourceServerPolicyScopeDetailCtrl', function($scope, $route var policies = []; - for (i = 0; i < $scope.policy.policies.length; i++) { - policies.push($scope.policy.policies[i].id); + for (i = 0; i < $scope.selectedPolicies.length; i++) { + policies.push($scope.selectedPolicies[i].id); } $scope.policy.policies = policies; diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/authz/permission/provider/resource-server-policy-scope-detail.html b/themes/src/main/resources/theme/base/admin/resources/partials/authz/permission/provider/resource-server-policy-scope-detail.html index 1754fe5b73..5da3a12798 100644 --- a/themes/src/main/resources/theme/base/admin/resources/partials/authz/permission/provider/resource-server-policy-scope-detail.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/authz/permission/provider/resource-server-policy-scope-detail.html @@ -32,28 +32,28 @@
- +
{{:: 'authz-permission-scope-resource.tooltip' | translate}} -
+
{{:: 'authz-permission-scope-scope.tooltip' | translate}}
-
+
- +
{{:: 'authz-permission-scope-scope.tooltip' | translate}}
@@ -61,7 +61,7 @@
- +
{{:: 'authz-policy-apply-policy.tooltip' | translate}}