From 8e57cee30ff97698ae2aaead273e2c48aa4b0a48 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 28 Sep 2018 14:17:58 -0300 Subject: [PATCH] [KEYCLOAK-8445] - Owner not granted with permissions when using only scope-based permissions --- .../permission/UMAPolicyProvider.java | 21 +++++ .../authz/UserManagedAccessTest.java | 86 +++++++++++++++++-- 2 files changed, 99 insertions(+), 8 deletions(-) diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProvider.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProvider.java index 50f6993637..a1a60c79da 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProvider.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProvider.java @@ -16,9 +16,30 @@ */ package org.keycloak.authorization.policy.provider.permission; +import org.keycloak.authorization.identity.Identity; +import org.keycloak.authorization.model.Resource; +import org.keycloak.authorization.permission.ResourcePermission; +import org.keycloak.authorization.policy.evaluation.Evaluation; + /** * @author Pedro Igor */ public class UMAPolicyProvider extends AbstractPermissionProvider { + @Override + public void evaluate(Evaluation evaluation) { + ResourcePermission permission = evaluation.getPermission(); + Resource resource = permission.getResource(); + + if (resource != null) { + Identity identity = evaluation.getContext().getIdentity(); + + // no need to evaluate UMA permissions to resource owner resources + if (resource.getOwner().equals(identity.getId())) { + return; + } + } + + super.evaluate(evaluation); + } } 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 2d7a69a2c7..1b6b9a35b7 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 @@ -43,6 +43,7 @@ import org.keycloak.representations.idm.authorization.PolicyEnforcementMode; import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.representations.idm.authorization.ResourceServerRepresentation; +import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; /** * @author Pedro Igor @@ -101,6 +102,82 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { } } + @Test + public void testOnlyOwnerCanAccessPermissionsToScope() throws Exception { + resource = addResource("Resource A", "marta", true, "ScopeA", "ScopeB"); + ScopePermissionRepresentation permission = new ScopePermissionRepresentation(); + + permission.setName(resource.getName() + " Scope A Permission"); + permission.addScope("ScopeA"); + permission.addPolicy("Only Owner Policy"); + + getClient(getRealm()).authorization().permissions().scope().create(permission).close(); + + permission = new ScopePermissionRepresentation(); + + permission.setName(resource.getName() + " Scope B Permission"); + permission.addScope("ScopeB"); + permission.addPolicy("Only Owner Policy"); + + getClient(getRealm()).authorization().permissions().scope().create(permission).close(); + + AuthorizationResponse response = authorize("marta", "password", resource.getName(), new String[] {"ScopeA", "ScopeB"}); + String rpt = response.getToken(); + + assertNotNull(rpt); + assertFalse(response.isUpgraded()); + + AccessToken accessToken = toAccessToken(rpt); + AccessToken.Authorization authorization = accessToken.getAuthorization(); + + assertNotNull(authorization); + + Collection permissions = authorization.getPermissions(); + + assertNotNull(permissions); + assertPermissions(permissions, resource.getName(), "ScopeA", "ScopeB"); + assertTrue(permissions.isEmpty()); + + try { + response = authorize("kolo", "password", resource.getId(), new String[] {"ScopeA", "ScopeB"}); + fail("User should not have access to resource from another user"); + } catch (AuthorizationDeniedException ade) { + } + + List tickets = getAuthzClient().protection().permission().find(resource.getId(), null, null, null, null, null, null, null); + + for (PermissionTicketRepresentation ticket : tickets) { + ticket.setGranted(true); + getAuthzClient().protection().permission().update(ticket); + } + + try { + response = authorize("kolo", "password", resource.getId(), new String[] {"ScopeA", "ScopeB"}); + } catch (AuthorizationDeniedException ade) { + fail("User should have access to resource from another user"); + } + + rpt = response.getToken(); + accessToken = toAccessToken(rpt); + authorization = accessToken.getAuthorization(); + permissions = authorization.getPermissions(); + assertPermissions(permissions, resource.getName(), "ScopeA", "ScopeB"); + assertTrue(permissions.isEmpty()); + + try { + response = authorize("marta", "password", resource.getId(), new String[] {"ScopeB"}); + } catch (AuthorizationDeniedException ade) { + fail("User should have access to his own resources"); + } + + rpt = response.getToken(); + accessToken = toAccessToken(rpt); + authorization = accessToken.getAuthorization(); + permissions = authorization.getPermissions(); + assertPermissions(permissions, resource.getName(), "ScopeB"); + assertTrue(permissions.isEmpty()); + } + /** * Makes sure permissions granted to a typed resource instance does not grant access to resource instances with the same type. * @@ -176,13 +253,6 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertNotNull(permissions); assertPermissions(permissions, resource.getName(), "ScopeA", "ScopeB"); assertTrue(permissions.isEmpty()); - - try { - response = authorize("kolo", "password", resourceB.getId(), new String[] {"ScopeA", "ScopeB"}); - fail("User should not have access to resource from another user"); - } catch (AuthorizationDeniedException ade) { - - } } @Test @@ -364,7 +434,7 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { } @Test - public void testUserGrantsAccessToScope() throws Exception { + public void testScopePermissionsToScopeOnly() throws Exception { ResourcePermissionRepresentation permission = new ResourcePermissionRepresentation(); resource = addResource("Resource A", "marta", true, "ScopeA", "ScopeB");