From d0f505455dce54242bdec3052d837345af8cc561 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 2 Jun 2017 19:06:40 -0300 Subject: [PATCH] [KEYCLOAK-4991] - Allow clients to limit the number of permission in a RPT when using entitlements --- .../AuthorizationRequestMetadata.java | 12 +- .../client/resource/EntitlementResource.java | 30 +--- .../evaluation/DecisionResultCollector.java | 3 +- .../AuthorizationRequestMetadata.java | 20 +-- .../entitlement/EntitlementService.java | 69 +++++---- .../authorization/util/Permissions.java | 3 +- .../testsuite/authz/EntitlementAPITest.java | 143 +++++++++++++++--- 7 files changed, 182 insertions(+), 98 deletions(-) diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/representation/AuthorizationRequestMetadata.java b/authz/client/src/main/java/org/keycloak/authorization/client/representation/AuthorizationRequestMetadata.java index 175d081434..ca45017410 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/representation/AuthorizationRequestMetadata.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/representation/AuthorizationRequestMetadata.java @@ -26,7 +26,9 @@ public class AuthorizationRequestMetadata { public static final String INCLUDE_RESOURCE_NAME = "include_resource_name"; @JsonProperty(INCLUDE_RESOURCE_NAME) - private boolean includeResourceName; + private boolean includeResourceName = true; + + private int limit; public boolean isIncludeResourceName() { return includeResourceName; @@ -35,4 +37,12 @@ public class AuthorizationRequestMetadata { public void setIncludeResourceName(boolean includeResourceName) { this.includeResourceName = includeResourceName; } + + public void setLimit(int limit) { + this.limit = limit; + } + + public int getLimit() { + return limit; + } } diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/resource/EntitlementResource.java b/authz/client/src/main/java/org/keycloak/authorization/client/resource/EntitlementResource.java index d0dc2cff07..8c12abf7e4 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/resource/EntitlementResource.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/resource/EntitlementResource.java @@ -24,31 +24,9 @@ public class EntitlementResource { public EntitlementResponse getAll(String resourceServerId) { try { - return getAll(resourceServerId, null); - } catch (HttpResponseException e) { - if (403 == e.getStatusCode()) { - throw new AuthorizationDeniedException(e); - } - throw new RuntimeException("Failed to obtain entitlements.", e); - } catch (Exception e) { - throw new RuntimeException("Failed to obtain entitlements.", e); - } - } - - public EntitlementResponse getAll(String resourceServerId, AuthorizationRequestMetadata metadata) { - try { - HttpMethod method = this.http.get("/authz/entitlement/" + resourceServerId) - .authorizationBearer(this.eat); - - if (metadata != null) { - StringBuilder params = new StringBuilder(); - - params.append(AuthorizationRequestMetadata.INCLUDE_RESOURCE_NAME).append("=").append(metadata.isIncludeResourceName()); - - method.param("metadata", params.toString()); - } - - return method.response().json(EntitlementResponse.class).execute(); + return this.http.get("/authz/entitlement/" + resourceServerId) + .authorizationBearer(eat) + .response().json(EntitlementResponse.class).execute(); } catch (HttpResponseException e) { if (403 == e.getStatusCode()) { throw new AuthorizationDeniedException(e); @@ -62,7 +40,7 @@ public class EntitlementResource { public EntitlementResponse get(String resourceServerId, EntitlementRequest request) { try { return this.http.post("/authz/entitlement/" + resourceServerId) - .authorizationBearer(this.eat) + .authorizationBearer(eat) .json(JsonSerialization.writeValueAsBytes(request)) .response().json(EntitlementResponse.class).execute(); } catch (HttpResponseException e) { diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionResultCollector.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionResultCollector.java index 02312ba86a..2ffc049ee4 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionResultCollector.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionResultCollector.java @@ -19,6 +19,7 @@ package org.keycloak.authorization.policy.evaluation; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -33,7 +34,7 @@ import org.keycloak.representations.idm.authorization.DecisionStrategy; */ public abstract class DecisionResultCollector implements Decision { - private Map results = new HashMap(); + private Map results = new LinkedHashMap<>(); @Override public void onDecision(DefaultEvaluation evaluation) { diff --git a/services/src/main/java/org/keycloak/authorization/authorization/representation/AuthorizationRequestMetadata.java b/services/src/main/java/org/keycloak/authorization/authorization/representation/AuthorizationRequestMetadata.java index 77aa1d5625..faa90ce2df 100644 --- a/services/src/main/java/org/keycloak/authorization/authorization/representation/AuthorizationRequestMetadata.java +++ b/services/src/main/java/org/keycloak/authorization/authorization/representation/AuthorizationRequestMetadata.java @@ -29,17 +29,9 @@ public class AuthorizationRequestMetadata { public static final String INCLUDE_RESOURCE_NAME = "include_resource_name"; @JsonProperty(INCLUDE_RESOURCE_NAME) - private boolean includeResourceName; + private boolean includeResourceName = true; - public AuthorizationRequestMetadata() { - this(null); - } - - public AuthorizationRequestMetadata(Map claims) { - if (claims != null) { - includeResourceName = Boolean.valueOf(claims.getOrDefault(INCLUDE_RESOURCE_NAME, "true")).booleanValue(); - } - } + private int limit; public boolean isIncludeResourceName() { return includeResourceName; @@ -48,4 +40,12 @@ public class AuthorizationRequestMetadata { public void setIncludeResourceName(boolean includeResourceName) { this.includeResourceName = includeResourceName; } + + public int getLimit() { + return limit; + } + + public void setLimit(int limit) { + this.limit = limit; + } } diff --git a/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java b/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java index a37269d3a1..54097bbe36 100644 --- a/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java +++ b/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -56,6 +57,7 @@ import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.permission.ResourcePermission; import org.keycloak.authorization.policy.evaluation.Result; +import org.keycloak.authorization.protection.permission.representation.PermissionRequest; import org.keycloak.authorization.store.ResourceStore; import org.keycloak.authorization.store.ScopeStore; import org.keycloak.authorization.store.StoreFactory; @@ -102,7 +104,7 @@ public class EntitlementService { @GET() @Produces("application/json") @Consumes("application/json") - public Response getAll(@PathParam("resource_server_id") String resourceServerId, @QueryParam("metadata") String metadataParam) { + public Response getAll(@PathParam("resource_server_id") String resourceServerId) { KeycloakIdentity identity = new KeycloakIdentity(this.authorization.getKeycloakSession()); if (resourceServerId == null) { @@ -123,7 +125,7 @@ public class EntitlementService { throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Client does not support permissions", Status.FORBIDDEN); } - return evaluate(getMetadata(metadataParam), Permissions.all(resourceServer, identity, authorization), identity, resourceServer); + return evaluate(null, Permissions.all(resourceServer, identity, authorization), identity, resourceServer); } @Path("{resource_server_id}") @@ -194,9 +196,15 @@ public class EntitlementService { private List createPermissions(EntitlementRequest entitlementRequest, ResourceServer resourceServer, AuthorizationProvider authorization) { StoreFactory storeFactory = authorization.getStoreFactory(); - Map> permissionsToEvaluate = new HashMap<>(); + Map> permissionsToEvaluate = new LinkedHashMap<>(); + AuthorizationRequestMetadata metadata = entitlementRequest.getMetadata(); + Integer limit = metadata != null && metadata.getLimit() > 0 ? metadata.getLimit() : null; + + for (PermissionRequest requestedResource : entitlementRequest.getPermissions()) { + if (limit != null && limit <= 0) { + break; + } - entitlementRequest.getPermissions().forEach(requestedResource -> { Resource resource; if (requestedResource.getResourceSetId() != null) { @@ -210,14 +218,17 @@ public class EntitlementService { } Set requestedScopes = requestedResource.getScopes().stream().map(ScopeRepresentation::new).collect(Collectors.toSet()); - Set collect = requestedScopes.stream().map(ScopeRepresentation::getName).collect(Collectors.toSet()); + Set scopeNames = requestedScopes.stream().map(ScopeRepresentation::getName).collect(Collectors.toSet()); if (resource != null) { - permissionsToEvaluate.put(resource.getId(), collect); + permissionsToEvaluate.put(resource.getId(), scopeNames); + if (limit != null) { + limit--; + } } else { ResourceStore resourceStore = authorization.getStoreFactory().getResourceStore(); ScopeStore scopeStore = authorization.getStoreFactory().getScopeStore(); - List resources = new ArrayList(); + List resources = new ArrayList<>(); resources.addAll(resourceStore.findByScope(requestedScopes.stream().map(scopeRepresentation -> { Scope scope = scopeStore.findByName(scopeRepresentation.getName(), resourceServer.getId()); @@ -230,17 +241,21 @@ public class EntitlementService { }).filter(s -> s != null).collect(Collectors.toList()), resourceServer.getId())); for (Resource resource1 : resources) { - permissionsToEvaluate.put(resource1.getId(), collect); + permissionsToEvaluate.put(resource1.getId(), scopeNames); + if (limit != null) { + limit--; + } } - permissionsToEvaluate.put("$KC_SCOPE_PERMISSION", collect); + permissionsToEvaluate.put("$KC_SCOPE_PERMISSION", scopeNames); } - }); + } String rpt = entitlementRequest.getRpt(); if (rpt != null && !"".equals(rpt)) { KeycloakContext context = authorization.getKeycloakSession().getContext(); + if (!Tokens.verifySignature(session, context.getRealm(), rpt)) { throw new ErrorResponseException("invalid_rpt", "RPT signature is invalid", Status.FORBIDDEN); } @@ -260,7 +275,11 @@ public class EntitlementService { List permissions = authorizationData.getPermissions(); if (permissions != null) { - permissions.forEach(permission -> { + for (Permission permission : permissions) { + if (limit != null && limit <= 0) { + break; + } + Resource resourcePermission = storeFactory.getResourceStore().findById(permission.getResourceSetId(), resourceServer.getId()); if (resourcePermission != null) { @@ -269,6 +288,9 @@ public class EntitlementService { if (scopes == null) { scopes = new HashSet<>(); permissionsToEvaluate.put(resourcePermission.getId(), scopes); + if (limit != null) { + limit--; + } } Set scopePermission = permission.getScopes(); @@ -277,7 +299,7 @@ public class EntitlementService { scopes.addAll(scopePermission); } } - }); + } } } } @@ -297,27 +319,4 @@ public class EntitlementService { } }).collect(Collectors.toList()); } - - private AuthorizationRequestMetadata getMetadata(@QueryParam("metadata") String metadataParam) { - AuthorizationRequestMetadata metadata; - - if (metadataParam != null) { - Map claims = new HashMap<>(); - - for (String claim : metadataParam.split(",")) { - String[] values = claim.split("="); - - if (values.length < 2) { - throw new ErrorResponseException("invalid_metadata", "Invalid metadata", Status.BAD_REQUEST); - } - - claims.put(values[0], values[1]); - } - - metadata = new AuthorizationRequestMetadata(claims); - } else { - metadata = null; - } - return metadata; - } } diff --git a/services/src/main/java/org/keycloak/authorization/util/Permissions.java b/services/src/main/java/org/keycloak/authorization/util/Permissions.java index 2ee705e149..325c056eb7 100644 --- a/services/src/main/java/org/keycloak/authorization/util/Permissions.java +++ b/services/src/main/java/org/keycloak/authorization/util/Permissions.java @@ -21,6 +21,7 @@ package org.keycloak.authorization.util; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -136,7 +137,7 @@ public final class Permissions { } public static List permits(List evaluation, AuthorizationRequestMetadata metadata, AuthorizationProvider authorizationProvider, ResourceServer resourceServer) { - Map permissions = new HashMap<>(); + Map permissions = new LinkedHashMap<>(); for (Result result : evaluation) { Set deniedScopes = new HashSet<>(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java index b9f422611d..0f2ac61da4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java @@ -16,6 +16,7 @@ */ package org.keycloak.testsuite.authz; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -80,27 +81,27 @@ public class EntitlementAPITest extends AbstractKeycloakTest { public void configureAuthorization() throws Exception { ClientResource client = getClient(getRealm()); AuthorizationResource authorization = client.authorization(); - ResourceRepresentation resource = new ResourceRepresentation("Resource A"); - - Response response = authorization.resources().create(resource); - response.close(); JSPolicyRepresentation policy = new JSPolicyRepresentation(); policy.setName("Default Policy"); policy.setCode("$evaluation.grant();"); - response = authorization.policies().js().create(policy); - response.close(); + authorization.policies().js().create(policy).close(); - ResourcePermissionRepresentation permission = new ResourcePermissionRepresentation(); + for (int i = 1; i <= 20; i++) { + ResourceRepresentation resource = new ResourceRepresentation("Resource " + i); - permission.setName(resource.getName() + " Permission"); - permission.addResource(resource.getName()); - permission.addPolicy(policy.getName()); + authorization.resources().create(resource).close(); - response = authorization.permissions().resource().create(permission); - response.close(); + ResourcePermissionRepresentation permission = new ResourcePermissionRepresentation(); + + permission.setName(resource.getName() + " Permission"); + permission.addResource(resource.getName()); + permission.addPolicy(policy.getName()); + + authorization.permissions().resource().create(permission).close(); + } } @Test @@ -109,12 +110,11 @@ public class EntitlementAPITest extends AbstractKeycloakTest { metadata.setIncludeResourceName(false); - assertResponse(metadata, () -> getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).getAll("resource-server-test", metadata)); assertResponse(metadata, () -> { EntitlementRequest request = new EntitlementRequest(); request.setMetadata(metadata); - request.addPermission(new PermissionRequest("Resource A")); + request.addPermission(new PermissionRequest("Resource 1")); return getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).get("resource-server-test", request); }); @@ -126,13 +126,12 @@ public class EntitlementAPITest extends AbstractKeycloakTest { metadata.setIncludeResourceName(true); - assertResponse(metadata, () -> getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).getAll("resource-server-test", metadata)); assertResponse(metadata, () -> getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).getAll("resource-server-test")); EntitlementRequest request = new EntitlementRequest(); request.setMetadata(metadata); - request.addPermission(new PermissionRequest("Resource A")); + request.addPermission(new PermissionRequest("Resource 13")); assertResponse(metadata, () -> getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).get("resource-server-test", request)); @@ -141,17 +140,102 @@ public class EntitlementAPITest extends AbstractKeycloakTest { assertResponse(metadata, () -> getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).get("resource-server-test", request)); } - private void assertResponse(AuthorizationRequestMetadata metadata, Supplier responseSupplier) { - EntitlementResponse response = responseSupplier.get(); - AccessToken accessToken; + @Test + public void testPermissionLimit() { + EntitlementRequest request = new EntitlementRequest(); - try { - accessToken = new JWSInput(response.getRpt()).readJsonContent(AccessToken.class); - } catch (JWSInputException cause) { - throw new RuntimeException("Failed to deserialize RPT", cause); + for (int i = 1; i <= 10; i++) { + request.addPermission(new PermissionRequest("Resource " + i)); } - AccessToken.Authorization authorization = accessToken.getAuthorization(); + AuthorizationRequestMetadata metadata = new AuthorizationRequestMetadata(); + + metadata.setLimit(10); + + request.setMetadata(metadata); + + EntitlementResponse response = getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).get("resource-server-test", request); + AccessToken rpt = toAccessToken(response); + + List permissions = rpt.getAuthorization().getPermissions(); + + assertEquals(10, permissions.size()); + + for (int i = 0; i < 10; i++) { + assertEquals("Resource " + (i + 1), permissions.get(i).getResourceSetName()); + } + + request = new EntitlementRequest(); + + for (int i = 11; i <= 15; i++) { + request.addPermission(new PermissionRequest("Resource " + i)); + } + + request.setMetadata(metadata); + request.setRpt(response.getRpt()); + + response = getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).get("resource-server-test", request); + rpt = toAccessToken(response); + + permissions = rpt.getAuthorization().getPermissions(); + + assertEquals(10, permissions.size()); + + for (int i = 0; i < 10; i++) { + if (i < 5) { + assertEquals("Resource " + (i + 11), permissions.get(i).getResourceSetName()); + } else { + assertEquals("Resource " + (i - 4), permissions.get(i).getResourceSetName()); + } + } + + request = new EntitlementRequest(); + + for (int i = 16; i <= 18; i++) { + request.addPermission(new PermissionRequest("Resource " + i)); + } + + request.setMetadata(metadata); + request.setRpt(response.getRpt()); + + response = getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).get("resource-server-test", request); + rpt = toAccessToken(response); + + permissions = rpt.getAuthorization().getPermissions(); + + assertEquals(10, permissions.size()); + assertEquals("Resource 16", permissions.get(0).getResourceSetName()); + assertEquals("Resource 17", permissions.get(1).getResourceSetName()); + assertEquals("Resource 18", permissions.get(2).getResourceSetName()); + assertEquals("Resource 11", permissions.get(3).getResourceSetName()); + assertEquals("Resource 12", permissions.get(4).getResourceSetName()); + assertEquals("Resource 13", permissions.get(5).getResourceSetName()); + assertEquals("Resource 14", permissions.get(6).getResourceSetName()); + assertEquals("Resource 15", permissions.get(7).getResourceSetName()); + assertEquals("Resource 1", permissions.get(8).getResourceSetName()); + assertEquals("Resource 2", permissions.get(9).getResourceSetName()); + + request = new EntitlementRequest(); + + metadata.setLimit(5); + request.setMetadata(metadata); + request.setRpt(response.getRpt()); + + response = getAuthzClient().entitlement(authzClient.obtainAccessToken("marta", "password").getToken()).get("resource-server-test", request); + rpt = toAccessToken(response); + + permissions = rpt.getAuthorization().getPermissions(); + + assertEquals(5, permissions.size()); + assertEquals("Resource 16", permissions.get(0).getResourceSetName()); + assertEquals("Resource 17", permissions.get(1).getResourceSetName()); + assertEquals("Resource 18", permissions.get(2).getResourceSetName()); + assertEquals("Resource 11", permissions.get(3).getResourceSetName()); + assertEquals("Resource 12", permissions.get(4).getResourceSetName()); + } + + private void assertResponse(AuthorizationRequestMetadata metadata, Supplier responseSupplier) { + AccessToken.Authorization authorization = toAccessToken(responseSupplier.get()).getAuthorization(); List permissions = authorization.getPermissions(); @@ -167,6 +251,17 @@ public class EntitlementAPITest extends AbstractKeycloakTest { } } + private AccessToken toAccessToken(EntitlementResponse response) { + AccessToken accessToken; + + try { + accessToken = new JWSInput(response.getRpt()).readJsonContent(AccessToken.class); + } catch (JWSInputException cause) { + throw new RuntimeException("Failed to deserialize RPT", cause); + } + return accessToken; + } + private RealmResource getRealm() throws Exception { return adminClient.realm("authz-test"); }