From 3c8ed8e3d8463a16767bc8cde3dbb0a577a92b7b Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 29 Jul 2016 05:18:38 -0300 Subject: [PATCH] [KEYCLOAK-3372] - Code cleanup --- .../authorization/AbstractPolicyEnforcer.java | 27 +++-- .../KeycloakAdapterPolicyEnforcer.java | 2 +- .../authorization/PolicyEnforcer.java | 2 - .../adapters/config/PolicyEnforcerConfig.java | 7 +- .../authorization/PolicyRepresentation.java | 3 + .../photoz-restful-api-authz-service.json | 5 - .../example/photoz/album/AlbumService.java | 7 +- .../src/main/webapp/WEB-INF/keycloak.json | 13 ++- .../servlet-authz-app-config.json | 6 +- .../authorization/attribute/Attributes.java | 3 +- .../evaluation/DefaultPolicyEvaluator.java | 50 ++++++--- .../admin/PolicyEvaluationService.java | 12 +- .../AuthorizationTokenService.java | 18 ++- .../entitlement/EntitlementService.java | 19 ++-- .../authorization/util/Permissions.java | 105 +++++++++--------- .../example/photoz/album/AlbumService.java | 7 +- .../src/main/webapp/WEB-INF/keycloak.json | 15 ++- .../AbstractPhotozExampleAdapterTest.java | 7 ++ 18 files changed, 179 insertions(+), 129 deletions(-) diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java index 6b1fe19fd8..0c0fc236f9 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java @@ -126,11 +126,13 @@ public abstract class AbstractPolicyEnforcer { List permissions = authorization.getPermissions(); for (Permission permission : permissions) { - Set allowedScopes = permission.getScopes(); - if (permission.getResourceSetId() != null) { if (isResourcePermission(actualPathConfig, permission)) { - if (((allowedScopes == null || allowedScopes.isEmpty()) && requiredScopes.isEmpty()) || allowedScopes.containsAll(requiredScopes)) { + if (actualPathConfig.isInstance() && !matchResourcePermission(actualPathConfig, permission)) { + continue; + + } + if (hasResourceScopePermission(requiredScopes, permission, actualPathConfig)) { LOGGER.debugf("Authorization GRANTED for path [%s]. Permissions [%s].", actualPathConfig, permissions); if (request.getMethod().equalsIgnoreCase("DELETE") && actualPathConfig.isInstance()) { this.paths.remove(actualPathConfig); @@ -138,11 +140,6 @@ public abstract class AbstractPolicyEnforcer { return true; } } - } else { - if ((allowedScopes.isEmpty() && requiredScopes.isEmpty()) || allowedScopes.containsAll(requiredScopes)) { - LOGGER.debugf("Authorization GRANTED for path [%s]. Permissions [%s].", actualPathConfig, permissions); - return true; - } } } @@ -151,6 +148,11 @@ public abstract class AbstractPolicyEnforcer { return false; } + private boolean hasResourceScopePermission(Set requiredScopes, Permission permission, PathConfig actualPathConfig) { + Set allowedScopes = permission.getScopes(); + return (allowedScopes.containsAll(requiredScopes) || allowedScopes.isEmpty()); + } + protected AuthzClient getAuthzClient() { return this.authzClient; } @@ -210,7 +212,6 @@ public abstract class AbstractPolicyEnforcer { config.setPath(targetResource.getUri()); config.setScopes(originalConfig.getScopes()); config.setMethods(originalConfig.getMethods()); - config.setInstance(true); config.setParentConfig(originalConfig); this.paths.add(config); @@ -244,13 +245,17 @@ public abstract class AbstractPolicyEnforcer { private boolean isResourcePermission(PathConfig actualPathConfig, Permission permission) { // first we try a match using resource id - boolean resourceMatch = permission.getResourceSetId().equals(actualPathConfig.getId()); + boolean resourceMatch = matchResourcePermission(actualPathConfig, permission); // as a fallback, check if the current path is an instance and if so, check if parent's id matches the permission if (!resourceMatch && actualPathConfig.isInstance()) { - resourceMatch = permission.getResourceSetId().equals(actualPathConfig.getParentConfig().getId()); + resourceMatch = matchResourcePermission(actualPathConfig.getParentConfig(), permission); } return resourceMatch; } + + private boolean matchResourcePermission(PathConfig actualPathConfig, Permission permission) { + return permission.getResourceSetId().equals(actualPathConfig.getId()); + } } diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java index e151f7620e..a77676292c 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java @@ -50,7 +50,7 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer { int retry = 2; AccessToken original = accessToken; - while (retry >= 0) { + while (retry > 0) { if (super.isAuthorized(pathConfig, requiredScopes, accessToken, httpFacade)) { original.setAuthorization(accessToken.getAuthorization()); return true; diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java index 5c2612449d..aa6d3d25d0 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java @@ -45,7 +45,6 @@ public class PolicyEnforcer { private static Logger LOGGER = Logger.getLogger(PolicyEnforcer.class); private final KeycloakDeployment deployment; - private final PathMatcher pathMatcher; private final AuthzClient authzClient; private final PolicyEnforcerConfig enforcerConfig; private final List paths; @@ -54,7 +53,6 @@ public class PolicyEnforcer { this.deployment = deployment; this.enforcerConfig = adapterConfig.getPolicyEnforcerConfig(); this.authzClient = AuthzClient.create(new Configuration(adapterConfig.getAuthServerUrl(), adapterConfig.getRealm(), adapterConfig.getResource(), adapterConfig.getCredentials(), deployment.getClient())); - this.pathMatcher = new PathMatcher(); this.paths = configurePaths(this.authzClient.protection().resource(), this.enforcerConfig); if (LOGGER.isDebugEnabled()) { diff --git a/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java b/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java index b2c5757faa..9cf710ac5d 100644 --- a/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java +++ b/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java @@ -109,7 +109,6 @@ public class PolicyEnforcerConfig { private List methods = new ArrayList<>(); private List scopes = Collections.emptyList(); private String id; - private boolean instance; @JsonIgnore private PathConfig parentConfig; @@ -178,11 +177,7 @@ public class PolicyEnforcerConfig { } public boolean isInstance() { - return instance; - } - - public void setInstance(boolean instance) { - this.instance = instance; + return this.parentConfig != null; } public void setParentConfig(PathConfig parentConfig) { diff --git a/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyRepresentation.java index dd219012dc..9a51031782 100644 --- a/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyRepresentation.java @@ -16,6 +16,8 @@ */ package org.keycloak.representations.idm.authorization; +import com.fasterxml.jackson.annotation.JsonInclude; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -35,6 +37,7 @@ public class PolicyRepresentation { private DecisionStrategy decisionStrategy = DecisionStrategy.UNANIMOUS; private Map config = new HashMap(); private List dependentPolicies; + @JsonInclude(JsonInclude.Include.NON_EMPTY) private List associatedPolicies = new ArrayList<>(); public String getId() { diff --git a/examples/authz/photoz/photoz-restful-api-authz-service.json b/examples/authz/photoz/photoz-restful-api-authz-service.json index 6547d2fcc4..455948c493 100644 --- a/examples/authz/photoz/photoz-restful-api-authz-service.json +++ b/examples/authz/photoz/photoz-restful-api-authz-service.json @@ -52,7 +52,6 @@ "sessionName": "MainOwnerSession", "mavenArtifactGroupId": "org.keycloak", "moduleName": "PhotozAuthzOwnerPolicy", - "applyPolicies": "[]", "scannerPeriod": "1", "scannerPeriodUnit": "Hours" } @@ -64,7 +63,6 @@ "logic": "POSITIVE", "decisionStrategy": "UNANIMOUS", "config": { - "applyPolicies": "[]", "roles": "[{\"id\":\"admin\",\"required\":true}]" } }, @@ -75,7 +73,6 @@ "logic": "POSITIVE", "decisionStrategy": "UNANIMOUS", "config": { - "applyPolicies": "[]", "roles": "[{\"id\":\"user\"},{\"id\":\"manage-albums\",\"required\":true}]" } }, @@ -86,7 +83,6 @@ "logic": "POSITIVE", "decisionStrategy": "UNANIMOUS", "config": { - "applyPolicies": "[]", "code": "var contextAttributes = $evaluation.getContext().getAttributes();\n\nif (contextAttributes.containsValue('kc.client.network.ip_address', '127.0.0.1')) {\n $evaluation.grant();\n}" } }, @@ -117,7 +113,6 @@ "logic": "POSITIVE", "decisionStrategy": "UNANIMOUS", "config": { - "applyPolicies": "[]", "code": "var context = $evaluation.getContext();\nvar identity = context.getIdentity();\nvar attributes = identity.getAttributes();\nvar email = attributes.getValue('email').asString(0);\n\nif (identity.hasRole('admin') || email.endsWith('@keycloak.org')) {\n $evaluation.grant();\n}" } }, diff --git a/examples/authz/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java b/examples/authz/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java index c28bca3bc0..a5d7f161b4 100644 --- a/examples/authz/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java +++ b/examples/authz/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java @@ -109,7 +109,12 @@ public class AlbumService { private void createProtectedResource(Album album) { try { - ResourceRepresentation albumResource = new ResourceRepresentation(album.getName(), new HashSet(), "/album/" + album.getId(), "http://photoz.com/album"); + HashSet scopes = new HashSet<>(); + + scopes.add(new ScopeRepresentation(SCOPE_ALBUM_VIEW)); + scopes.add(new ScopeRepresentation(SCOPE_ALBUM_DELETE)); + + ResourceRepresentation albumResource = new ResourceRepresentation(album.getName(), scopes, "/album/" + album.getId(), "http://photoz.com/album"); albumResource.setOwner(album.getUserId()); diff --git a/examples/authz/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json b/examples/authz/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json index 95fb58bf70..6849d0771b 100644 --- a/examples/authz/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json +++ b/examples/authz/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json @@ -9,17 +9,18 @@ "secret": "secret" }, "policy-enforcer": { + "user-managed-access" : {}, "paths": [ { "path" : "/album/*", "methods" : [ - { - "method": "GET", - "scopes" : ["urn:photoz.com:scopes:album:view"] - }, { "method": "POST", "scopes" : ["urn:photoz.com:scopes:album:create"] + }, + { + "method": "GET", + "scopes" : ["urn:photoz.com:scopes:album:view"] } ] }, @@ -30,6 +31,10 @@ { "method": "DELETE", "scopes" : ["urn:photoz.com:scopes:album:delete"] + }, + { + "method": "GET", + "scopes" : ["urn:photoz.com:scopes:album:view"] } ] }, diff --git a/examples/authz/servlet-authz/servlet-authz-app-config.json b/examples/authz/servlet-authz/servlet-authz-app-config.json index d5fb1cb811..43ebde42b8 100644 --- a/examples/authz/servlet-authz/servlet-authz-app-config.json +++ b/examples/authz/servlet-authz/servlet-authz-app-config.json @@ -54,7 +54,7 @@ "description": "Defines that adminsitrators can do something", "type": "role", "config": { - "roles": "[\"admin\"]" + "roles": "[{\"id\":\"admin\"}]" } }, { @@ -62,7 +62,7 @@ "description": "Defines that any user can do something", "type": "role", "config": { - "roles": "[\"user\"]" + "roles": "[{\"id\":\"user\"}]" } }, { @@ -71,7 +71,7 @@ "type": "role", "logic": "POSITIVE", "config": { - "roles": "[\"user_premium\"]" + "roles": "[{\"id\":\"user_premium\"}]" } }, { diff --git a/server-spi/src/main/java/org/keycloak/authorization/attribute/Attributes.java b/server-spi/src/main/java/org/keycloak/authorization/attribute/Attributes.java index a70d3c075e..a0a7b6c0d1 100644 --- a/server-spi/src/main/java/org/keycloak/authorization/attribute/Attributes.java +++ b/server-spi/src/main/java/org/keycloak/authorization/attribute/Attributes.java @@ -67,7 +67,8 @@ public interface Attributes { * @return true if any attribute with name and value exist. Otherwise, returns false. */ default boolean containsValue(String name, String value) { - return toMap().getOrDefault(name, emptyList()).stream().anyMatch(value::equals); + Collection values = toMap().get(name); + return values != null && values.stream().anyMatch(value::equals); } /** diff --git a/server-spi/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultPolicyEvaluator.java b/server-spi/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultPolicyEvaluator.java index e2ef2f96d1..d5fa9cc9fa 100644 --- a/server-spi/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultPolicyEvaluator.java +++ b/server-spi/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultPolicyEvaluator.java @@ -32,8 +32,10 @@ import org.keycloak.authorization.store.StoreFactory; import org.keycloak.representations.idm.authorization.PolicyEnforcementMode; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -130,27 +132,49 @@ public class DefaultPolicyEvaluator implements PolicyEvaluator { return true; } - if (policy.getScopes().isEmpty()) { - return true; - } + Set scopes = new HashSet<>(policy.getScopes()); - boolean hasScope = true; + if (scopes.isEmpty()) { + Set resources = new HashSet<>(); - for (Scope givenScope : policy.getScopes()) { - boolean hasGivenScope = false; + resources.addAll(policy.getResources()); - for (Scope scope : permission.getScopes()) { - if (givenScope.getId().equals(scope.getId())) { - hasGivenScope = true; - break; + for (Resource resource : resources) { + scopes.addAll(resource.getScopes()); + } + + if (!resources.isEmpty() && scopes.isEmpty()) { + return false; + } + + if (scopes.isEmpty()) { + Resource resource = permission.getResource(); + String type = resource.getType(); + + if (type != null) { + List resourcesByType = authorization.getStoreFactory().getResourceStore().findByType(type); + + for (Resource resourceType : resourcesByType) { + if (resourceType.getOwner().equals(resource.getResourceServer().getClientId())) { + resources.add(resourceType); + } + } } } - if (!hasGivenScope) { - return false; + for (Resource resource : resources) { + scopes.addAll(resource.getScopes()); } } - return hasScope; + for (Scope givenScope : scopes) { + for (Scope scope : permission.getScopes()) { + if (givenScope.getId().equals(scope.getId())) { + return true; + } + } + } + + return false; } } 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 8e4ec5e318..6712a9dac3 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyEvaluationService.java @@ -240,10 +240,16 @@ public class PolicyEvaluationService { } } - accessToken.addAccess(clientModel.getClientId()); - AccessToken.Access resourceAccess = accessToken.getResourceAccess(clientModel.getClientId()); + AccessToken.Access clientAccess = accessToken.addAccess(clientModel.getClientId()); + clientAccess.roles(new HashSet<>()); - userModel.getClientRoleMappings(clientModel).stream().map(RoleModel::getName).forEach(roleName -> resourceAccess.addRole(roleName)); + userModel.getClientRoleMappings(clientModel).stream().map(RoleModel::getName).forEach(roleName -> clientAccess.addRole(roleName)); + + ClientModel resourceServerClient = realm.getClientById(resourceServer.getClientId()); + AccessToken.Access resourceServerAccess = accessToken.addAccess(resourceServerClient.getClientId()); + resourceServerAccess.roles(new HashSet<>()); + + userModel.getClientRoleMappings(resourceServerClient).stream().map(RoleModel::getName).forEach(roleName -> resourceServerAccess.addRole(roleName)); } } } 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 405675a544..0fd347488d 100644 --- a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java +++ b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java @@ -17,6 +17,7 @@ package org.keycloak.authorization.authorization; import org.jboss.resteasy.spi.HttpRequest; +import org.keycloak.OAuth2Constants; import org.keycloak.OAuthErrorException; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.authorization.representation.AuthorizationRequest; @@ -50,8 +51,6 @@ import javax.ws.rs.container.Suspended; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -104,8 +103,12 @@ public class AuthorizationTokenService { List entitlements = Permissions.allPermits(results); if (entitlements.isEmpty()) { + HashMap error = new HashMap<>(); + + error.put(OAuth2Constants.ERROR, "not_authorized"); + asyncResponse.resume(Cors.add(httpRequest, Response.status(Status.FORBIDDEN) - .entity(new ErrorResponseException("not_authorized", "Authorization denied.", Status.FORBIDDEN))) + .entity(error)) .allowedOrigins(identity.getAccessToken()) .exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS).build()); } else { @@ -193,14 +196,7 @@ public class AuthorizationTokenService { return permissionsToEvaluate.entrySet().stream() .flatMap((Function>, Stream>) entry -> { Resource entryResource = storeFactory.getResourceStore().findById(entry.getKey()); - if (entry.getValue().isEmpty()) { - return Arrays.asList(new ResourcePermission(entryResource, Collections.emptyList(), entryResource.getResourceServer())).stream(); - } else { - return entry.getValue().stream() - .map(scopeName -> storeFactory.getScopeStore().findByName(scopeName, entryResource.getResourceServer().getId())) - .filter(scope -> scope != null) - .map(scope -> new ResourcePermission(entryResource, Arrays.asList(scope), entryResource.getResourceServer())); - } + return Permissions.createResourcePermissions(entryResource, entry.getValue(), authorization).stream(); }).collect(Collectors.toList()); } 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 ccc457d6b9..e5986b78ae 100644 --- a/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java +++ b/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java @@ -18,6 +18,7 @@ package org.keycloak.authorization.entitlement; import org.jboss.resteasy.spi.HttpRequest; +import org.keycloak.OAuth2Constants; import org.keycloak.OAuthErrorException; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.common.KeycloakEvaluationContext; @@ -55,8 +56,6 @@ import javax.ws.rs.container.Suspended; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -119,8 +118,12 @@ public class EntitlementService { List entitlements = Permissions.allPermits(results); if (entitlements.isEmpty()) { + HashMap error = new HashMap<>(); + + error.put(OAuth2Constants.ERROR, "not_authorized"); + asyncResponse.resume(Cors.add(request, Response.status(Status.FORBIDDEN) - .entity(new ErrorResponseException("not_authorized", "Authorization denied.", Status.FORBIDDEN))) + .entity(error)) .allowedOrigins(identity.getAccessToken()) .exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS).build()); } else { @@ -249,15 +252,7 @@ public class EntitlementService { return permissionsToEvaluate.entrySet().stream() .flatMap((Function>, Stream>) entry -> { Resource entryResource = storeFactory.getResourceStore().findById(entry.getKey()); - - if (entry.getValue().isEmpty()) { - return Arrays.asList(new ResourcePermission(entryResource, Collections.emptyList(), entryResource.getResourceServer())).stream(); - } else { - return entry.getValue().stream() - .map(scopeName -> storeFactory.getScopeStore().findByName(scopeName, entryResource.getResourceServer().getId())) - .filter(scope -> scope != null) - .map(scope -> new ResourcePermission(entryResource, Arrays.asList(scope), entryResource.getResourceServer())); - } + return Permissions.createResourcePermissions(entryResource, entry.getValue(), authorization).stream(); }).collect(Collectors.toList()); } } 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 ed49697d60..ebc57c2951 100644 --- a/services/src/main/java/org/keycloak/authorization/util/Permissions.java +++ b/services/src/main/java/org/keycloak/authorization/util/Permissions.java @@ -27,6 +27,7 @@ import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.permission.ResourcePermission; import org.keycloak.authorization.policy.evaluation.Result; import org.keycloak.authorization.store.ResourceStore; +import org.keycloak.authorization.store.ScopeStore; import org.keycloak.authorization.store.StoreFactory; import org.keycloak.representations.idm.authorization.Permission; @@ -61,91 +62,95 @@ public final class Permissions { StoreFactory storeFactory = authorization.getStoreFactory(); ResourceStore resourceStore = storeFactory.getResourceStore(); - resourceStore.findByOwner(resourceServer.getClientId()).stream().forEach(resource -> permissions.addAll(createResourcePermissions(resource, resourceServer, authorization))); - resourceStore.findByOwner(identity.getId()).stream().forEach(resource -> permissions.addAll(createResourcePermissions(resource, resourceServer, authorization))); + resourceStore.findByOwner(resourceServer.getClientId()).stream().forEach(resource -> permissions.addAll(createResourcePermissions(resource, resource.getScopes().stream().map(Scope::getName).collect(Collectors.toSet()), authorization))); + resourceStore.findByOwner(identity.getId()).stream().forEach(resource -> permissions.addAll(createResourcePermissions(resource, resource.getScopes().stream().map(Scope::getName).collect(Collectors.toSet()), authorization))); return permissions; } - public static List createResourcePermissions(Resource resource, ResourceServer resourceServer, AuthorizationProvider authorization) { + public static List createResourcePermissions(Resource resource, Set requestedScopes, AuthorizationProvider authorization) { List permissions = new ArrayList<>(); - List scopes = resource.getScopes(); - - if (scopes.isEmpty()) { - String type = resource.getType(); + String type = resource.getType(); + ResourceServer resourceServer = resource.getResourceServer(); + List scopes; + if (requestedScopes.isEmpty()) { + scopes = 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) { + if (type != null && !resource.getOwner().equals(resourceServer.getClientId())) { StoreFactory storeFactory = authorization.getStoreFactory(); ResourceStore resourceStore = storeFactory.getResourceStore(); resourceStore.findByType(type).forEach(resource1 -> { if (resource1.getOwner().equals(resourceServer.getClientId())) { - scopes.addAll(resource1.getScopes()); + for (Scope typeScope : resource1.getScopes()) { + if (!scopes.contains(typeScope)) { + scopes.add(typeScope); + } + } } }); } + } else { + ScopeStore scopeStore = authorization.getStoreFactory().getScopeStore(); + scopes = requestedScopes.stream().map(scopeName -> { + Scope byName = scopeStore.findByName(scopeName, resource.getResourceServer().getId()); + return byName; + }).collect(Collectors.toList()); } - if (scopes.size() > 1) { + if (scopes.isEmpty()) { + permissions.add(new ResourcePermission(resource, Collections.emptyList(), resource.getResourceServer())); + } else { for (Scope scope : scopes) { permissions.add(new ResourcePermission(resource, Arrays.asList(scope), resource.getResourceServer())); } - } else { - permissions.add(new ResourcePermission(resource, Collections.emptyList(), resource.getResourceServer())); } return permissions; } public static List allPermits(List evaluation) { - List permissions = evaluation.stream() - .filter(evaluationResult -> evaluationResult.getEffect().equals(Effect.PERMIT)) - .map(evaluationResult -> { - ResourcePermission permission = evaluationResult.getPermission(); - String resourceId = null; - String resourceName = null; + Map permissions = new HashMap<>(); - Resource resource = permission.getResource(); - - if (resource != null) { - resourceId = resource.getId(); - resourceName = resource.getName(); - } - - Set scopes = permission.getScopes().stream().map(Scope::getName).collect(Collectors.toSet()); - - return new Permission(resourceId, resourceName, scopes); - }).collect(Collectors.toList()); - - Map perms = new HashMap<>(); - - permissions.forEach(permission -> { - Permission evalPermission = perms.get(permission.getResourceSetId()); - - if (evalPermission == null) { - evalPermission = permission; - perms.put(permission.getResourceSetId(), evalPermission); + for (Result evaluationResult : evaluation) { + ResourcePermission permission = evaluationResult.getPermission(); + Set scopes = permission.getScopes().stream().map(Scope::getName).collect(Collectors.toSet()); + if (evaluationResult.getEffect().equals(Effect.DENY)) { + continue; } + Resource resource = permission.getResource(); - Set permissionScopes = permission.getScopes(); + if (resource != null) { + String resourceId = resource.getId(); + String resourceName = resource.getName(); + Permission evalPermission = permissions.get(resource.getId()); - if (permissionScopes != null && !permissionScopes.isEmpty()) { - Set scopes = evalPermission.getScopes(); - - if (scopes == null) { - scopes = new HashSet(); - evalPermission.setScopes(scopes); + if (evalPermission == null) { + evalPermission = new Permission(resourceId, resourceName, scopes); + permissions.put(resourceId, evalPermission); } - for (String scopeName : permissionScopes) { - if (!scopes.contains(scopeName)) { - scopes.add(scopeName); + if (scopes != null && !scopes.isEmpty()) { + Set finalScopes = evalPermission.getScopes(); + + if (finalScopes == null) { + finalScopes = new HashSet(); + evalPermission.setScopes(finalScopes); + } + + for (String scopeName : scopes) { + if (!finalScopes.contains(scopeName)) { + finalScopes.add(scopeName); + } } } + } else { + Permission scopePermission = new Permission(null, null, scopes); + permissions.put(scopePermission.toString(), scopePermission); } - }); + } - return perms.values().stream().collect(Collectors.toList()); + return permissions.values().stream().collect(Collectors.toList()); } } diff --git a/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java b/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java index c28bca3bc0..a5d7f161b4 100644 --- a/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java +++ b/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/java/org/keycloak/example/photoz/album/AlbumService.java @@ -109,7 +109,12 @@ public class AlbumService { private void createProtectedResource(Album album) { try { - ResourceRepresentation albumResource = new ResourceRepresentation(album.getName(), new HashSet(), "/album/" + album.getId(), "http://photoz.com/album"); + HashSet scopes = new HashSet<>(); + + scopes.add(new ScopeRepresentation(SCOPE_ALBUM_VIEW)); + scopes.add(new ScopeRepresentation(SCOPE_ALBUM_DELETE)); + + ResourceRepresentation albumResource = new ResourceRepresentation(album.getName(), scopes, "/album/" + album.getId(), "http://photoz.com/album"); albumResource.setOwner(album.getUserId()); diff --git a/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json b/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json index 95fb58bf70..a3ac697cb3 100644 --- a/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json +++ b/testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api/src/main/webapp/WEB-INF/keycloak.json @@ -1,7 +1,7 @@ { "realm": "photoz", "realm-public-key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQAB", - "auth-server-url": "http://localhost:8080/auth", + "auth-server-url": "http://localhost:8180/auth", "ssl-required": "external", "resource": "photoz-restful-api", "bearer-only" : true, @@ -9,17 +9,18 @@ "secret": "secret" }, "policy-enforcer": { + "user-managed-access" : {}, "paths": [ { "path" : "/album/*", "methods" : [ - { - "method": "GET", - "scopes" : ["urn:photoz.com:scopes:album:view"] - }, { "method": "POST", "scopes" : ["urn:photoz.com:scopes:album:create"] + }, + { + "method": "GET", + "scopes" : ["urn:photoz.com:scopes:album:view"] } ] }, @@ -30,6 +31,10 @@ { "method": "DELETE", "scopes" : ["urn:photoz.com:scopes:album:delete"] + }, + { + "method": "GET", + "scopes" : ["urn:photoz.com:scopes:album:view"] } ] }, diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractPhotozExampleAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractPhotozExampleAdapterTest.java index 8a8d483a24..28662fa573 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractPhotozExampleAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractPhotozExampleAdapterTest.java @@ -94,6 +94,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd importResourceServerSettings(); } + @Test public void testCreateDeleteAlbum() throws Exception { try { this.deployer.deploy(RESOURCE_SERVER_ID); @@ -115,6 +116,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd } } + @Test public void testOnlyOwnerCanDeleteAlbum() throws Exception { try { this.deployer.deploy(RESOURCE_SERVER_ID); @@ -160,6 +162,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd } } + @Test public void testRegularUserCanNotAccessAdminResources() throws Exception { try { this.deployer.deploy(RESOURCE_SERVER_ID); @@ -172,6 +175,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd } } + @Test public void testAdminOnlyFromSpecificAddress() throws Exception { try { this.deployer.deploy(RESOURCE_SERVER_ID); @@ -264,6 +268,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd } } + @Test public void testAdminWithoutPermissionsToDeleteScopePermission() throws Exception { try { this.deployer.deploy(RESOURCE_SERVER_ID); @@ -327,6 +332,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd } } + @Test public void testClientRoleRepresentingUserConsent() throws Exception { try { this.deployer.deploy(RESOURCE_SERVER_ID); @@ -366,6 +372,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd } } + @Test public void testClientRoleNotRequired() throws Exception { try { this.deployer.deploy(RESOURCE_SERVER_ID);