From 0b95cdacb8733f0b01f61336e1f39b570f9085c3 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Mon, 13 Aug 2018 00:23:44 -0300 Subject: [PATCH] [KEYCLOAK-7885] Add user policy support to the policy API --- .../permission/UMAPolicyProviderFactory.java | 64 +++++++++++++++++ .../UmaPermissionRepresentation.java | 40 ++++++----- .../UserManagedPermissionServiceTest.java | 72 +++++++++++++++++-- 3 files changed, 152 insertions(+), 24 deletions(-) diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProviderFactory.java index 9b0ddec1aa..4fe6e890aa 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProviderFactory.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProviderFactory.java @@ -45,6 +45,7 @@ import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.representations.idm.authorization.RolePolicyRepresentation; import org.keycloak.representations.idm.authorization.RolePolicyRepresentation.RoleDefinition; import org.keycloak.representations.idm.authorization.UmaPermissionRepresentation; +import org.keycloak.representations.idm.authorization.UserPolicyRepresentation; /** * @author Pedro Igor @@ -106,6 +107,14 @@ public class UMAPolicyProviderFactory implements PolicyProviderFactory users = representation.getUsers(); + + if (users != null) { + for (String user : users) { + createUserPolicy(policy, policyStore, user, representation.getOwner()); + } + } + String condition = representation.getCondition(); if (condition != null) { @@ -184,6 +193,24 @@ public class UMAPolicyProviderFactory implements PolicyProviderFactory()); + + Set updatedUsers = representation.getUsers(); + + if (updatedUsers != null) { + for (String user : updatedUsers) { + rep.addUser(user); + } + } + + if (rep.getUsers().isEmpty()) { + policyStore.delete(associatedPolicy.getId()); + } else { + RepresentationToModel.toModel(rep, authorization, associatedPolicy); + } } } @@ -241,6 +268,24 @@ public class UMAPolicyProviderFactory implements PolicyProviderFactory updatedUsers = representation.getUsers(); + + if (updatedUsers != null) { + boolean createPolicy = true; + + for (Policy associatedPolicy : associatedPolicies) { + if ("user".equals(associatedPolicy.getType())) { + createPolicy = false; + } + } + + if (createPolicy) { + for (String user : updatedUsers) { + createUserPolicy(policy, policyStore, user, policy.getOwner()); + } + } + } + String condition = representation.getCondition(); if (condition != null) { @@ -300,6 +345,12 @@ public class UMAPolicyProviderFactory implements PolicyProviderFactory roles; private Set groups; private Set clients; + private Set users; private String condition; @Override @@ -38,22 +37,6 @@ public class UmaPermissionRepresentation extends AbstractPolicyRepresentation { return "uma"; } - public void setId(String id){ - this.id = id; - } - - public String getId(){ - return id; - } - - public String getDescription() { - return description; - } - - public void setDescription(String description) { - this.description = description; - } - public void setRoles(Set roles) { this.roles = roles; } @@ -124,6 +107,27 @@ public class UmaPermissionRepresentation extends AbstractPolicyRepresentation { return clients; } + public void setUsers(Set users) { + this.users = users; + } + + public void addUser(String... user) { + if (this.users == null) { + this.users = new HashSet<>(); + } + this.users.addAll(Arrays.asList(user)); + } + + public void removeUser(String user) { + if (this.users != null) { + this.users.remove(user); + } + } + + public Set getUsers() { + return this.users; + } + public void setCondition(String condition) { this.condition = condition; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java index 4abe7597c8..1c886b09d2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java @@ -111,6 +111,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest newPermission.addGroup("/group_a", "/group_a/group_b", "/group_c"); newPermission.addClient("client-a", "resource-server-test"); newPermission.setCondition("$evaluation.grant()"); + newPermission.addUser("kolo"); ProtectionResource protection = getAuthzClient().protection("marta", "password"); @@ -118,11 +119,17 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest assertEquals(newPermission.getName(), permission.getName()); assertEquals(newPermission.getDescription(), permission.getDescription()); + assertNotNull(permission.getScopes()); assertTrue(permission.getScopes().containsAll(newPermission.getScopes())); + assertNotNull(permission.getRoles()); assertTrue(permission.getRoles().containsAll(newPermission.getRoles())); + assertNotNull(permission.getGroups()); assertTrue(permission.getGroups().containsAll(newPermission.getGroups())); + assertNotNull(permission.getClients()); assertTrue(permission.getClients().containsAll(newPermission.getClients())); assertEquals(newPermission.getCondition(), permission.getCondition()); + assertNotNull(permission.getUsers()); + assertTrue(permission.getUsers().containsAll(newPermission.getUsers())); } @Test @@ -233,6 +240,38 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest assertEquals(permission.getCondition(), updated.getCondition()); + permission.addUser("alice"); + + protection.policy(resource.getId()).update(permission); + assertEquals(5, getAssociatedPolicies(permission).size()); + updated = protection.policy(resource.getId()).findById(permission.getId()); + assertEquals(1, updated.getUsers().size()); + assertEquals(permission.getUsers(), updated.getUsers()); + + permission.addUser("kolo"); + + protection.policy(resource.getId()).update(permission); + assertEquals(5, getAssociatedPolicies(permission).size()); + updated = protection.policy(resource.getId()).findById(permission.getId()); + assertEquals(2, updated.getUsers().size()); + assertEquals(permission.getUsers(), updated.getUsers()); + + permission.removeUser("alice"); + + protection.policy(resource.getId()).update(permission); + assertEquals(5, getAssociatedPolicies(permission).size()); + updated = protection.policy(resource.getId()).findById(permission.getId()); + assertEquals(1, updated.getUsers().size()); + assertEquals(permission.getUsers(), updated.getUsers()); + + permission.setUsers(null); + + protection.policy(resource.getId()).update(permission); + assertEquals(4, getAssociatedPolicies(permission).size()); + updated = protection.policy(resource.getId()).findById(permission.getId()); + + assertEquals(permission.getUsers(), updated.getUsers()); + permission.setCondition(null); protection.policy(resource.getId()).update(permission); @@ -308,14 +347,14 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest protection.policy(resource.getId()).update(permission); try { - authzResponse = authorization.authorize(request); + authorization.authorize(request); fail("User should not have permission"); } catch (Exception e) { assertTrue(AuthorizationDeniedException.class.isInstance(e)); } try { - authzResponse = getAuthzClient().authorization("alice", "password").authorize(request); + getAuthzClient().authorization("alice", "password").authorize(request); fail("User should not have permission"); } catch (Exception e) { assertTrue(AuthorizationDeniedException.class.isInstance(e)); @@ -332,7 +371,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest protection.policy(resource.getId()).delete(permission.getId()); try { - authzResponse = authorization.authorize(request); + authorization.authorize(request); fail("User should not have permission"); } catch (Exception e) { assertTrue(AuthorizationDeniedException.class.isInstance(e)); @@ -344,6 +383,27 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest } catch (Exception e) { assertEquals(404, HttpResponseException.class.cast(e.getCause()).getStatusCode()); } + + // create a user based permission, where only selected users are allowed access to the resource. + permission = new UmaPermissionRepresentation(); + permission.setName("Custom User-Managed Permission"); + permission.setDescription("Specific users are allowed access to the resource"); + permission.addScope("Scope A"); + permission.addUser("alice"); + protection.policy(resource.getId()).create(permission); + + // alice should be able to access the resource with the updated permission. + authzResponse = getAuthzClient().authorization("alice", "password").authorize(request); + assertNotNull(authzResponse); + + // kolo shouldn't be able to access the resource with the updated permission. + try { + authorization.authorize(request); + fail("User should not have permission to access the protected resource"); + } catch(Exception e) { + assertTrue(AuthorizationDeniedException.class.isInstance(e)); + } + } @Test @@ -395,13 +455,13 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest permission = protection.policy(resource.getId()).create(permission); - authzResponse = getAuthzClient().authorization("kolo", "password").authorize(request); + getAuthzClient().authorization("kolo", "password").authorize(request); ticket.setGranted(false); getAuthzClient().protection().permission().update(ticket); - authzResponse = getAuthzClient().authorization("kolo", "password").authorize(request); + getAuthzClient().authorization("kolo", "password").authorize(request); permission = getAuthzClient().protection("marta", "password").policy(resource.getId()).findById(permission.getId()); @@ -495,7 +555,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest getAuthzClient().protection("alice", "password").policy(resource.getId()).create(new UmaPermissionRepresentation()); fail("Error expected"); } catch (Exception e) { - assertTrue(HttpResponseException.class.cast(e.getCause()).toString().contains("Only resource onwer can access policies for resource")); + assertTrue(HttpResponseException.class.cast(e.getCause()).toString().contains("Only resource owner can access policies for resource")); } }