From 709cbfd4b71390514a4535af419c2bb0aa0a7ef2 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 13 Dec 2019 19:18:13 -0300 Subject: [PATCH] [KEYCLOAK-10705] - Return full resource representation when querying policies by id --- .../AbstractPolicyRepresentation.java | 24 ++++++++++++ .../client/resource/PoliciesResource.java | 14 +++++++ .../admin/client/resource/PolicyResource.java | 6 +++ .../models/utils/ModelToRepresentation.java | 11 ++++++ .../admin/PermissionService.java | 12 +++--- .../admin/PolicyResourceService.java | 13 +++++-- .../authorization/admin/PolicyService.java | 15 +++---- .../admin/PolicyTypeResourceService.java | 5 +-- .../admin/PolicyTypeService.java | 6 +-- .../policy/UserManagedPermissionService.java | 4 +- .../GenericPolicyManagementTest.java | 39 ++++++++++++++++++- 11 files changed, 123 insertions(+), 26 deletions(-) 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 8d7bbeb9ba..ed9bc9c30e 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 @@ -21,6 +21,8 @@ import java.util.HashSet; import java.util.Objects; import java.util.Set; +import com.fasterxml.jackson.annotation.JsonInclude; + /** * @author Pedro Igor */ @@ -36,6 +38,12 @@ public class AbstractPolicyRepresentation { private Logic logic = Logic.POSITIVE; private DecisionStrategy decisionStrategy = DecisionStrategy.UNANIMOUS; private String owner; + + @JsonInclude(JsonInclude.Include.NON_EMPTY) + private Set resourcesData; + + @JsonInclude(JsonInclude.Include.NON_EMPTY) + private Set scopesData; public String getId() { return this.id; @@ -162,4 +170,20 @@ public class AbstractPolicyRepresentation { public int hashCode() { return Objects.hash(getId()); } + + public void setResourcesData(Set resources) { + this.resourcesData = resources; + } + + public Set getResourcesData() { + return resourcesData; + } + + public void setScopesData(Set scopesData) { + this.scopesData = scopesData; + } + + public Set getScopesData() { + return scopesData; + } } \ No newline at end of file diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PoliciesResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PoliciesResource.java index 2f8af928c3..411dba91f4 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PoliciesResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PoliciesResource.java @@ -57,6 +57,20 @@ public interface PoliciesResource { @NoCache List policies(); + @GET + @Produces(MediaType.APPLICATION_JSON) + @NoCache + List policies(@QueryParam("policyId") String id, + @QueryParam("name") String name, + @QueryParam("type") String type, + @QueryParam("resource") String resource, + @QueryParam("scope") String scope, + @QueryParam("permission") Boolean permission, + @QueryParam("owner") String owner, + @QueryParam("fields") String fields, + @QueryParam("first") Integer firstResult, + @QueryParam("max") Integer maxResult); + @Path("providers") @GET @Produces(MediaType.APPLICATION_JSON) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PolicyResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PolicyResource.java index 28202f5676..7287795750 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PolicyResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/PolicyResource.java @@ -24,6 +24,7 @@ import javax.ws.rs.GET; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; import org.jboss.resteasy.annotations.cache.NoCache; @@ -41,6 +42,11 @@ public interface PolicyResource { @NoCache PolicyRepresentation toRepresentation(); + @GET + @Produces(MediaType.APPLICATION_JSON) + @NoCache + PolicyRepresentation toRepresentation(@QueryParam("fields") String fields); + @PUT @Consumes(MediaType.APPLICATION_JSON) void update(PolicyRepresentation representation); 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 6358ef207d..e817704d93 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 @@ -817,6 +817,10 @@ public class ModelToRepresentation { } public static R toRepresentation(Policy policy, AuthorizationProvider authorization, boolean genericRepresentation, boolean export) { + return toRepresentation(policy, authorization, genericRepresentation, export, false); + } + + public static R toRepresentation(Policy policy, AuthorizationProvider authorization, boolean genericRepresentation, boolean export, boolean allFields) { PolicyProviderFactory providerFactory = authorization.getProviderFactory(policy.getType()); R representation; @@ -840,6 +844,13 @@ public class ModelToRepresentation { representation.setType(policy.getType()); representation.setDecisionStrategy(policy.getDecisionStrategy()); representation.setLogic(policy.getLogic()); + + if (allFields) { + representation.setResourcesData(policy.getResources().stream().map( + resource -> toRepresentation(resource, resource.getResourceServer(), authorization, true)).collect(Collectors.toSet())); + representation.setScopesData(policy.getScopes().stream().map( + resource -> toRepresentation(resource)).collect(Collectors.toSet())); + } return representation; } diff --git a/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java b/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java index b2772f9fa8..89212b679a 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java @@ -45,22 +45,22 @@ public class PermissionService extends PolicyService { protected PolicyTypeService doCreatePolicyTypeResource(String type) { return new PolicyTypeService(type, resourceServer, authorization, auth, adminEvent) { @Override - protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { + protected List doSearch(Integer firstResult, Integer maxResult, String fields, Map filters) { filters.put("permission", new String[] {Boolean.TRUE.toString()}); filters.put("type", new String[] {type}); - return super.doSearch(firstResult, maxResult, filters); + return super.doSearch(firstResult, maxResult, fields, filters); } }; } @Override - protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { + protected List doSearch(Integer firstResult, Integer maxResult, String fields, Map filters) { filters.put("permission", new String[] {Boolean.TRUE.toString()}); - return super.doSearch(firstResult, maxResult, filters); + return super.doSearch(firstResult, maxResult, fields, filters); } @Override - protected AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { - return ModelToRepresentation.toRepresentation(policy, authorization, false, false); + protected AbstractPolicyRepresentation toRepresentation(Policy policy, String fields, AuthorizationProvider authorization) { + return ModelToRepresentation.toRepresentation(policy, authorization, false, false, fields != null && fields.equals("*")); } } 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 377436cf90..c17cd14947 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java @@ -26,6 +26,7 @@ import javax.ws.rs.GET; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; @@ -121,7 +122,7 @@ public class PolicyResourceService { @GET @Produces("application/json") @NoCache - public Response findById() { + public Response findById(@QueryParam("fields") String fields) { if (auth != null) { this.auth.realm().requireViewAuthorization(); } @@ -130,11 +131,15 @@ public class PolicyResourceService { return Response.status(Status.NOT_FOUND).build(); } - return Response.ok(toRepresentation(policy, authorization)).build(); + return Response.ok(toRepresentation(policy, fields, authorization)).build(); } - protected AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { - return ModelToRepresentation.toRepresentation(policy, authorization, true, false); + private AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { + return toRepresentation(policy, null, authorization); + } + + protected AbstractPolicyRepresentation toRepresentation(Policy policy, String fields, AuthorizationProvider authorization) { + return ModelToRepresentation.toRepresentation(policy, authorization, true, false, fields != null && fields.equals("*")); } @Path("/dependentPolicies") 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 60a984841a..9e126e540f 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java @@ -146,7 +146,7 @@ public class PolicyService { @GET @Produces(MediaType.APPLICATION_JSON) @NoCache - public Response findByName(@QueryParam("name") String name) { + public Response findByName(@QueryParam("name") String name, @QueryParam("fields") String fields) { if (auth != null) { this.auth.realm().requireViewAuthorization(); } @@ -163,7 +163,7 @@ public class PolicyService { return Response.status(Status.OK).build(); } - return Response.ok(toRepresentation(model, authorization)).build(); + return Response.ok(toRepresentation(model, fields, authorization)).build(); } @GET @@ -176,6 +176,7 @@ public class PolicyService { @QueryParam("scope") String scope, @QueryParam("permission") Boolean permission, @QueryParam("owner") String owner, + @QueryParam("fields") String fields, @QueryParam("first") Integer firstResult, @QueryParam("max") Integer maxResult) { if (auth != null) { @@ -253,18 +254,18 @@ public class PolicyService { } return Response.ok( - doSearch(firstResult, maxResult, search)) + doSearch(firstResult, maxResult, fields, search)) .build(); } - protected AbstractPolicyRepresentation toRepresentation(Policy model, AuthorizationProvider authorization) { - return ModelToRepresentation.toRepresentation(model, authorization, true, false); + protected AbstractPolicyRepresentation toRepresentation(Policy model, String fields, AuthorizationProvider authorization) { + return ModelToRepresentation.toRepresentation(model, authorization, true, false, fields != null && fields.equals("*")); } - protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { + protected List doSearch(Integer firstResult, Integer maxResult, String fields, 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, authorization)) + .map(policy -> toRepresentation(policy, fields, authorization)) .collect(Collectors.toList()); } 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 f6eef35269..d440547d83 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeResourceService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeResourceService.java @@ -58,8 +58,7 @@ public class PolicyTypeResourceService extends PolicyResourceService { return representation; } - @Override - protected AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { - return ModelToRepresentation.toRepresentation(policy, authorization, false, false); + protected AbstractPolicyRepresentation toRepresentation(Policy policy, String fields, AuthorizationProvider authorization) { + return ModelToRepresentation.toRepresentation(policy, authorization, false, false, fields != null && fields.equals("*")); } } 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 6acc57236b..1d28a9bfcf 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java @@ -87,13 +87,13 @@ public class PolicyTypeService extends PolicyService { } @Override - protected AbstractPolicyRepresentation toRepresentation(Policy policy, AuthorizationProvider authorization) { + protected AbstractPolicyRepresentation toRepresentation(Policy policy, String fields, AuthorizationProvider authorization) { return ModelToRepresentation.toRepresentation(policy, authorization, false, false); } @Override - protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { + protected List doSearch(Integer firstResult, Integer maxResult, String fields, Map filters) { filters.put("type", new String[] {type}); - return super.doSearch(firstResult, maxResult, filters); + return super.doSearch(firstResult, maxResult, fields, filters); } } diff --git a/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java b/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java index affd74bb01..aceeb9043d 100644 --- a/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java +++ b/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java @@ -117,7 +117,7 @@ public class UserManagedPermissionService { @Produces("application/json") public Response findById(@PathParam("policyId") String policyId) { checkRequest(getAssociatedResourceId(policyId), null); - return PolicyTypeResourceService.class.cast(delegate.getResource(policyId)).findById(); + return PolicyTypeResourceService.class.cast(delegate.getResource(policyId)).findById(null); } @GET @@ -128,7 +128,7 @@ public class UserManagedPermissionService { @QueryParam("scope") String scope, @QueryParam("first") Integer firstResult, @QueryParam("max") Integer maxResult) { - return delegate.findAll(null, name, "uma", resource, scope, true, identity.getId(), firstResult, maxResult); + return delegate.findAll(null, name, "uma", resource, scope, true, identity.getId(), null, firstResult, maxResult); } private Policy getPolicy(@PathParam("policyId") String policyId) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java index dcb45d4cbe..c1c39bcd89 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java @@ -18,17 +18,18 @@ package org.keycloak.testsuite.admin.client.authorization; import org.junit.Test; +import org.keycloak.admin.client.resource.AuthorizationResource; import org.keycloak.admin.client.resource.PoliciesResource; import org.keycloak.admin.client.resource.PolicyResource; import org.keycloak.admin.client.resource.ResourceResource; import org.keycloak.admin.client.resource.ResourceScopeResource; import org.keycloak.admin.client.resource.ResourceScopesResource; import org.keycloak.admin.client.resource.ResourcesResource; -import org.keycloak.common.Profile; import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.Logic; import org.keycloak.representations.idm.authorization.PolicyProviderRepresentation; import org.keycloak.representations.idm.authorization.PolicyRepresentation; +import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.representations.idm.authorization.ScopeRepresentation; @@ -38,6 +39,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -147,6 +149,41 @@ public class GenericPolicyManagementTest extends AbstractAuthorizationTest { assertTrue(providers.containsAll(expected)); } + @Test + public void testQueryPolicyByIdAllFields() { + PolicyResource policy = createTestingPolicy(); + PolicyRepresentation representation = policy.toRepresentation("*"); + Set resources = representation.getResourcesData(); + + assertEquals(3, resources.size()); + + representation = policy.toRepresentation(); + assertNull(representation.getResourcesData()); + } + + @Test + public void testQueryPolicyAllFields() { + AuthorizationResource authorization = getClientResource().authorization(); + + authorization.resources().create(new ResourceRepresentation("Resource A")); + ResourcePermissionRepresentation permission = new ResourcePermissionRepresentation(); + permission.setName("Permission A"); + permission.addResource("Resource A"); + authorization.permissions().resource().create(permission); + + List policies = authorization.policies() + .policies(null, "Permission A", null, null, null, true, null, "*", -1, -1); + + assertEquals(1, policies.size()); + assertEquals(1, policies.get(0).getResourcesData().size()); + + policies = authorization.policies() + .policies(null, "Permission A", null, null, null, true, null, null, -1, -1); + + assertEquals(1, policies.size()); + assertNull(policies.get(0).getResourcesData()); + } + private PolicyResource createTestingPolicy() { Map config = new HashMap<>();