From 625f6131288f513d13619281d7d4decb24c23442 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Wed, 15 Aug 2018 18:36:50 -0300 Subject: [PATCH] [KEYCLOAK-4902] - Using streams to process requested permissions and limit support for scope responses --- .../permission/ScopePolicyProvider.java | 14 +- .../permission/ResourcePermission.java | 24 ++ .../evaluation/AbstractDecisionCollector.java | 33 ++- .../policy/evaluation/Result.java | 6 +- .../admin/PolicyEvaluationService.java | 6 +- .../AuthorizationTokenService.java | 75 +++-- .../authorization/util/Permissions.java | 59 ++-- .../testsuite/authz/EntitlementAPITest.java | 222 ++++++++++++++- .../testsuite/authz/PermissionClaimTest.java | 267 +++++++++++++++++- .../UmaPermissionTicketPushedClaimsTest.java | 21 ++ 10 files changed, 622 insertions(+), 105 deletions(-) diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/ScopePolicyProvider.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/ScopePolicyProvider.java index 7cc043f7a0..c4d4b53ffb 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/ScopePolicyProvider.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/ScopePolicyProvider.java @@ -39,13 +39,11 @@ public class ScopePolicyProvider extends AbstractPermissionProvider { Map decisions = decisionCache.computeIfAbsent(policy, p -> new HashMap<>()); ResourcePermission permission = evaluation.getPermission(); - for (Scope scope : permission.getScopes()) { - Decision.Effect effect = decisions.get(scope); + Decision.Effect effect = decisions.get(permission); - if (effect != null) { - defaultEvaluation.setEffect(effect); - return; - } + if (effect != null) { + defaultEvaluation.setEffect(effect); + return; } Decision.Effect decision = defaultEvaluation.getEffect(); @@ -53,9 +51,7 @@ public class ScopePolicyProvider extends AbstractPermissionProvider { if (decision == null) { super.evaluate(evaluation); - for (Scope scope : policy.getScopes()) { - decisions.put(scope, defaultEvaluation.getEffect()); - } + decisions.put(permission, defaultEvaluation.getEffect()); } } } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/permission/ResourcePermission.java b/server-spi-private/src/main/java/org/keycloak/authorization/permission/ResourcePermission.java index 3789281567..3b8dedb8ac 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/permission/ResourcePermission.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/permission/ResourcePermission.java @@ -22,6 +22,7 @@ import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -48,6 +49,10 @@ public class ResourcePermission { this(resource, scopes, resourceServer, null); } + public ResourcePermission(Resource resource, ResourceServer resourceServer, Map> claims) { + this(resource, new ArrayList<>(resource.getScopes()), resourceServer, claims); + } + public ResourcePermission(Resource resource, List scopes, ResourceServer resourceServer, Map> claims) { this.resource = resource; this.scopes = scopes; @@ -125,4 +130,23 @@ public class ResourcePermission { claims.remove(name); } } + + public void addScope(Scope scope) { + if (resource != null) { + if (!resource.getScopes().contains(scope)) { + return; + } + } + + if (!scopes.contains(scope)) { + scopes.add(scope); + } + } + + public void addClaims(Map> claims) { + if (this.claims == null) { + this.claims = new HashMap<>(); + } + this.claims.putAll(claims); + } } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/AbstractDecisionCollector.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/AbstractDecisionCollector.java index 6983ae1fca..9cffa0ba67 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/AbstractDecisionCollector.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/AbstractDecisionCollector.java @@ -25,6 +25,7 @@ import org.keycloak.representations.idm.authorization.DecisionStrategy; import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Set; /** * @author Pedro Igor @@ -36,25 +37,39 @@ public abstract class AbstractDecisionCollector implements Decision new Result(permission, evaluation)).policy(parentPolicy); + results.computeIfAbsent(permission, permission1 -> { + for (Result result : results.values()) { + Result.PolicyResult policyResult = result.getPolicy(parentPolicy); - for (Result result : results.values()) { - Result.PolicyResult policyResult = result.getPolicy(parentPolicy); + if (policyResult != null) { + Result newResult = new Result(permission1, evaluation); + Result.PolicyResult newPolicyResult = newResult.policy(parentPolicy); - if (policyResult != null) { - for (Result.PolicyResult associatePolicy : policyResult.getAssociatedPolicies()) { - cached.policy(associatePolicy.getPolicy(), associatePolicy.getEffect()); + for (Result.PolicyResult associatePolicy : policyResult.getAssociatedPolicies()) { + newPolicyResult.policy(associatePolicy.getPolicy(), associatePolicy.getEffect()); + } + + Map> claims = result.getPermission().getClaims(); + + if (!claims.isEmpty()) { + permission1.addClaims(claims); + } + + return newResult; } } - } + + return null; + }).policy(parentPolicy); } else { - results.computeIfAbsent(evaluation.getPermission(), permission -> new Result(permission, evaluation)).policy(parentPolicy).policy(evaluation.getPolicy(), evaluation.getEffect()); + results.computeIfAbsent(permission, p -> new Result(p, evaluation)).policy(parentPolicy).policy(evaluation.getPolicy(), evaluation.getEffect()); } } else { - results.computeIfAbsent(evaluation.getPermission(), permission -> new Result(permission, evaluation)).setStatus(evaluation.getEffect()); + results.computeIfAbsent(permission, p -> new Result(p, evaluation)).setStatus(evaluation.getEffect()); } } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/Result.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/Result.java index 5ce2446b04..3c886b6c74 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/Result.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/Result.java @@ -86,11 +86,7 @@ public class Result { } public PolicyResult policy(Policy policy, Effect effect) { - PolicyResult result = associatedPolicies.computeIfAbsent(policy.getId(), id -> new PolicyResult(policy, effect)); - - result.setEffect(effect); - - return result; + return associatedPolicies.computeIfAbsent(policy.getId(), id -> new PolicyResult(policy, effect)); } public Policy getPolicy() { diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java index 71f4b3e128..1ee4d9136a 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java @@ -176,9 +176,9 @@ public class PolicyEvaluationService { if (resource.getId() != null) { Resource resourceModel = storeFactory.getResourceStore().findById(resource.getId(), resourceServer.getId()); - return new ArrayList<>(Arrays.asList(Permissions.createResourcePermissions(resourceModel, scopes.stream().map(Scope::getName).collect(Collectors.toSet()), authorization, request))).stream(); + return new ArrayList<>(Arrays.asList(Permissions.createResourcePermissions(resourceModel, scopes, authorization, request))).stream(); } else if (resource.getType() != null) { - return storeFactory.getResourceStore().findByType(resource.getType(), resourceServer.getId()).stream().map(resource1 -> Permissions.createResourcePermissions(resource1, scopes.stream().map(Scope::getName).collect(Collectors.toSet()), authorization, request)); + return storeFactory.getResourceStore().findByType(resource.getType(), resourceServer.getId()).stream().map(resource1 -> Permissions.createResourcePermissions(resource1, scopes, authorization, request)); } else { if (scopes.isEmpty()) { return Permissions.all(resourceServer, evaluationContext.getIdentity(), authorization, request).stream(); @@ -191,7 +191,7 @@ public class PolicyEvaluationService { } - return resources.stream().map(resource12 -> Permissions.createResourcePermissions(resource12, scopes.stream().map(Scope::getName).collect(Collectors.toSet()), authorization, request)); + return resources.stream().map(resource12 -> Permissions.createResourcePermissions(resource12, scopes, authorization, request)); } }).collect(Collectors.toList()); } diff --git a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java index e9ebee39c2..afc29dc2ce 100644 --- a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java +++ b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java @@ -28,6 +28,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; import java.util.stream.Collectors; @@ -355,10 +357,10 @@ public class AuthorizationTokenService { ResourceStore resourceStore = storeFactory.getResourceStore(); ScopeStore scopeStore = storeFactory.getScopeStore(); Metadata metadata = request.getMetadata(); - Integer limit = metadata != null ? metadata.getLimit() : null; + final AtomicInteger limit = metadata != null && metadata.getLimit() != null ? new AtomicInteger(metadata.getLimit()) : null; for (Permission permission : ticket.getPermissions()) { - if (limit != null && limit <= 0) { + if (limit != null && limit.get() <= 0) { break; } @@ -368,7 +370,7 @@ public class AuthorizationTokenService { requestedScopes = new HashSet<>(); } - List existingResources = new ArrayList<>(); + List requestedResources = new ArrayList<>(); String resourceId = permission.getResourceId(); if (resourceId != null) { @@ -379,14 +381,14 @@ public class AuthorizationTokenService { } if (resource != null) { - existingResources.add(resource); + requestedResources.add(resource); } else { String resourceName = resourceId; Resource ownerResource = resourceStore.findByName(resourceName, identity.getId(), resourceServer.getId()); if (ownerResource != null) { permission.setResourceId(ownerResource.getId()); - existingResources.add(ownerResource); + requestedResources.add(ownerResource); } if (!identity.isResourceServer()) { @@ -394,7 +396,7 @@ public class AuthorizationTokenService { if (serverResource != null) { permission.setResourceId(serverResource.getId()); - existingResources.add(serverResource); + requestedResources.add(serverResource); } } } @@ -406,45 +408,66 @@ public class AuthorizationTokenService { requestedScopes.addAll(Arrays.asList(clientAdditionalScopes.split(" "))); } - List requestedScopesModel = requestedScopes.stream().map(s -> scopeStore.findByName(s, resourceServer.getId())).filter(Objects::nonNull).collect(Collectors.toList()); + Set requestedScopesModel = requestedScopes.stream().map(s -> scopeStore.findByName(s, resourceServer.getId())).filter(Objects::nonNull).collect(Collectors.toSet()); - if (resourceId != null && existingResources.isEmpty()) { + if (resourceId != null && requestedResources.isEmpty()) { throw new CorsErrorResponseException(request.getCors(), "invalid_resource", "Resource with id [" + resourceId + "] does not exist.", Status.BAD_REQUEST); } - if ((permission.getScopes() != null && !permission.getScopes().isEmpty()) && requestedScopesModel.isEmpty()) { - throw new CorsErrorResponseException(request.getCors(), "invalid_scope", "One of the given scopes " + permission.getScopes() + " are invalid", Status.BAD_REQUEST); + if (!requestedScopes.isEmpty() && requestedScopesModel.isEmpty()) { + throw new CorsErrorResponseException(request.getCors(), "invalid_scope", "One of the given scopes " + permission.getScopes() + " is invalid", Status.BAD_REQUEST); } - if (!existingResources.isEmpty()) { - for (Resource resource : existingResources) { + if (!requestedResources.isEmpty()) { + for (Resource resource : requestedResources) { + if (limit != null && limit.get() <= 0) { + break; + } ResourcePermission perm = permissionsToEvaluate.get(resource.getId()); if (perm == null) { - perm = Permissions.createResourcePermissions(resource, requestedScopes, authorization, request); + perm = Permissions.createResourcePermissions(resource, requestedScopesModel, authorization, request); permissionsToEvaluate.put(resource.getId(), perm); if (limit != null) { - limit--; + limit.decrementAndGet(); } } else { for (Scope scope : requestedScopesModel) { - if (!perm.getScopes().contains(scope)) { - perm.getScopes().add(scope); - } + perm.addScope(scope); } } } } else { - List resources = resourceStore.findByScope(requestedScopesModel.stream().map(Scope::getId).collect(Collectors.toList()), resourceServer.getId()); + AtomicBoolean processed = new AtomicBoolean(); - if (resources.isEmpty()) { - permissionsToEvaluate.put("$KC_SCOPE_PERMISSION", new ResourcePermission(null, requestedScopesModel, resourceServer, request.getClaims())); - } else { - for (Resource resource : resources) { - permissionsToEvaluate.put(resource.getId(), Permissions.createResourcePermissions(resource, requestedScopes, authorization, request)); + resourceStore.findByScope(requestedScopesModel.stream().map(Scope::getId).collect(Collectors.toList()), resourceServer.getId(), resource -> { + if (limit != null && limit.get() <= 0) { + return; + } + + ResourcePermission perm = permissionsToEvaluate.get(resource.getId()); + + if (perm == null) { + perm = Permissions.createResourcePermissions(resource, requestedScopesModel, authorization, request); + permissionsToEvaluate.put(resource.getId(), perm); if (limit != null) { - limit--; + limit.decrementAndGet(); } + } else { + for (Scope scope : requestedScopesModel) { + perm.addScope(scope); + } + } + + processed.compareAndSet(false, true); + }); + + if (!processed.get()) { + for (Scope scope : requestedScopesModel) { + if (limit != null && limit.getAndDecrement() <= 0) { + break; + } + permissionsToEvaluate.computeIfAbsent(scope.getId(), s -> new ResourcePermission(null, new ArrayList<>(Arrays.asList(scope)), resourceServer, request.getClaims())); } } } @@ -460,7 +483,7 @@ public class AuthorizationTokenService { if (permissions != null) { for (Permission grantedPermission : permissions) { - if (limit != null && limit <= 0) { + if (limit != null && limit.get() <= 0) { break; } @@ -473,7 +496,7 @@ public class AuthorizationTokenService { permission = new ResourcePermission(resource, new ArrayList<>(), resourceServer, grantedPermission.getClaims()); permissionsToEvaluate.put(resource.getId(), permission); if (limit != null) { - limit--; + limit.decrementAndGet(); } } else { if (grantedPermission.getClaims() != null) { 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 7bb38ab497..ecb9defef5 100644 --- a/services/src/main/java/org/keycloak/authorization/util/Permissions.java +++ b/services/src/main/java/org/keycloak/authorization/util/Permissions.java @@ -20,16 +20,14 @@ package org.keycloak.authorization.util; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; -import javax.ws.rs.core.Response.Status; - import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.identity.Identity; import org.keycloak.authorization.model.PermissionTicket; @@ -38,11 +36,9 @@ import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.permission.ResourcePermission; import org.keycloak.authorization.store.ResourceStore; -import org.keycloak.authorization.store.ScopeStore; import org.keycloak.authorization.store.StoreFactory; import org.keycloak.representations.idm.authorization.AuthorizationRequest; import org.keycloak.representations.idm.authorization.AuthorizationRequest.Metadata; -import org.keycloak.services.ErrorResponseException; /** * @author Pedro Igor @@ -80,14 +76,14 @@ public final class Permissions { // obtain all resources where owner is the resource server resourceStore.findByOwner(resourceServer.getId(), resourceServer.getId(), resource -> { if (limit.decrementAndGet() >= 0) { - permissions.add(createResourcePermissionsWithScopes(resource, new LinkedList(resource.getScopes()), authorization, request)); + permissions.add(createResourcePermissions(resource, authorization, request)); } }); // obtain all resources where owner is the current user resourceStore.findByOwner(identity.getId(), resourceServer.getId(), resource -> { if (limit.decrementAndGet() >= 0) { - permissions.add(createResourcePermissionsWithScopes(resource, new LinkedList(resource.getScopes()), authorization, request)); + permissions.add(createResourcePermissions(resource, authorization, request)); } }); @@ -116,45 +112,30 @@ public final class Permissions { return permissions; } - public static ResourcePermission createResourcePermissions(Resource resource, Set requestedScopes, AuthorizationProvider authorization, AuthorizationRequest request) { - String type = resource.getType(); - ResourceServer resourceServer = resource.getResourceServer(); + public static ResourcePermission createResourcePermissions(Resource resource, Collection requestedScopes, AuthorizationProvider authorization, AuthorizationRequest request) { List scopes; if (requestedScopes.isEmpty()) { - scopes = new LinkedList<>(resource.getScopes()); - // check if there is a typed resource whose scopes are inherited by the resource being requested. In this case, we assume that parent resource - // is owned by the resource server itself - if (type != null && !resource.getOwner().equals(resourceServer.getId())) { - StoreFactory storeFactory = authorization.getStoreFactory(); - ResourceStore resourceStore = storeFactory.getResourceStore(); - resourceStore.findByType(type, resourceServer.getId(), resource1 -> { - if (resource1.getOwner().equals(resourceServer.getId())) { - for (Scope typeScope : resource1.getScopes()) { - if (!scopes.contains(typeScope)) { - scopes.add(typeScope); - } - } - } - }); - } + scopes = populateTypedScopes(resource, authorization); } else { - ScopeStore scopeStore = authorization.getStoreFactory().getScopeStore(); - scopes = requestedScopes.stream().map(scopeName -> { - Scope byName = scopeStore.findByName(scopeName, resource.getResourceServer().getId()); - - if (byName == null) { - throw new ErrorResponseException("invalid_scope", "Invalid scope [" + scopeName + "].", Status.BAD_REQUEST); - } - - return byName; - }).filter(resource.getScopes()::contains).collect(Collectors.toList()); + scopes = requestedScopes.stream().filter(scope -> resource.getScopes().contains(scope)).collect(Collectors.toList()); } return new ResourcePermission(resource, scopes, resource.getResourceServer(), request.getClaims()); } - public static ResourcePermission createResourcePermissionsWithScopes(Resource resource, List scopes, AuthorizationProvider authorization, AuthorizationRequest request) { + public static ResourcePermission createResourcePermissions(Resource resource, AuthorizationProvider authorization, AuthorizationRequest request) { + List requestedScopes = resource.getScopes(); + + if (requestedScopes.isEmpty()) { + return new ResourcePermission(resource, populateTypedScopes(resource, authorization), resource.getResourceServer(), request.getClaims()); + } + + return new ResourcePermission(resource, resource.getResourceServer(), request.getClaims()); + } + + private static List populateTypedScopes(Resource resource, AuthorizationProvider authorization) { + List scopes = new LinkedList<>(resource.getScopes()); String type = resource.getType(); ResourceServer resourceServer = resource.getResourceServer(); @@ -163,7 +144,7 @@ public final class Permissions { if (type != null && !resource.getOwner().equals(resourceServer.getId())) { StoreFactory storeFactory = authorization.getStoreFactory(); ResourceStore resourceStore = storeFactory.getResourceStore(); - resourceStore.findByType(type, resourceServer.getId()).forEach(resource1 -> { + resourceStore.findByType(type, resourceServer.getId(), resource1 -> { if (resource1.getOwner().equals(resourceServer.getId())) { for (Scope typeScope : resource1.getScopes()) { if (!scopes.contains(typeScope)) { @@ -174,6 +155,6 @@ public final class Permissions { }); } - return new ResourcePermission(resource, scopes, resource.getResourceServer(), request.getClaims()); + return scopes; } } 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 736c0a512d..da2e3a84c4 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 @@ -31,10 +31,12 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Supplier; import org.hamcrest.Matchers; +import org.jetbrains.annotations.NotNull; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -679,6 +681,22 @@ public class EntitlementAPITest extends AbstractAuthzTest { assertEquals(2, grantedPermission.getScopes().size()); assertTrue(grantedPermission.getScopes().containsAll(Arrays.asList("sensors:view", "sensors:update"))); } + + request = new AuthorizationRequest(); + + request.addPermission(null, "sensors:view"); + request.addPermission(null, "sensors:update"); + + response = authzClient.authorization(accessToken).authorize(request); + assertNotNull(response.getToken()); + permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + assertEquals(2, permissions.size()); + + for (Permission grantedPermission : permissions) { + assertTrue(resourceIds.containsAll(Arrays.asList(grantedPermission.getResourceId()))); + assertEquals(2, grantedPermission.getScopes().size()); + assertTrue(grantedPermission.getScopes().containsAll(Arrays.asList("sensors:view", "sensors:update"))); + } } @Test @@ -760,19 +778,7 @@ public class EntitlementAPITest extends AbstractAuthzTest { public void testOverridePermission() throws Exception { ClientResource client = getClient(getRealm(), RESOURCE_SERVER_TEST); AuthorizationResource authorization = client.authorization(); - JSPolicyRepresentation onlyOwnerPolicy = new JSPolicyRepresentation(); - - onlyOwnerPolicy.setName(KeycloakModelUtils.generateId()); - onlyOwnerPolicy.setCode("var context = $evaluation.getContext();\n" + - "var identity = context.getIdentity();\n" + - "var permission = $evaluation.getPermission();\n" + - "var resource = permission.getResource();\n" + - "\n" + - "if (resource) {\n" + - " if (resource.owner == identity.id) {\n" + - " $evaluation.grant();\n" + - " }\n" + - "}"); + JSPolicyRepresentation onlyOwnerPolicy = createOnlyOwnerPolicy(); authorization.policies().js().create(onlyOwnerPolicy).close(); @@ -959,6 +965,196 @@ public class EntitlementAPITest extends AbstractAuthzTest { } } + @NotNull + private JSPolicyRepresentation createOnlyOwnerPolicy() { + JSPolicyRepresentation onlyOwnerPolicy = new JSPolicyRepresentation(); + + onlyOwnerPolicy.setName(KeycloakModelUtils.generateId()); + onlyOwnerPolicy.setCode("var context = $evaluation.getContext();\n" + + "var identity = context.getIdentity();\n" + + "var permission = $evaluation.getPermission();\n" + + "var resource = permission.getResource();\n" + + "\n" + + "if (resource) {\n" + + " if (resource.owner == identity.id) {\n" + + " $evaluation.grant();\n" + + " }\n" + + "}"); + + return onlyOwnerPolicy; + } + + @Test + public void testPermissionsWithResourceAttributes() throws Exception { + ClientResource client = getClient(getRealm(), RESOURCE_SERVER_TEST); + AuthorizationResource authorization = client.authorization(); + JSPolicyRepresentation onlyPublicResourcesPolicy = new JSPolicyRepresentation(); + + onlyPublicResourcesPolicy.setName(KeycloakModelUtils.generateId()); + onlyPublicResourcesPolicy.setCode("var createPermission = $evaluation.getPermission();\n" + + "var resource = createPermission.getResource();\n" + + "\n" + + "if (resource) {\n" + + " var attributes = resource.getAttributes();\n" + + " var visibility = attributes.get('visibility');\n" + + " \n" + + " if (visibility && \"private\".equals(visibility.get(0))) {\n" + + " $evaluation.deny();\n" + + " } else {\n" + + " $evaluation.grant();\n" + + " }\n" + + "}"); + + authorization.policies().js().create(onlyPublicResourcesPolicy).close(); + + JSPolicyRepresentation onlyOwnerPolicy = createOnlyOwnerPolicy(); + + authorization.policies().js().create(onlyOwnerPolicy).close(); + + ResourceRepresentation typedResource = new ResourceRepresentation(); + + typedResource.setType("resource"); + typedResource.setName(KeycloakModelUtils.generateId()); + + typedResource = authorization.resources().create(typedResource).readEntity(ResourceRepresentation.class); + + ResourceRepresentation userResource = new ResourceRepresentation(); + + userResource.setName(KeycloakModelUtils.generateId()); + userResource.setType("resource"); + userResource.setOwner("marta"); + Map> attributes = new HashMap<>(); + attributes.put("visibility", Arrays.asList("private")); + userResource.setAttributes(attributes); + + userResource = authorization.resources().create(userResource).readEntity(ResourceRepresentation.class); + + ResourcePermissionRepresentation typedResourcePermission = new ResourcePermissionRepresentation(); + + typedResourcePermission.setName(KeycloakModelUtils.generateId()); + typedResourcePermission.setResourceType("resource"); + typedResourcePermission.addPolicy(onlyPublicResourcesPolicy.getName()); + + typedResourcePermission = authorization.permissions().resource().create(typedResourcePermission).readEntity(ResourcePermissionRepresentation.class); + + // marta can access any public resource + AuthzClient authzClient = getAuthzClient(AUTHZ_CLIENT_CONFIG); + AuthorizationRequest request = new AuthorizationRequest(); + + request.addPermission(typedResource.getId()); + request.addPermission(userResource.getId()); + + AuthorizationResponse response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + Collection permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + assertEquals(1, permissions.size()); + + for (Permission grantedPermission : permissions) { + assertEquals(typedResource.getName(), grantedPermission.getResourceName()); + } + + typedResourcePermission.addPolicy(onlyOwnerPolicy.getName()); + typedResourcePermission.setDecisionStrategy(DecisionStrategy.AFFIRMATIVE); + + authorization.permissions().resource().findById(typedResourcePermission.getId()).update(typedResourcePermission); + + response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + assertEquals(2, permissions.size()); + + for (Permission grantedPermission : permissions) { + assertThat(Arrays.asList(typedResource.getName(), userResource.getName()), Matchers.hasItem(grantedPermission.getResourceName())); + } + + typedResource.setAttributes(attributes); + + authorization.resources().resource(typedResource.getId()).update(typedResource); + + response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + assertEquals(1, permissions.size()); + + for (Permission grantedPermission : permissions) { + assertThat(userResource.getName(), Matchers.equalTo(grantedPermission.getResourceName())); + } + + userResource.addScope("create", "read"); + authorization.resources().resource(userResource.getId()).update(userResource); + + typedResource.addScope("create", "read"); + authorization.resources().resource(typedResource.getId()).update(typedResource); + + ScopePermissionRepresentation createPermission = new ScopePermissionRepresentation(); + + createPermission.setName(KeycloakModelUtils.generateId()); + createPermission.addScope("create"); + createPermission.addPolicy(onlyPublicResourcesPolicy.getName()); + + authorization.permissions().scope().create(createPermission); + + response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + assertEquals(1, permissions.size()); + + for (Permission grantedPermission : permissions) { + assertThat(userResource.getName(), Matchers.equalTo(grantedPermission.getResourceName())); + assertThat(grantedPermission.getScopes(), Matchers.not(Matchers.hasItem("create"))); + } + + typedResource.setAttributes(new HashMap<>()); + + authorization.resources().resource(typedResource.getId()).update(typedResource); + + response = authzClient.authorization("marta", "password").authorize(); + assertNotNull(response.getToken()); + permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + + for (Permission grantedPermission : permissions) { + if (grantedPermission.getResourceName().equals(userResource.getName())) { + assertThat(grantedPermission.getScopes(), Matchers.not(Matchers.hasItem("create"))); + } else if (grantedPermission.getResourceName().equals(typedResource.getName())) { + assertThat(grantedPermission.getScopes(), Matchers.containsInAnyOrder("create", "read")); + } + } + + request = new AuthorizationRequest(); + + request.addPermission(typedResource.getId()); + request.addPermission(userResource.getId()); + + response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + + for (Permission grantedPermission : permissions) { + if (grantedPermission.getResourceName().equals(userResource.getName())) { + assertThat(grantedPermission.getScopes(), Matchers.not(Matchers.hasItem("create"))); + } else if (grantedPermission.getResourceName().equals(typedResource.getName())) { + assertThat(grantedPermission.getScopes(), Matchers.containsInAnyOrder("create", "read")); + } + } + + request = new AuthorizationRequest(); + + request.addPermission(userResource.getId()); + request.addPermission(typedResource.getId()); + + response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + + for (Permission grantedPermission : permissions) { + if (grantedPermission.getResourceName().equals(userResource.getName())) { + assertThat(grantedPermission.getScopes(), Matchers.not(Matchers.hasItem("create"))); + } else if (grantedPermission.getResourceName().equals(typedResource.getName())) { + assertThat(grantedPermission.getScopes(), Matchers.containsInAnyOrder("create", "read")); + } + } + } + private void testRptRequestWithResourceName(String configFile) { Metadata metadata = new Metadata(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionClaimTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionClaimTest.java index 8e3e50b09b..66dc1331d5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionClaimTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionClaimTest.java @@ -18,26 +18,34 @@ package org.keycloak.testsuite.authz; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import org.hamcrest.Matchers; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.keycloak.admin.client.resource.AuthorizationResource; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.admin.client.resource.ResourcesResource; import org.keycloak.authorization.client.AuthzClient; import org.keycloak.authorization.client.Configuration; +import org.keycloak.authorization.client.util.HttpResponseException; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.AccessToken; import org.keycloak.representations.AccessToken.Authorization; +import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.authorization.AuthorizationRequest; import org.keycloak.representations.idm.authorization.AuthorizationResponse; @@ -46,6 +54,7 @@ import org.keycloak.representations.idm.authorization.Permission; import org.keycloak.representations.idm.authorization.PermissionRequest; import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; +import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RealmBuilder; @@ -61,6 +70,8 @@ public class PermissionClaimTest extends AbstractAuthzTest { private JSPolicyRepresentation claimAPolicy; private JSPolicyRepresentation claimBPolicy; + private JSPolicyRepresentation claimCPolicy; + private JSPolicyRepresentation denyPolicy; @Override public void addTestRealms(List testRealms) { @@ -100,6 +111,39 @@ public class PermissionClaimTest extends AbstractAuthzTest { claimBPolicy.setCode("$evaluation.getPermission().addClaim('claim-b', 'claim-b');$evaluation.grant();"); authorization.policies().js().create(claimBPolicy).close(); + + claimCPolicy = new JSPolicyRepresentation(); + + claimCPolicy.setName("Policy Claim C"); + claimCPolicy.setCode("$evaluation.getPermission().addClaim('claim-c', 'claim-c');$evaluation.grant();"); + + authorization.policies().js().create(claimCPolicy).close(); + + denyPolicy = new JSPolicyRepresentation(); + + denyPolicy.setName("Deny Policy"); + denyPolicy.setCode("$evaluation.getPermission().addClaim('deny-policy', 'deny-policy');$evaluation.deny();"); + + authorization.policies().js().create(denyPolicy).close(); + } + + @After + public void removeAuthorization() throws Exception { + ClientResource client = getClient(getRealm()); + ClientRepresentation representation = client.toRepresentation(); + + representation.setAuthorizationServicesEnabled(false); + + client.update(representation); + + representation.setAuthorizationServicesEnabled(true); + + client.update(representation); + + ResourcesResource resources = client.authorization().resources(); + List defaultResource = resources.findByName("Default Resource"); + + resources.resource(defaultResource.get(0).getId()).remove(); } @Test @@ -176,6 +220,227 @@ public class PermissionClaimTest extends AbstractAuthzTest { assertTrue(claims.containsKey("claim-b")); } + @Test + public void testClaimsFromDifferentScopePermissions() throws Exception { + ClientResource client = getClient(getRealm()); + AuthorizationResource authorization = client.authorization(); + + ResourceRepresentation resourceA = new ResourceRepresentation(KeycloakModelUtils.generateId(), "create", "update"); + + authorization.resources().create(resourceA).close(); + + ResourceRepresentation resourceB = new ResourceRepresentation(KeycloakModelUtils.generateId(), "create", "update"); + + authorization.resources().create(resourceB).close(); + + ScopePermissionRepresentation allScopesPermission = new ScopePermissionRepresentation(); + + allScopesPermission.setName(KeycloakModelUtils.generateId()); + allScopesPermission.addScope("create", "update"); + allScopesPermission.addPolicy(claimAPolicy.getName(), claimBPolicy.getName()); + + authorization.permissions().scope().create(allScopesPermission).close(); + + ScopePermissionRepresentation updatePermission = new ScopePermissionRepresentation(); + + updatePermission.setName(KeycloakModelUtils.generateId()); + updatePermission.addScope("update"); + updatePermission.addPolicy(claimCPolicy.getName()); + + updatePermission = authorization.permissions().scope().create(updatePermission).readEntity(ScopePermissionRepresentation.class); + + AuthzClient authzClient = getAuthzClient(); + AuthorizationRequest request = new AuthorizationRequest(); + + request.addPermission(null, "create", "update"); + + AuthorizationResponse response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + AccessToken rpt = toAccessToken(response.getToken()); + Authorization authorizationClaim = rpt.getAuthorization(); + List permissions = new ArrayList<>(authorizationClaim.getPermissions()); + + assertEquals(2, permissions.size()); + + for (Permission permission : permissions) { + Map> claims = permission.getClaims(); + + assertNotNull(claims); + + assertThat(claims.get("claim-a"), Matchers.containsInAnyOrder("claim-a", "claim-a1")); + assertThat(claims.get("claim-b"), Matchers.containsInAnyOrder("claim-b")); + assertThat(claims.get("claim-c"), Matchers.containsInAnyOrder("claim-c")); + } + + updatePermission.addPolicy(denyPolicy.getName()); + authorization.permissions().scope().findById(updatePermission.getId()).update(updatePermission); + + response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + rpt = toAccessToken(response.getToken()); + authorizationClaim = rpt.getAuthorization(); + permissions = new ArrayList<>(authorizationClaim.getPermissions()); + + assertEquals(2, permissions.size()); + + for (Permission permission : permissions) { + Map> claims = permission.getClaims(); + + assertNotNull(claims); + + assertThat(claims.get("claim-a"), Matchers.containsInAnyOrder("claim-a", "claim-a1")); + assertThat(claims.get("claim-b"), Matchers.containsInAnyOrder("claim-b")); + assertThat(claims.get("claim-c"), Matchers.containsInAnyOrder("claim-c")); + assertThat(claims.get("deny-policy"), Matchers.containsInAnyOrder("deny-policy")); + } + } + + @Test + public void testClaimsFromDifferentResourcePermissions() throws Exception { + ClientResource client = getClient(getRealm()); + AuthorizationResource authorization = client.authorization(); + + ResourceRepresentation resourceA = new ResourceRepresentation(KeycloakModelUtils.generateId()); + + resourceA.setType("typed-resource"); + + authorization.resources().create(resourceA).close(); + + ResourcePermissionRepresentation allScopesPermission = new ResourcePermissionRepresentation(); + + allScopesPermission.setName(KeycloakModelUtils.generateId()); + allScopesPermission.addResource(resourceA.getName()); + allScopesPermission.addPolicy(claimAPolicy.getName(), claimBPolicy.getName()); + + authorization.permissions().resource().create(allScopesPermission).close(); + + ResourcePermissionRepresentation updatePermission = new ResourcePermissionRepresentation(); + + updatePermission.setName(KeycloakModelUtils.generateId()); + updatePermission.addResource(resourceA.getName()); + updatePermission.addPolicy(claimCPolicy.getName()); + + updatePermission = authorization.permissions().resource().create(updatePermission).readEntity(ResourcePermissionRepresentation.class); + + AuthzClient authzClient = getAuthzClient(); + AuthorizationResponse response = authzClient.authorization("marta", "password").authorize(); + assertNotNull(response.getToken()); + AccessToken rpt = toAccessToken(response.getToken()); + Authorization authorizationClaim = rpt.getAuthorization(); + List permissions = new ArrayList<>(authorizationClaim.getPermissions()); + + assertEquals(1, permissions.size()); + + for (Permission permission : permissions) { + Map> claims = permission.getClaims(); + + assertNotNull(claims); + + assertThat(claims.get("claim-a"), Matchers.containsInAnyOrder("claim-a", "claim-a1")); + assertThat(claims.get("claim-b"), Matchers.containsInAnyOrder("claim-b")); + assertThat(claims.get("claim-c"), Matchers.containsInAnyOrder("claim-c")); + } + + updatePermission.addPolicy(denyPolicy.getName()); + authorization.permissions().resource().findById(updatePermission.getId()).update(updatePermission); + + try { + authzClient.authorization("marta", "password").authorize(); + fail("can not access resource"); + } catch (RuntimeException expected) { + assertEquals(403, HttpResponseException.class.cast(expected.getCause()).getStatusCode()); + assertTrue(HttpResponseException.class.cast(expected.getCause()).toString().contains("access_denied")); + } + + ResourceRepresentation resourceInstance = new ResourceRepresentation(KeycloakModelUtils.generateId(), "create", "update"); + + resourceInstance.setType(resourceA.getType()); + resourceInstance.setOwner("marta"); + + resourceInstance = authorization.resources().create(resourceInstance).readEntity(ResourceRepresentation.class); + + AuthorizationRequest request = new AuthorizationRequest(); + + request.addPermission(null, "create", "update"); + + try { + authzClient.authorization("marta", "password").authorize(request); + fail("can not access resource"); + } catch (RuntimeException expected) { + assertEquals(403, HttpResponseException.class.cast(expected.getCause()).getStatusCode()); + assertTrue(HttpResponseException.class.cast(expected.getCause()).toString().contains("access_denied")); + } + + ResourcePermissionRepresentation resourceInstancePermission = new ResourcePermissionRepresentation(); + + resourceInstancePermission.setName(KeycloakModelUtils.generateId()); + resourceInstancePermission.addResource(resourceInstance.getId()); + resourceInstancePermission.addPolicy(claimCPolicy.getName()); + + resourceInstancePermission = authorization.permissions().resource().create(resourceInstancePermission).readEntity(ResourcePermissionRepresentation.class); + + response = authzClient.authorization("marta", "password").authorize(request); + assertNotNull(response.getToken()); + rpt = toAccessToken(response.getToken()); + authorizationClaim = rpt.getAuthorization(); + permissions = new ArrayList<>(authorizationClaim.getPermissions()); + + assertEquals(1, permissions.size()); + + for (Permission permission : permissions) { + Map> claims = permission.getClaims(); + + assertNotNull(claims); + + assertThat(claims.get("claim-a"), Matchers.containsInAnyOrder("claim-a", "claim-a1")); + assertThat(claims.get("claim-b"), Matchers.containsInAnyOrder("claim-b")); + assertThat(claims.get("claim-c"), Matchers.containsInAnyOrder("claim-c")); + assertThat(claims.get("deny-policy"), Matchers.containsInAnyOrder("deny-policy")); + } + + response = authzClient.authorization("marta", "password").authorize(); + assertNotNull(response.getToken()); + rpt = toAccessToken(response.getToken()); + authorizationClaim = rpt.getAuthorization(); + permissions = new ArrayList<>(authorizationClaim.getPermissions()); + + assertEquals(1, permissions.size()); + + for (Permission permission : permissions) { + Map> claims = permission.getClaims(); + + assertNotNull(claims); + + assertThat(claims.get("claim-a"), Matchers.containsInAnyOrder("claim-a", "claim-a1")); + assertThat(claims.get("claim-b"), Matchers.containsInAnyOrder("claim-b")); + assertThat(claims.get("claim-c"), Matchers.containsInAnyOrder("claim-c")); + assertThat(claims.get("deny-policy"), Matchers.containsInAnyOrder("deny-policy")); + assertThat(permission.getScopes(), Matchers.containsInAnyOrder("create", "update")); + } + + updatePermission.setPolicies(new HashSet<>()); + updatePermission.addPolicy(claimCPolicy.getName()); + authorization.permissions().resource().findById(updatePermission.getId()).update(updatePermission); + + response = authzClient.authorization("marta", "password").authorize(); + assertNotNull(response.getToken()); + rpt = toAccessToken(response.getToken()); + authorizationClaim = rpt.getAuthorization(); + permissions = new ArrayList<>(authorizationClaim.getPermissions()); + + assertEquals(2, permissions.size()); + + for (Permission permission : permissions) { + Map> claims = permission.getClaims(); + + assertNotNull(claims); + + assertThat(claims.get("claim-a"), Matchers.containsInAnyOrder("claim-a", "claim-a1")); + assertThat(claims.get("claim-b"), Matchers.containsInAnyOrder("claim-b")); + assertThat(claims.get("claim-c"), Matchers.containsInAnyOrder("claim-c")); + } + } + private RealmResource getRealm() throws Exception { return adminClient.realm("authz-test"); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaPermissionTicketPushedClaimsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaPermissionTicketPushedClaimsTest.java index 0d36913742..cdbd9e4f29 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaPermissionTicketPushedClaimsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaPermissionTicketPushedClaimsTest.java @@ -16,15 +16,24 @@ */ package org.keycloak.testsuite.authz; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import java.util.Collection; +import java.util.Map; +import java.util.Set; + +import org.hamcrest.Matchers; import org.junit.Test; import org.keycloak.admin.client.resource.AuthorizationResource; import org.keycloak.authorization.client.AuthzClient; +import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.authorization.AuthorizationRequest; import org.keycloak.representations.idm.authorization.AuthorizationResponse; import org.keycloak.representations.idm.authorization.JSPolicyRepresentation; +import org.keycloak.representations.idm.authorization.Permission; import org.keycloak.representations.idm.authorization.PermissionRequest; import org.keycloak.representations.idm.authorization.PermissionResponse; import org.keycloak.representations.idm.authorization.ResourceRepresentation; @@ -82,6 +91,18 @@ public class UmaPermissionTicketPushedClaimsTest extends AbstractResourceServerT assertNotNull(authorizationResponse); assertNotNull(authorizationResponse.getToken()); + AccessToken token = toAccessToken(authorizationResponse.getToken()); + Collection permissions = token.getAuthorization().getPermissions(); + + assertEquals(1, permissions.size()); + + Permission permission = permissions.iterator().next(); + Map> claims = permission.getClaims(); + + assertNotNull(claims); + + assertThat(claims.get("my.bank.account.withdraw.value"), Matchers.containsInAnyOrder("50.5")); + permissionRequest.setClaim("my.bank.account.withdraw.value", "100.5"); response = authzClient.protection("marta", "password").permission().create(permissionRequest);