From 1e1de85685bb5d5f180f510630cd7133f8a35375 Mon Sep 17 00:00:00 2001 From: pedroigor Date: Thu, 1 Mar 2018 16:50:05 -0300 Subject: [PATCH] [KEYCLOAK-6787] - Wrong validation of resources with same name and different owners --- .../authorization/client/AuthzClient.java | 6 +- .../client/resource/ProtectedResource.java | 26 +- .../client/resource/ProtectionResource.java | 7 +- .../client/util/HttpResponseException.java | 8 + .../authorization/client/util/Throwables.java | 16 +- .../client/resource/ResourcesResource.java | 5 + .../StoreFactoryCacheManager.java | 2 +- .../StoreFactoryCacheSession.java | 23 +- .../jpa/entities/ResourceEntity.java | 2 +- .../jpa/store/JPAResourceStore.java | 6 + .../authorization/AuthorizationProvider.java | 7 +- ...ionTicketAwareDecisionResultCollector.java | 2 +- .../authorization/store/ResourceStore.java | 12 +- .../models/utils/RepresentationToModel.java | 52 ++-- .../admin/ResourceSetService.java | 5 +- .../AuthorizationTokenService.java | 50 ++-- .../common/KeycloakIdentity.java | 2 +- .../permission/AbstractPermissionService.java | 50 ++-- .../AbstractAuthorizationTest.java | 65 +++-- .../authorization/AuthorizationTest.java | 30 ++- .../ExportAuthorizationSettingsTest.java | 6 - .../GenericPolicyManagementTest.java | 13 - .../ImportAuthorizationSettingsTest.java | 17 +- .../authorization/ResourceManagementTest.java | 66 ++--- .../authorization/ScopeManagementTest.java | 7 - .../testsuite/authz/AuthorizationAPITest.java | 2 +- .../testsuite/authz/AuthorizationTest.java | 229 ++++++++++++++++++ .../authz/PermissionManagementTest.java | 8 +- .../testsuite/authz/UmaGrantTypeTest.java | 4 +- .../authz/UserManagedAccessTest.java | 24 +- 30 files changed, 535 insertions(+), 217 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationTest.java diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/AuthzClient.java b/authz/client/src/main/java/org/keycloak/authorization/client/AuthzClient.java index 1bf5c8697f..df24fdba95 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/AuthzClient.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/AuthzClient.java @@ -97,7 +97,7 @@ public class AuthzClient { * @return a {@link ProtectionResource} */ public ProtectionResource protection() { - return new ProtectionResource(this.http, this.serverConfiguration, createPatSupplier()); + return new ProtectionResource(this.http, this.serverConfiguration, configuration, createPatSupplier()); } /** @@ -107,7 +107,7 @@ public class AuthzClient { * @return a {@link ProtectionResource} */ public ProtectionResource protection(final String accessToken) { - return new ProtectionResource(this.http, this.serverConfiguration, new TokenCallable(http, configuration, serverConfiguration) { + return new ProtectionResource(this.http, this.serverConfiguration, configuration, new TokenCallable(http, configuration, serverConfiguration) { @Override public String call() { return accessToken; @@ -128,7 +128,7 @@ public class AuthzClient { * @return a {@link ProtectionResource} */ public ProtectionResource protection(String userName, String password) { - return new ProtectionResource(this.http, this.serverConfiguration, createPatSupplier(userName, password)); + return new ProtectionResource(this.http, this.serverConfiguration, configuration, createPatSupplier(userName, password)); } /** diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectedResource.java b/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectedResource.java index cc92712726..cf2e91a5f6 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectedResource.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectedResource.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.Callable; +import org.keycloak.authorization.client.Configuration; import org.keycloak.authorization.client.representation.ResourceRepresentation; import org.keycloak.authorization.client.representation.ServerConfiguration; import org.keycloak.authorization.client.util.Http; @@ -38,11 +39,13 @@ public class ProtectedResource { private final Http http; private ServerConfiguration serverConfiguration; + private final Configuration configuration; private final TokenCallable pat; - ProtectedResource(Http http, ServerConfiguration serverConfiguration, TokenCallable pat) { + ProtectedResource(Http http, ServerConfiguration serverConfiguration, Configuration configuration, TokenCallable pat) { this.http = http; this.serverConfiguration = serverConfiguration; + this.configuration = configuration; this.pat = pat; } @@ -119,13 +122,30 @@ public class ProtectedResource { } /** - * Query the server for a resource given its name. + * Query the server for a resource given its name where the owner is the resource server itself. * * @param id the resource name * @return a {@link ResourceRepresentation} */ public ResourceRepresentation findByName(String name) { - String[] representations = find(null, name, null, null, null, null, null, null); + String[] representations = find(null, name, null, configuration.getResource(), null, null, null, null); + + if (representations.length == 0) { + return null; + } + + return findById(representations[0]); + } + + /** + * Query the server for a resource given its name and a given ownerId. + * + * @param name the resource name + * @param ownerId the owner id + * @return a {@link ResourceRepresentation} + */ + public ResourceRepresentation findByName(String name, String ownerId) { + String[] representations = find(null, name, null, ownerId, null, null, null, null); if (representations.length == 0) { return null; diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectionResource.java b/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectionResource.java index 7268fe954a..03fa9451ae 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectionResource.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/resource/ProtectionResource.java @@ -17,6 +17,7 @@ */ package org.keycloak.authorization.client.resource; +import org.keycloak.authorization.client.Configuration; import org.keycloak.authorization.client.representation.ServerConfiguration; import org.keycloak.authorization.client.representation.TokenIntrospectionResponse; import org.keycloak.authorization.client.util.Http; @@ -31,15 +32,17 @@ public class ProtectionResource { private final TokenCallable pat; private final Http http; + private final Configuration configuration; private ServerConfiguration serverConfiguration; - public ProtectionResource(Http http, ServerConfiguration serverConfiguration, TokenCallable pat) { + public ProtectionResource(Http http, ServerConfiguration serverConfiguration, Configuration configuration, TokenCallable pat) { if (pat == null) { throw new RuntimeException("No access token was provided when creating client for Protection API."); } this.http = http; this.serverConfiguration = serverConfiguration; + this.configuration = configuration; this.pat = pat; } @@ -49,7 +52,7 @@ public class ProtectionResource { * @return a {@link ProtectedResource} */ public ProtectedResource resource() { - return new ProtectedResource(http, serverConfiguration, pat); + return new ProtectedResource(http, serverConfiguration, configuration, pat); } /** diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/util/HttpResponseException.java b/authz/client/src/main/java/org/keycloak/authorization/client/util/HttpResponseException.java index 3531f40a5f..ed20a67fed 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/util/HttpResponseException.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/util/HttpResponseException.java @@ -44,4 +44,12 @@ public class HttpResponseException extends RuntimeException { public byte[] getBytes() { return bytes; } + + @Override + public String toString() { + if (bytes != null) { + return new StringBuilder(super.toString()).append(" / Response from server: ").append(new String(bytes)).toString(); + } + return super.toString(); + } } diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/util/Throwables.java b/authz/client/src/main/java/org/keycloak/authorization/client/util/Throwables.java index ae2eaf11a6..2b0b79a712 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/util/Throwables.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/util/Throwables.java @@ -39,7 +39,7 @@ public final class Throwables { */ public static RuntimeException handleWrapException(String message, Throwable cause) { if (cause instanceof HttpResponseException) { - throw handleAndWrapHttpResponseException(message, HttpResponseException.class.cast(cause)); + throw handleAndWrapHttpResponseException(HttpResponseException.class.cast(cause)); } return new RuntimeException(message, cause); @@ -91,19 +91,11 @@ public final class Throwables { throw new RuntimeException(message, cause); } - private static RuntimeException handleAndWrapHttpResponseException(String message, HttpResponseException exception) { - HttpResponseException hre = HttpResponseException.class.cast(exception); - StringBuilder detail = new StringBuilder(message); - byte[] bytes = hre.getBytes(); - - if (bytes != null) { - detail.append(". Server message: ").append(new String(bytes)); - } - + private static RuntimeException handleAndWrapHttpResponseException(HttpResponseException exception) { if (403 == exception.getStatusCode()) { - throw new AuthorizationDeniedException(detail.toString(), exception); + throw new AuthorizationDeniedException(exception); } - return new RuntimeException(detail.toString(), exception); + return new RuntimeException(exception); } } diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ResourcesResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ResourcesResource.java index e7daaa18e8..7f2c39457b 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ResourcesResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ResourcesResource.java @@ -59,6 +59,11 @@ public interface ResourcesResource { @Produces(MediaType.APPLICATION_JSON) List findByName(@QueryParam("name") String name); + @GET + @NoCache + @Produces(MediaType.APPLICATION_JSON) + List findByName(@QueryParam("name") String name, @QueryParam("owner") String owner); + @GET @NoCache @Produces(MediaType.APPLICATION_JSON) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheManager.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheManager.java index 0f245eeaa5..e54316d022 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheManager.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheManager.java @@ -79,7 +79,7 @@ public class StoreFactoryCacheManager extends CacheManager { public void resourceUpdated(String id, String name, String type, String uri, Set scopes, String serverId, String owner, Set invalidations) { invalidations.add(id); - invalidations.add(StoreFactoryCacheSession.getResourceByNameCacheKey(name, serverId)); + invalidations.add(StoreFactoryCacheSession.getResourceByNameCacheKey(name, owner, serverId)); invalidations.add(StoreFactoryCacheSession.getResourceByOwnerCacheKey(owner, serverId)); invalidations.add(StoreFactoryCacheSession.getResourceByOwnerCacheKey(owner, null)); invalidations.add(StoreFactoryCacheSession.getPermissionTicketByResource(id, serverId)); diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java index c50cec1ec7..5924573df6 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java @@ -336,8 +336,8 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider { return "scope.name." + name + "." + serverId; } - public static String getResourceByNameCacheKey(String name, String serverId) { - return "resource.name." + name + "." + serverId; + public static String getResourceByNameCacheKey(String name, String ownerId, String serverId) { + return "resource.name." + name + "." + ownerId + "." + serverId; } public static String getResourceByOwnerCacheKey(String owner, String serverId) { @@ -580,17 +580,22 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider { @Override public Resource findByName(String name, String resourceServerId) { + return findByName(name, resourceServerId, resourceServerId); + } + + @Override + public Resource findByName(String name, String ownerId, String resourceServerId) { if (name == null) return null; - String cacheKey = getResourceByNameCacheKey(name, resourceServerId); + String cacheKey = getResourceByNameCacheKey(name, ownerId, resourceServerId); List result = cacheQuery(cacheKey, ResourceListQuery.class, () -> { - Resource resource = getResourceStoreDelegate().findByName(name, resourceServerId); + Resource resource = getResourceStoreDelegate().findByName(name, ownerId, resourceServerId); - if (resource == null) { - return Collections.emptyList(); - } + if (resource == null) { + return Collections.emptyList(); + } - return Arrays.asList(resource); - }, + return Arrays.asList(resource); + }, (revision, resources) -> new ResourceListQuery(revision, cacheKey, resources.stream().map(resource -> resource.getId()).collect(Collectors.toSet()), resourceServerId), resourceServerId); if (result.isEmpty()) { diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/ResourceEntity.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/ResourceEntity.java index 6031a6e92a..861853a32d 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/ResourceEntity.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/ResourceEntity.java @@ -47,7 +47,7 @@ import java.util.List; @NamedQuery(name="findResourceIdByOwner", query="select r.id from ResourceEntity r where r.resourceServer.id = :serverId and r.owner = :owner"), @NamedQuery(name="findAnyResourceIdByOwner", query="select r.id from ResourceEntity r where r.owner = :owner"), @NamedQuery(name="findResourceIdByUri", query="select r.id from ResourceEntity r where r.resourceServer.id = :serverId and r.uri = :uri"), - @NamedQuery(name="findResourceIdByName", query="select r.id from ResourceEntity r where r.resourceServer.id = :serverId and r.name = :name"), + @NamedQuery(name="findResourceIdByName", query="select r.id from ResourceEntity r where r.resourceServer.id = :serverId and r.owner = :ownerId and r.name = :name"), @NamedQuery(name="findResourceIdByType", query="select r.id from ResourceEntity r where r.resourceServer.id = :serverId and r.type = :type"), @NamedQuery(name="findResourceIdByServerId", query="select r.id from ResourceEntity r where r.resourceServer.id = :serverId "), @NamedQuery(name="findResourceIdByScope", query="select r.id from ResourceEntity r inner join r.scopes s where r.resourceServer.id = :serverId and (s.resourceServer.id = :serverId and s.id in (:scopeIds))"), diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java index c938afe239..0c82dc01de 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java @@ -237,11 +237,17 @@ public class JPAResourceStore implements ResourceStore { @Override public Resource findByName(String name, String resourceServerId) { + return findByName(name, resourceServerId, resourceServerId); + } + + @Override + public Resource findByName(String name, String ownerId, String resourceServerId) { TypedQuery query = entityManager.createNamedQuery("findResourceIdByName", String.class); query.setFlushMode(FlushModeType.COMMIT); query.setParameter("serverId", resourceServerId); query.setParameter("name", name); + query.setParameter("ownerId", ownerId); try { String id = query.getSingleResult(); diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java b/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java index 0f58741fe2..88c898b3ce 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java @@ -285,7 +285,7 @@ public final class AuthorizationProvider implements Provider { } if (resource == null) { - throw new RuntimeException("Resource [" + id + "] does not exist"); + throw new RuntimeException("Resource [" + id + "] does not exist or is not owned by the resource server."); } return resource.getId(); @@ -459,6 +459,11 @@ public final class AuthorizationProvider implements Provider { return delegate.findByName(name, resourceServerId); } + @Override + public Resource findByName(String name, String ownerId, String resourceServerId) { + return delegate.findByName(name, ownerId, resourceServerId); + } + @Override public List findByType(String type, String resourceServerId) { return delegate.findByType(type, resourceServerId); diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java index 708f7e1c5d..df7c705eb6 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java @@ -117,7 +117,7 @@ public class PermissionTicketAwareDecisionResultCollector extends DecisionResult Resource resource = resourceStore.findById(permission.getResourceId(), resourceServer.getId()); if (resource == null) { - resource = resourceStore.findByName(permission.getResourceId(), resourceServer.getId()); + resource = resourceStore.findByName(permission.getResourceId(), identity.getId(), resourceServer.getId()); } if (!resource.isOwnerManagedAccess() || resource.getOwner().equals(identity.getId()) || resource.getOwner().equals(resourceServer.getId())) { diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java index 9a2ac51609..fd6f85c4d1 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/ResourceStore.java @@ -97,7 +97,7 @@ public interface ResourceStore { List findByScope(List id, String resourceServerId); /** - * Find a {@link Resource} by its name. + * Find a {@link Resource} by its name where the owner is the resource server itself. * * @param name the name of the resource * @param resourceServerId the identifier of the resource server @@ -105,6 +105,16 @@ public interface ResourceStore { */ Resource findByName(String name, String resourceServerId); + /** + * Find a {@link Resource} by its name where the owner is the given ownerId. + * + * @param name the name of the resource + * @param ownerId the owner id + * @param resourceServerId the identifier of the resource server + * @return a resource with the given name + */ + Resource findByName(String name, String ownerId, String resourceServerId); + /** * Finds all {@link Resource} with the given type. * diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index c738b818ab..732491383a 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -2275,7 +2275,7 @@ public class RepresentationToModel { if (resource == null) { resource = storeFactory.getResourceStore().findByName(resourceId, policy.getResourceServer().getId()); if (resource == null) { - throw new RuntimeException("Resource with id or name [" + resourceId + "] does not exist"); + throw new RuntimeException("Resource with id or name [" + resourceId + "] does not exist or is not owned by the resource server"); } } @@ -2303,28 +2303,6 @@ public class RepresentationToModel { public static Resource toModel(ResourceRepresentation resource, ResourceServer resourceServer, AuthorizationProvider authorization) { ResourceStore resourceStore = authorization.getStoreFactory().getResourceStore(); - Resource existing; - - if (resource.getId() != null) { - existing = resourceStore.findById(resource.getId(), resourceServer.getId()); - } else { - existing = resourceStore.findByName(resource.getName(), resourceServer.getId()); - } - - if (existing != null) { - existing.setName(resource.getName()); - existing.setDisplayName(resource.getDisplayName()); - existing.setType(resource.getType()); - existing.setUri(resource.getUri()); - existing.setIconUri(resource.getIconUri()); - existing.setOwnerManagedAccess(Boolean.TRUE.equals(resource.getOwnerManagedAccess())); - existing.updateScopes(resource.getScopes().stream() - .map((ScopeRepresentation scope) -> toModel(scope, resourceServer, authorization)) - .collect(Collectors.toSet())); - - return existing; - } - ResourceOwnerRepresentation owner = resource.getOwner(); if (owner == null) { @@ -2338,12 +2316,6 @@ public class RepresentationToModel { throw new RuntimeException("No owner specified for resource [" + resource.getName() + "]."); } - ClientModel clientModel = authorization.getRealm().getClientById(resourceServer.getId()); - - if (ownerId.equals(clientModel.getClientId())) { - ownerId = resourceServer.getId(); - } - if (!resourceServer.getId().equals(ownerId)) { RealmModel realm = authorization.getRealm(); KeycloakSession keycloakSession = authorization.getKeycloakSession(); @@ -2361,6 +2333,28 @@ public class RepresentationToModel { ownerId = ownerModel.getId(); } + Resource existing; + + if (resource.getId() != null) { + existing = resourceStore.findById(resource.getId(), resourceServer.getId()); + } else { + existing = resourceStore.findByName(resource.getName(), ownerId, resourceServer.getId()); + } + + if (existing != null) { + existing.setName(resource.getName()); + existing.setDisplayName(resource.getDisplayName()); + existing.setType(resource.getType()); + existing.setUri(resource.getUri()); + existing.setIconUri(resource.getIconUri()); + existing.setOwnerManagedAccess(Boolean.TRUE.equals(resource.getOwnerManagedAccess())); + existing.updateScopes(resource.getScopes().stream() + .map((ScopeRepresentation scope) -> toModel(scope, resourceServer, authorization)) + .collect(Collectors.toSet())); + + return existing; + } + Resource model = resourceStore.create(resource.getName(), resourceServer, ownerId); model.setDisplayName(resource.getDisplayName()); diff --git a/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java b/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java index 0e4f9117d1..04c9d74de5 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java @@ -118,7 +118,6 @@ public class ResourceSetService { public Response create(ResourceRepresentation resource, Function toRepresentation) { requireManage(); StoreFactory storeFactory = this.authorization.getStoreFactory(); - Resource existingResource = storeFactory.getResourceStore().findByName(resource.getName(), this.resourceServer.getId()); ResourceOwnerRepresentation owner = resource.getOwner(); if (owner == null) { @@ -132,7 +131,9 @@ public class ResourceSetService { return ErrorResponse.error("You must specify the resource owner.", Status.BAD_REQUEST); } - if (existingResource != null && existingResource.getOwner().equals(ownerId)) { + Resource existingResource = storeFactory.getResourceStore().findByName(resource.getName(), ownerId, this.resourceServer.getId()); + + if (existingResource != null) { return ErrorResponse.exists("Resource with name [" + resource.getName() + "] already exists."); } 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 a8075a09cb..989d228a9b 100644 --- a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java +++ b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java @@ -172,13 +172,13 @@ public class AuthorizationTokenService { private List evaluatePermissions(AuthorizationRequest authorizationRequest, PermissionTicketToken ticket, ResourceServer resourceServer, KeycloakEvaluationContext evaluationContext, KeycloakIdentity identity) { return authorization.evaluators() - .from(createPermissions(ticket, authorizationRequest, resourceServer, authorization), evaluationContext) + .from(createPermissions(ticket, authorizationRequest, resourceServer, identity, authorization), evaluationContext) .evaluate(); } private List evaluateUserManagedPermissions(AuthorizationRequest request, PermissionTicketToken ticket, ResourceServer resourceServer, KeycloakEvaluationContext evaluationContext, KeycloakIdentity identity) { return authorization.evaluators() - .from(createPermissions(ticket, request, resourceServer, authorization), evaluationContext) + .from(createPermissions(ticket, request, resourceServer, identity, authorization), evaluationContext) .evaluate(new PermissionTicketAwareDecisionResultCollector(request, ticket, identity, resourceServer, authorization)).results(); } @@ -276,7 +276,7 @@ public class AuthorizationTokenService { return evaluationContextProvider.apply(authorizationRequest, authorization); } - private List createPermissions(PermissionTicketToken ticket, AuthorizationRequest request, ResourceServer resourceServer, AuthorizationProvider authorization) { + private List createPermissions(PermissionTicketToken ticket, AuthorizationRequest request, ResourceServer resourceServer, KeycloakIdentity identity, AuthorizationProvider authorization) { StoreFactory storeFactory = authorization.getStoreFactory(); Map> permissionsToEvaluate = new LinkedHashMap<>(); ResourceStore resourceStore = storeFactory.getResourceStore(); @@ -294,17 +294,31 @@ public class AuthorizationTokenService { requestedScopes = new HashSet<>(); } - Resource existingResource = null; + List existingResources = new ArrayList<>(); if (requestedResource.getResourceId() != null) { - existingResource = resourceStore.findById(requestedResource.getResourceId(), resourceServer.getId()); + Resource resource = resourceStore.findById(requestedResource.getResourceId(), resourceServer.getId()); - if (existingResource == null) { - existingResource = resourceStore.findByName(requestedResource.getResourceId(), resourceServer.getId()); + if (resource != null) { + existingResources.add(resource); + } else { + Resource ownerResource = resourceStore.findByName(requestedResource.getResourceId(), identity.getId(), resourceServer.getId()); + + if (ownerResource != null) { + existingResources.add(ownerResource); + } + + if (!identity.isResourceServer()) { + Resource serverResource = resourceStore.findByName(requestedResource.getResourceId(), resourceServer.getId()); + + if (serverResource != null) { + existingResources.add(serverResource); + } + } } } - if (existingResource == null && (requestedScopes == null || requestedScopes.isEmpty())) { + if (existingResources.isEmpty() && (requestedScopes == null || requestedScopes.isEmpty())) { throw new CorsErrorResponseException(cors, "invalid_resource", "Resource with id [" + requestedResource.getResourceId() + "] does not exist.", Status.FORBIDDEN); } @@ -314,18 +328,20 @@ public class AuthorizationTokenService { requestedScopes.addAll(Arrays.asList(clientAdditionalScopes.split(" "))); } - if (existingResource != null) { - Set scopes = permissionsToEvaluate.get(existingResource.getId()); + if (!existingResources.isEmpty()) { + for (Resource resource : existingResources) { + Set scopes = permissionsToEvaluate.get(resource.getId()); - if (scopes == null) { - scopes = new HashSet<>(); - permissionsToEvaluate.put(existingResource.getId(), scopes); - if (limit != null) { - limit--; + if (scopes == null) { + scopes = new HashSet<>(); + permissionsToEvaluate.put(resource.getId(), scopes); + if (limit != null) { + limit--; + } } - } - scopes.addAll(requestedScopes); + scopes.addAll(requestedScopes); + } } else { List resources = resourceStore.findByScope(new ArrayList<>(requestedScopes), ticket.getAudience()[0]); diff --git a/services/src/main/java/org/keycloak/authorization/common/KeycloakIdentity.java b/services/src/main/java/org/keycloak/authorization/common/KeycloakIdentity.java index 654193d0b8..7322eb2270 100644 --- a/services/src/main/java/org/keycloak/authorization/common/KeycloakIdentity.java +++ b/services/src/main/java/org/keycloak/authorization/common/KeycloakIdentity.java @@ -224,7 +224,7 @@ public class KeycloakIdentity implements Identity { return this.accessToken; } - private boolean isResourceServer() { + public boolean isResourceServer() { UserModel clientUser = null; ClientModel clientModel = getTargetClient(); diff --git a/services/src/main/java/org/keycloak/authorization/protection/permission/AbstractPermissionService.java b/services/src/main/java/org/keycloak/authorization/protection/permission/AbstractPermissionService.java index 7a8e05e27c..7dd34963b5 100644 --- a/services/src/main/java/org/keycloak/authorization/protection/permission/AbstractPermissionService.java +++ b/services/src/main/java/org/keycloak/authorization/protection/permission/AbstractPermissionService.java @@ -67,41 +67,61 @@ public class AbstractPermissionService { private List verifyRequestedResource(List request) { ResourceStore resourceStore = authorization.getStoreFactory().getResourceStore(); + List requestedResources = new ArrayList<>(); - return request.stream().map(permissionRequest -> { + for (PermissionRequest permissionRequest : request) { String resourceSetId = permissionRequest.getResourceId(); - Resource resource = null; + List resources = new ArrayList<>(); if (resourceSetId == null) { if (permissionRequest.getScopes() == null || permissionRequest.getScopes().isEmpty()) { throw new ErrorResponseException("invalid_resource_id", "Resource id or name not provided.", Response.Status.BAD_REQUEST); } } else { - resource = resourceStore.findById(resourceSetId, resourceServer.getId()); + Resource resource = resourceStore.findById(resourceSetId, resourceServer.getId()); - if (resource == null) { - resource = resourceStore.findByName(resourceSetId, this.resourceServer.getId()); + if (resource != null) { + resources.add(resource); + } else { + Resource userResource = resourceStore.findByName(resourceSetId, identity.getId(), this.resourceServer.getId()); + + if (userResource != null) { + resources.add(userResource); + } + + if (!identity.isResourceServer()) { + Resource serverResource = resourceStore.findByName(resourceSetId, this.resourceServer.getId()); + + if (serverResource != null) { + resources.add(serverResource); + } + } } - if (resource == null) { + if (resources.isEmpty()) { throw new ErrorResponseException("invalid_resource_id", "Resource set with id [" + resourceSetId + "] does not exists in this server.", Response.Status.BAD_REQUEST); } } - Set scopes = verifyRequestedScopes(permissionRequest, resource); + if (resources.isEmpty()) { + requestedResources.add(new ResourceRepresentation(null, verifyRequestedScopes(permissionRequest, null))); - if (resource != null) { - ResourceRepresentation representation = new ResourceRepresentation(resource.getName(), scopes); + } else { + for (Resource resource : resources) { + Set scopes = verifyRequestedScopes(permissionRequest, resource); - representation.setId(resource.getId()); - representation.setOwnerManagedAccess(resource.isOwnerManagedAccess()); - representation.setOwner(new ResourceOwnerRepresentation(resource.getOwner())); + ResourceRepresentation representation = new ResourceRepresentation(resource.getName(), scopes); - return representation; + representation.setId(resource.getId()); + representation.setOwnerManagedAccess(resource.isOwnerManagedAccess()); + representation.setOwner(new ResourceOwnerRepresentation(resource.getOwner())); + + requestedResources.add(representation); + } } + } - return new ResourceRepresentation(null, scopes); - }).collect(Collectors.toList()); + return requestedResources; } private Set verifyRequestedScopes(PermissionRequest request, Resource resource) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AbstractAuthorizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AbstractAuthorizationTest.java index 51a2caebc9..8546c2a15d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AbstractAuthorizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AbstractAuthorizationTest.java @@ -18,22 +18,26 @@ package org.keycloak.testsuite.admin.client.authorization; import org.junit.After; -import org.junit.Before; import org.junit.BeforeClass; import org.keycloak.admin.client.resource.AuthorizationResource; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ResourceScopeResource; import org.keycloak.admin.client.resource.ResourceScopesResource; import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.authorization.ResourceServerRepresentation; import org.keycloak.representations.idm.authorization.ScopeRepresentation; import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.admin.client.AbstractClientTest; +import org.keycloak.testsuite.util.ClientBuilder; +import org.keycloak.testsuite.util.RealmBuilder; +import org.keycloak.testsuite.util.UserBuilder; import javax.ws.rs.core.Response; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; + +import java.util.List; /** * @author Pedro Igor @@ -42,24 +46,32 @@ public abstract class AbstractAuthorizationTest extends AbstractClientTest { protected static final String RESOURCE_SERVER_CLIENT_ID = "resource-server-test"; + @Override + public void setDefaultPageUriParameters() { + super.setDefaultPageUriParameters(); + testRealmPage.setAuthRealm("authz-test"); + } + + @Override + protected String getRealmId() { + return "authz-test"; + } + @BeforeClass public static void enabled() { ProfileAssume.assumePreview(); } - @Before - public void onBeforeAuthzTests() { - createOidcClient(RESOURCE_SERVER_CLIENT_ID); - - ClientRepresentation resourceServer = getResourceServer(); - - assertEquals(RESOURCE_SERVER_CLIENT_ID, resourceServer.getName()); - assertFalse(resourceServer.getAuthorizationServicesEnabled()); + @Override + public void addTestRealms(List testRealms) { + testRealms.add(createTestRealm().build()); + super.addTestRealms(testRealms); } @After - public void onAfterAuthzTests() { - getClientResource().remove(); + public void onAfterReenableAuthorization() { + enableAuthorizationServices(false); + enableAuthorizationServices(true); } protected ClientResource getClientResource() { @@ -70,22 +82,22 @@ public abstract class AbstractAuthorizationTest extends AbstractClientTest { return findClientRepresentation(RESOURCE_SERVER_CLIENT_ID); } - protected void enableAuthorizationServices() { + protected void enableAuthorizationServices(boolean enable) { ClientRepresentation resourceServer = getResourceServer(); - resourceServer.setAuthorizationServicesEnabled(true); + resourceServer.setAuthorizationServicesEnabled(enable); resourceServer.setServiceAccountsEnabled(true); resourceServer.setPublicClient(false); resourceServer.setSecret("secret"); getClientResource().update(resourceServer); - AuthorizationResource authorization = getClientResource().authorization(); - ResourceServerRepresentation settings = authorization.exportSettings(); - - settings.setAllowRemoteResourceManagement(true); - - authorization.update(settings); + if (enable) { + AuthorizationResource authorization = getClientResource().authorization(); + ResourceServerRepresentation settings = authorization.exportSettings(); + settings.setAllowRemoteResourceManagement(true); + authorization.update(settings); + } } protected ResourceScopeResource createDefaultScope() { @@ -108,4 +120,17 @@ public abstract class AbstractAuthorizationTest extends AbstractClientTest { return resources.scope(stored.getId()); } + + private RealmBuilder createTestRealm() { + return RealmBuilder.create().name("authz-test") + .user(UserBuilder.create().username("marta").password("password")) + .user(UserBuilder.create().username("kolo").password("password")) + .client(ClientBuilder.create().clientId(RESOURCE_SERVER_CLIENT_ID) + .name(RESOURCE_SERVER_CLIENT_ID) + .secret("secret") + .authorizationServicesEnabled(true) + .redirectUris("http://localhost/" + RESOURCE_SERVER_CLIENT_ID) + .defaultRoles("uma_protection") + .directAccessGrants()); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AuthorizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AuthorizationTest.java index 2d549de86b..f642979893 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AuthorizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/AuthorizationTest.java @@ -24,6 +24,7 @@ import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.adapters.config.PolicyEnforcerConfig; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.authorization.JSPolicyRepresentation; import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.representations.idm.authorization.ResourceServerRepresentation; @@ -44,17 +45,38 @@ public class AuthorizationTest extends AbstractAuthorizationTest { ClientResource clientResource = getClientResource(); ClientRepresentation resourceServer = getResourceServer(); - enableAuthorizationServices(); + enableAuthorizationServices(false); + enableAuthorizationServices(true); + + clientResource.authorization().resources().create(new ResourceRepresentation("Should be removed")); + + JSPolicyRepresentation policy = new JSPolicyRepresentation(); + + policy.setName("should be removed"); + policy.setCode(""); + + clientResource.authorization().policies().js().create(policy); + + List defaultResources = clientResource.authorization().resources().resources(); + + assertEquals(2, defaultResources.size()); + + List defaultPolicies = clientResource.authorization().policies().policies(); + + assertEquals(3, defaultPolicies.size()); + + enableAuthorizationServices(false); + enableAuthorizationServices(true); ResourceServerRepresentation settings = clientResource.authorization().getSettings(); assertEquals(PolicyEnforcerConfig.EnforcementMode.ENFORCING.name(), settings.getPolicyEnforcementMode().name()); assertEquals(resourceServer.getId(), settings.getClientId()); - List defaultResources = clientResource.authorization().resources().resources(); + defaultResources = clientResource.authorization().resources().resources(); assertEquals(1, defaultResources.size()); - List defaultPolicies = clientResource.authorization().policies().policies(); + defaultPolicies = clientResource.authorization().policies().policies(); assertEquals(2, defaultPolicies.size()); } @@ -72,8 +94,6 @@ public class AuthorizationTest extends AbstractAuthorizationTest { ClientResource clientResource = getClientResource(); ClientRepresentation resourceServer = getResourceServer(); - enableAuthorizationServices(); - ResourceServerRepresentation settings = clientResource.authorization().getSettings(); assertEquals(PolicyEnforcerConfig.EnforcementMode.ENFORCING.name(), settings.getPolicyEnforcementMode().name()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ExportAuthorizationSettingsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ExportAuthorizationSettingsTest.java index c71d12f53a..59d655aa46 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ExportAuthorizationSettingsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ExportAuthorizationSettingsTest.java @@ -46,8 +46,6 @@ public class ExportAuthorizationSettingsTest extends AbstractAuthorizationTest { String permissionName = "resource-based-permission"; ClientResource clientResource = getClientResource(); - - enableAuthorizationServices(); AuthorizationResource authorizationResource = clientResource.authorization(); //get Default Resource @@ -89,8 +87,6 @@ public class ExportAuthorizationSettingsTest extends AbstractAuthorizationTest { @Test public void testRoleBasedPolicy() { ClientResource clientResource = getClientResource(); - - enableAuthorizationServices(); AuthorizationResource authorizationResource = clientResource.authorization(); ClientRepresentation account = testRealmResource().clients().findByClientId("account").get(0); @@ -121,8 +117,6 @@ public class ExportAuthorizationSettingsTest extends AbstractAuthorizationTest { @Test public void testRoleBasedPolicyWithMultipleRoles() { ClientResource clientResource = getClientResource(); - - enableAuthorizationServices(); AuthorizationResource authorizationResource = clientResource.authorization(); testRealmResource().clients().create(ClientBuilder.create().clientId("test-client-1").defaultRoles("client-role").build()).close(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java index bd4d97351e..322492d5b8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/GenericPolicyManagementTest.java @@ -54,19 +54,6 @@ public class GenericPolicyManagementTest extends AbstractAuthorizationTest { private static final String[] EXPECTED_BUILTIN_POLICY_PROVIDERS = {"test", "user", "role", "rules", "js", "time", "aggregate", "scope", "resource"}; - @Before - @Override - public void onBeforeAuthzTests() { - super.onBeforeAuthzTests(); - enableAuthorizationServices(); - } - - @After - @Override - public void onAfterAuthzTests() { - super.onAfterAuthzTests(); - } - @Test public void testCreate() { PolicyRepresentation newPolicy = createTestingPolicy().toRepresentation(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ImportAuthorizationSettingsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ImportAuthorizationSettingsTest.java index a308ec0b33..0524111dc0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ImportAuthorizationSettingsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ImportAuthorizationSettingsTest.java @@ -48,24 +48,9 @@ public class ImportAuthorizationSettingsTest extends AbstractAuthorizationTest { testRealmResource().users().create(UserBuilder.create().username("alice").build()); } - @After - public void onAfterAuthzTests() { - ClientResource clientResource = getClientResource(); - - // Needed to disable authz first. TODO: Looks like a bug. Check later... - ClientRepresentation client = clientResource.toRepresentation(); - client.setAuthorizationServicesEnabled(false); - clientResource.update(client); - - getClientResource().remove(); - } - @Test public void testImportUnorderedSettings() throws Exception { ClientResource clientResource = getClientResource(); - - enableAuthorizationServices(); - ResourceServerRepresentation toImport = JsonSerialization.readValue(getClass().getResourceAsStream("/authorization-test/import-authorization-unordered-settings.json"), ResourceServerRepresentation.class); realmsResouce().realm(getRealmId()).roles().create(new RoleRepresentation("user", null, false)); @@ -75,6 +60,6 @@ public class ImportAuthorizationSettingsTest extends AbstractAuthorizationTest { authorizationResource.importSettings(toImport); - assertEquals(15, authorizationResource.policies().policies().size()); + assertEquals(13, authorizationResource.policies().policies().size()); } } \ No newline at end of file diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementTest.java index 3c1a2f189f..41fcb66eb3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementTest.java @@ -18,12 +18,11 @@ package org.keycloak.testsuite.admin.client.authorization; -import org.junit.Before; import org.junit.Test; import org.keycloak.admin.client.resource.ResourceResource; import org.keycloak.admin.client.resource.ResourcesResource; import org.keycloak.authorization.client.util.HttpResponseException; -import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.authorization.ResourceOwnerRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.representations.idm.authorization.ScopeRepresentation; @@ -35,6 +34,7 @@ import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -45,33 +45,6 @@ import static org.junit.Assert.fail; */ public class ResourceManagementTest extends AbstractAuthorizationTest { - @Before - @Override - public void onBeforeAuthzTests() { - super.onBeforeAuthzTests(); - enableAuthorizationServices(); - } - - @Override - public void addTestRealms(List testRealms) { - RealmRepresentation testRealmRep = new RealmRepresentation(); - testRealmRep.setId("authz-test"); - testRealmRep.setRealm("authz-test"); - testRealmRep.setEnabled(true); - testRealms.add(testRealmRep); - } - - @Override - public void setDefaultPageUriParameters() { - super.setDefaultPageUriParameters(); - testRealmPage.setAuthRealm("authz-test"); - } - - @Override - protected String getRealmId() { - return "authz-test"; - } - @Test public void testCreate() { ResourceRepresentation newResource = createResource(); @@ -102,6 +75,28 @@ public class ResourceManagementTest extends AbstractAuthorizationTest { assertEquals("Test Resource Another", newResource.getName()); } + @Test + public void failCreateWithSameNameDifferentOwner() { + ResourceRepresentation martaResource = createResource("Resource A", "marta", null, null, null); + ResourceRepresentation koloResource = createResource("Resource A", "kolo", null, null, null); + + assertNotNull(martaResource.getId()); + assertNotNull(koloResource.getId()); + assertNotEquals(martaResource.getId(), koloResource.getId()); + + assertEquals(2, getClientResource().authorization().resources().findByName(martaResource.getName()).size()); + + List martaResources = getClientResource().authorization().resources().findByName(martaResource.getName(), "marta"); + + assertEquals(1, martaResources.size()); + assertEquals(martaResource.getId(), martaResources.get(0).getId()); + + List koloResources = getClientResource().authorization().resources().findByName(martaResource.getName(), "kolo"); + + assertEquals(1, koloResources.size()); + assertEquals(koloResource.getId(), koloResources.get(0).getId()); + } + @Test public void testUpdate() { ResourceRepresentation resource = createResource(); @@ -198,12 +193,17 @@ public class ResourceManagementTest extends AbstractAuthorizationTest { } private ResourceRepresentation createResource() { + return createResource("Test Resource", null, "/test/*", "test-resource", "icon-test-resource"); + } + + private ResourceRepresentation createResource(String name, String owner, String uri, String type, String iconUri) { ResourceRepresentation newResource = new ResourceRepresentation(); - newResource.setName("Test Resource"); - newResource.setUri("/test/*"); - newResource.setType("test-resource"); - newResource.setIconUri("icon-test-resource"); + newResource.setName(name); + newResource.setUri(uri); + newResource.setType(type); + newResource.setIconUri(iconUri); + newResource.setOwner(owner != null ? new ResourceOwnerRepresentation(owner) : null); return doCreateResource(newResource); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopeManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopeManagementTest.java index bcb8b1b636..9590078048 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopeManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ScopeManagementTest.java @@ -33,13 +33,6 @@ import static org.junit.Assert.assertEquals; */ public class ScopeManagementTest extends AbstractAuthorizationTest { - @Before - @Override - public void onBeforeAuthzTests() { - super.onBeforeAuthzTests(); - enableAuthorizationServices(); - } - @Test public void testCreate() { ScopeRepresentation newScope = createDefaultScope().toRepresentation(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationAPITest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationAPITest.java index 61d27735c9..fb6fab12de 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationAPITest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationAPITest.java @@ -127,7 +127,7 @@ public class AuthorizationAPITest extends AbstractAuthzTest { assertEquals("resource-server-test", rpt.getAudience()[0]); } - private RealmResource getRealm() throws Exception { + private RealmResource getRealm() { return adminClient.realm("authz-test"); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationTest.java new file mode 100644 index 0000000000..16eb0cbf7e --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AuthorizationTest.java @@ -0,0 +1,229 @@ +/* + * Copyright 2018 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.testsuite.authz; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.UUID; + +import javax.ws.rs.core.Response; + +import org.jetbrains.annotations.NotNull; +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.jose.jws.JWSInputException; +import org.keycloak.representations.AccessToken; +import org.keycloak.representations.AccessToken.Authorization; +import org.keycloak.representations.idm.RealmRepresentation; +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.ResourceOwnerRepresentation; +import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation; +import org.keycloak.representations.idm.authorization.ResourceRepresentation; +import org.keycloak.testsuite.util.ClientBuilder; +import org.keycloak.testsuite.util.RealmBuilder; +import org.keycloak.testsuite.util.RoleBuilder; +import org.keycloak.testsuite.util.RolesBuilder; +import org.keycloak.testsuite.util.UserBuilder; +import org.keycloak.util.JsonSerialization; + +/** + * @author Pedro Igor + */ +public class AuthorizationTest extends AbstractAuthzTest { + + private AuthzClient authzClient; + + @Override + public void addTestRealms(List testRealms) { + testRealms.add(RealmBuilder.create().name("authz-test") + .roles(RolesBuilder.create().realmRole(RoleBuilder.create().name("uma_authorization").build())) + .user(UserBuilder.create().username("marta").password("password").addRoles("uma_authorization")) + .user(UserBuilder.create().username("kolo").password("password")) + .client(ClientBuilder.create().clientId("resource-server-test") + .secret("secret") + .authorizationServicesEnabled(true) + .redirectUris("http://localhost/resource-server-test") + .defaultRoles("uma_protection") + .directAccessGrants()) + .client(ClientBuilder.create().clientId("test-client") + .secret("secret") + .authorizationServicesEnabled(true) + .redirectUris("http://localhost/test-client") + .directAccessGrants()) + .build()); + } + + @Before + public void configureAuthorization() throws Exception { + ClientResource client = getClient(); + AuthorizationResource authorization = client.authorization(); + + JSPolicyRepresentation policy = new JSPolicyRepresentation(); + + policy.setName("Grant Policy"); + policy.setCode("$evaluation.grant();"); + + authorization.policies().js().create(policy).close(); + + policy = new JSPolicyRepresentation(); + + policy.setName("Deny Policy"); + policy.setCode("$evaluation.deny();"); + } + + @After + public void onAfter() { + ResourcesResource resources = getClient().authorization().resources(); + List existingResources = resources.resources(); + + for (ResourceRepresentation resource : existingResources) { + resources.resource(resource.getId()).remove(); + } + } + + @Test + public void testResourceWithSameNameDifferentOwner() throws JWSInputException { + ResourceRepresentation koloResource = createResource("Resource A", "kolo", "Scope A", "Scope B"); + + createResourcePermission(koloResource, "Grant Policy"); + + ResourceRepresentation martaResource = createResource("Resource A", "marta", "Scope A", "Scope B"); + + createResourcePermission(martaResource, "Grant Policy"); + + assertNotEquals(koloResource.getId(), martaResource.getId()); + + AuthorizationRequest request = new AuthorizationRequest(); + + request.addPermission("Resource A"); + + List permissions = authorize("kolo", "password", request); + + assertEquals(1, permissions.size()); + + Permission permission = permissions.get(0); + assertTrue(permission.getScopes().containsAll(Arrays.asList("Scope A", "Scope B"))); + + assertEquals(koloResource.getId(), permission.getResourceId()); + + permissions = authorize("marta", "password", request); + + assertEquals(1, permissions.size()); + + permission = permissions.get(0); + + assertEquals(martaResource.getId(), permission.getResourceId()); + assertTrue(permission.getScopes().containsAll(Arrays.asList("Scope A", "Scope B"))); + } + + @Test + public void testResourceServerWithSameNameDifferentOwner() { + ResourceRepresentation koloResource = createResource("Resource A", "kolo", "Scope A", "Scope B"); + + createResourcePermission(koloResource, "Grant Policy"); + + ResourceRepresentation serverResource = createResource("Resource A", null, "Scope A", "Scope B"); + + createResourcePermission(serverResource, "Grant Policy"); + + AuthorizationRequest request = new AuthorizationRequest(); + + request.addPermission("Resource A"); + + List permissions = authorize("kolo", "password", request); + + assertEquals(2, permissions.size()); + + for (Permission permission : permissions) { + assertTrue(permission.getResourceId().equals(koloResource.getId()) || permission.getResourceId().equals(serverResource.getId())); + assertEquals("Resource A", permission.getResourceName()); + } + } + + private List authorize(String userName, String password, AuthorizationRequest request) { + AuthorizationResponse response = getAuthzClient().authorization(userName, password).authorize(request); + AccessToken token = toAccessToken(response.getToken()); + Authorization authorization = token.getAuthorization(); + return authorization.getPermissions(); + } + + private void createResourcePermission(ResourceRepresentation resource, String... policies) { + ResourcePermissionRepresentation permission = new ResourcePermissionRepresentation(); + + permission.setName(resource.getName() + UUID.randomUUID().toString()); + permission.addResource(resource.getId()); + permission.addPolicy(policies); + + Response response = getClient().authorization().permissions().resource().create(permission); + + assertEquals(201, response.getStatus()); + } + + @NotNull + private ResourceRepresentation createResource(String name, String owner, String... scopes) { + ResourceRepresentation resource = new ResourceRepresentation(); + + resource.setName(name); + resource.setOwner(owner != null ? new ResourceOwnerRepresentation(owner) : null); + resource.addScope(scopes); + + Response response = getClient().authorization().resources().create(resource); + ResourceRepresentation stored = response.readEntity(ResourceRepresentation.class); + response.close(); + + resource.setId(stored.getId()); + + return resource; + } + + private RealmResource getRealm() { + return adminClient.realm("authz-test"); + } + + private ClientResource getClient() { + ClientsResource clients = getRealm().clients(); + return clients.findByClientId("resource-server-test").stream().map(representation -> clients.get(representation.getId())).findFirst().orElseThrow(() -> new RuntimeException("Expected client [resource-server-test]")); + } + + private AuthzClient getAuthzClient() { + if (authzClient == null) { + try { + authzClient = AuthzClient.create(JsonSerialization.readValue(getClass().getResourceAsStream("/authorization-test/default-keycloak.json"), Configuration.class)); + } catch (IOException cause) { + throw new RuntimeException("Failed to create authz client", cause); + } + } + + return authzClient; + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java index 21bb9ee33f..f488d32897 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java @@ -52,7 +52,7 @@ public class PermissionManagementTest extends AbstractResourceServerTest { public void testCreatePermissionTicketWithResourceName() throws Exception { ResourceRepresentation resource = addResource("Resource A", "kolo", true); AuthzClient authzClient = getAuthzClient(); - PermissionResponse response = authzClient.protection("marta", "password").permission().create(new PermissionRequest(resource.getName())); + PermissionResponse response = authzClient.protection("marta", "password").permission().create(new PermissionRequest(resource.getId())); AuthorizationRequest request = new AuthorizationRequest(); request.setTicket(response.getTicket()); request.setClaimToken(authzClient.obtainAccessToken("marta", "password").getToken()); @@ -125,7 +125,7 @@ public class PermissionManagementTest extends AbstractResourceServerTest { @Test public void testDeleteScopeAndPermissionTicket() throws Exception { ResourceRepresentation resource = addResource("Resource A", "kolo", true, "ScopeA", "ScopeB", "ScopeC"); - PermissionRequest permissionRequest = new PermissionRequest(resource.getName()); + PermissionRequest permissionRequest = new PermissionRequest(resource.getId()); permissionRequest.setScopes(new HashSet<>(Arrays.asList("ScopeA", "ScopeB", "ScopeC"))); @@ -164,7 +164,7 @@ public class PermissionManagementTest extends AbstractResourceServerTest { @Test public void testRemoveScopeFromResource() throws Exception { ResourceRepresentation resource = addResource("Resource A", "kolo", true, "ScopeA", "ScopeB"); - PermissionRequest permissionRequest = new PermissionRequest(resource.getName(), "ScopeA", "ScopeB"); + PermissionRequest permissionRequest = new PermissionRequest(resource.getId(), "ScopeA", "ScopeB"); AuthzClient authzClient = getAuthzClient(); PermissionResponse response = authzClient.protection("marta", "password").permission().create(permissionRequest); @@ -255,7 +255,7 @@ public class PermissionManagementTest extends AbstractResourceServerTest { getClient(getRealm()).authorization().resources().resource(resourceA.getId()).update(resourceA); - PermissionRequest permissionRequest = new PermissionRequest("Resource A"); + PermissionRequest permissionRequest = new PermissionRequest(resourceA.getId()); permissionRequest.setScopes(new HashSet<>(Arrays.asList("ScopeA", "ScopeC"))); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaGrantTypeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaGrantTypeTest.java index 41448cf175..5d4c1a12c1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaGrantTypeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaGrantTypeTest.java @@ -123,7 +123,7 @@ public class UmaGrantTypeTest extends AbstractResourceServerTest { ResourceRepresentation resourceA = addResource("Resource Marta", "marta", true, "ScopeA", "ScopeB", "ScopeC"); permission.setName(resourceA.getName() + " Permission"); - permission.addResource(resourceA.getName()); + permission.addResource(resourceA.getId()); permission.addPolicy("Default Policy"); getClient(getRealm()).authorization().permissions().resource().create(permission).close(); @@ -131,7 +131,7 @@ public class UmaGrantTypeTest extends AbstractResourceServerTest { ResourceRepresentation resourceB = addResource("Resource B", "marta", "ScopeA", "ScopeB", "ScopeC"); permission.setName(resourceB.getName() + " Permission"); - permission.addResource(resourceB.getName()); + permission.addResource(resourceB.getId()); permission.addPolicy("Default Policy"); getClient(getRealm()).authorization().permissions().resource().create(permission).close(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java index f8dd3def7c..5ef774efe8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java @@ -68,7 +68,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { resource = addResource("Resource A", "marta", true, "ScopeA", "ScopeB"); permission.setName(resource.getName() + " Permission"); - permission.addResource(resource.getName()); + permission.addResource(resource.getId()); permission.addPolicy("Only Owner Policy"); getClient(getRealm()).authorization().permissions().resource().create(permission).close(); @@ -91,7 +91,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertTrue(permissions.isEmpty()); try { - response = authorize("kolo", "password", resource.getName(), new String[] {"ScopeA", "ScopeB"}); + response = authorize("kolo", "password", resource.getId(), new String[] {"ScopeA", "ScopeB"}); fail("User should have access to resource from another user"); } catch (AuthorizationDeniedException ade) { @@ -104,7 +104,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { resource = addResource("Resource A", "marta", true, "ScopeA", "ScopeB"); permission.setName(resource.getName() + " Permission"); - permission.addResource(resource.getName()); + permission.addResource(resource.getId()); permission.addPolicy("Only Owner Policy"); getClient(getRealm()).authorization().permissions().resource().create(permission).close(); @@ -127,7 +127,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertTrue(permissions.isEmpty()); try { - response = authorize("kolo", "password", "Resource A", new String[] {}); + response = authorize("kolo", "password", resource.getId(), new String[] {}); fail("User should have access to resource from another user"); } catch (AuthorizationDeniedException ade) { @@ -156,7 +156,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertTrue(ticket.isGranted()); } - response = authorize("kolo", "password", resource.getName(), new String[] {"ScopeA", "ScopeB"}); + response = authorize("kolo", "password", resource.getId(), new String[] {"ScopeA", "ScopeB"}); rpt = response.getToken(); assertNotNull(rpt); @@ -180,7 +180,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { resource = addResource("Resource A", "marta", true); permission.setName(resource.getName() + " Permission"); - permission.addResource(resource.getName()); + permission.addResource(resource.getId()); permission.addPolicy("Only Owner Policy"); getClient(getRealm()).authorization().permissions().resource().create(permission).close(); @@ -203,7 +203,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertTrue(permissions.isEmpty()); try { - response = authorize("kolo", "password", "Resource A", new String[] {}); + response = authorize("kolo", "password", resource.getId(), new String[] {}); fail("User should have access to resource from another user"); } catch (AuthorizationDeniedException ade) { @@ -232,7 +232,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertTrue(ticket.isGranted()); } - response = authorize("kolo", "password", resource.getName(), new String[] {}); + response = authorize("kolo", "password", resource.getId(), new String[] {}); rpt = response.getToken(); assertNotNull(rpt); @@ -249,7 +249,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertPermissions(permissions, resource.getName()); assertTrue(permissions.isEmpty()); - response = authorize("kolo", "password", resource.getName(), new String[] {}); + response = authorize("kolo", "password", resource.getId(), new String[] {}); rpt = response.getToken(); assertNotNull(rpt); @@ -282,7 +282,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { resource = addResource("Resource A", "marta", true, "ScopeA", "ScopeB"); permission.setName(resource.getName() + " Permission"); - permission.addResource(resource.getName()); + permission.addResource(resource.getId()); permission.addPolicy("Only Owner Policy"); getClient(getRealm()).authorization().permissions().resource().create(permission).close(); @@ -305,7 +305,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertTrue(permissions.isEmpty()); try { - response = authorize("kolo", "password", "Resource A", new String[] {"ScopeA"}); + response = authorize("kolo", "password", resource.getId(), new String[] {"ScopeA"}); fail("User should have access to resource from another user"); } catch (AuthorizationDeniedException ade) { @@ -324,7 +324,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { permissionResource.update(ticket); - response = authorize("kolo", "password", resource.getName(), new String[] {"ScopeA", "ScopeB"}); + response = authorize("kolo", "password", resource.getId(), new String[] {"ScopeA", "ScopeB"}); rpt = response.getToken(); assertNotNull(rpt);