From de9a0a0a4ac302f2272f431012c00fc289201272 Mon Sep 17 00:00:00 2001 From: Eric Rodrigues Pires Date: Wed, 19 Feb 2020 14:37:03 -0300 Subject: [PATCH] [KEYCLOAK-13044] Fix owner name representations of UMA tickets for client-owned resources --- .../models/utils/ModelToRepresentation.java | 9 +- .../PolicyEvaluationResponseBuilder.java | 15 +- .../freemarker/model/AuthorizationBean.java | 32 ++- .../authz/UmaRepresentationTest.java | 182 ++++++++++++++++++ .../theme/base/account/resources.ftl | 4 +- 5 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaRepresentationTest.java diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 824f86bf9e..5591216098 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -1018,10 +1018,15 @@ public class ModelToRepresentation { representation.setResourceName(resource.getName()); KeycloakSession keycloakSession = authorization.getKeycloakSession(); RealmModel realm = authorization.getRealm(); - UserModel owner = keycloakSession.users().getUserById(ticket.getOwner(), realm); + UserModel userOwner = keycloakSession.users().getUserById(ticket.getOwner(), realm); UserModel requester = keycloakSession.users().getUserById(ticket.getRequester(), realm); representation.setRequesterName(requester.getUsername()); - representation.setOwnerName(owner.getUsername()); + if (userOwner != null) { + representation.setOwnerName(userOwner.getUsername()); + } else { + ClientModel clientOwner = realm.getClientById(ticket.getOwner()); + representation.setOwnerName(clientOwner.getClientId()); + } } Scope scope = ticket.getScope(); diff --git a/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java b/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java index e4bf9f8671..c6fe22b952 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java +++ b/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java @@ -27,6 +27,7 @@ import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.policy.evaluation.Result; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.authorization.DecisionEffect; @@ -196,11 +197,19 @@ public class PolicyEvaluationResponseBuilder { if (!tickets.isEmpty()) { KeycloakSession keycloakSession = authorization.getKeycloakSession(); + RealmModel realm = authorization.getRealm(); PermissionTicket ticket = tickets.get(0); - UserModel owner = keycloakSession.users().getUserById(ticket.getOwner(), authorization.getRealm()); - UserModel requester = keycloakSession.users().getUserById(ticket.getRequester(), authorization.getRealm()); + UserModel userOwner = keycloakSession.users().getUserById(ticket.getOwner(), realm); + UserModel requester = keycloakSession.users().getUserById(ticket.getRequester(), realm); + String resourceOwner; + if (userOwner != null) { + resourceOwner = getUserEmailOrUserName(userOwner); + } else { + ClientModel clientOwner = realm.getClientById(ticket.getOwner()); + resourceOwner = clientOwner.getClientId(); + } - representation.setDescription("Resource owner (" + getUserEmailOrUserName(owner) + ") grants access to " + getUserEmailOrUserName(requester)); + representation.setDescription("Resource owner (" + resourceOwner + ") grants access to " + getUserEmailOrUserName(requester)); } else { String description = representation.getDescription(); diff --git a/services/src/main/java/org/keycloak/forms/account/freemarker/model/AuthorizationBean.java b/services/src/main/java/org/keycloak/forms/account/freemarker/model/AuthorizationBean.java index f681cd3fd2..2b9e6a5765 100755 --- a/services/src/main/java/org/keycloak/forms/account/freemarker/model/AuthorizationBean.java +++ b/services/src/main/java/org/keycloak/forms/account/freemarker/model/AuthorizationBean.java @@ -225,7 +225,9 @@ public class AuthorizationBean { public class ResourceBean { private final ResourceServerBean resourceServer; - private final UserModel owner; + private final String ownerName; + private final UserModel userOwner; + private ClientModel clientOwner; private Resource resource; private Map permissions = new HashMap<>(); private Collection shares; @@ -234,7 +236,15 @@ public class AuthorizationBean { RealmModel realm = authorization.getRealm(); resourceServer = new ResourceServerBean(realm.getClientById(resource.getResourceServer().getId())); this.resource = resource; - owner = authorization.getKeycloakSession().users().getUserById(resource.getOwner(), realm); + userOwner = authorization.getKeycloakSession().users().getUserById(resource.getOwner(), realm); + if (userOwner == null) { + clientOwner = realm.getClientById(resource.getOwner()); + ownerName = clientOwner.getClientId(); + } else if (userOwner.getEmail() != null) { + ownerName = userOwner.getEmail(); + } else { + ownerName = userOwner.getUsername(); + } } public String getId() { @@ -253,8 +263,16 @@ public class AuthorizationBean { return resource.getIconUri(); } - public UserModel getOwner() { - return owner; + public String getOwnerName() { + return ownerName; + } + + public UserModel getUserOwner() { + return userOwner; + } + + public ClientModel getClientOwner() { + return clientOwner; } public List getScopes() { @@ -279,7 +297,11 @@ public class AuthorizationBean { filters.put("type", new String[] {"uma"}); filters.put("resource", new String[] {this.resource.getId()}); - filters.put("owner", new String[] {getOwner().getId()}); + if (getUserOwner() != null) { + filters.put("owner", new String[] {getUserOwner().getId()}); + } else { + filters.put("owner", new String[] {getClientOwner().getId()}); + } List policies = authorization.getStoreFactory().getPolicyStore().findByResourceServer(filters, getResourceServer().getId(), -1, -1); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaRepresentationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaRepresentationTest.java new file mode 100644 index 0000000000..dc050c721f --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UmaRepresentationTest.java @@ -0,0 +1,182 @@ +package org.keycloak.testsuite.authz; + +import org.jboss.resteasy.spi.ResteasyUriInfo; +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.authorization.AuthorizationProvider; +import org.keycloak.authorization.client.resource.PermissionResource; +import org.keycloak.forms.account.freemarker.model.AuthorizationBean; +import org.keycloak.forms.account.freemarker.model.AuthorizationBean.ResourceBean; +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.UserModel; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.representations.idm.authorization.*; +import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; +import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.List; + +@AuthServerContainerExclude(AuthServer.REMOTE) +public class UmaRepresentationTest extends AbstractResourceServerTest { + private ResourceRepresentation resource; + private PermissionResource permission; + + private void createPermissionTicket() { + PermissionTicketRepresentation ticket = new PermissionTicketRepresentation(); + ticket.setOwner(resource.getOwner().getId()); + ticket.setResource(resource.getId()); + ticket.setRequesterName("kolo"); + ticket.setScopeName("ScopeA"); + ticket.setGranted(true); + permission.create(ticket); + } + + @Test + public void testCanRepresentPermissionTicketWithNamesOfResourceOwnedByUser() throws Exception { + resource = addResource("Resource A", "marta", true, "ScopeA"); + permission = getAuthzClient().protection("marta", "password").permission(); + createPermissionTicket(); + + List permissionTickets = permission.find(resource.getId(), null, null, null, null, true, null, null); + Assert.assertFalse(permissionTickets.isEmpty()); + Assert.assertEquals(1, permissionTickets.size()); + + PermissionTicketRepresentation ticket = permissionTickets.get(0); + Assert.assertEquals(ticket.getOwnerName(), "marta"); + Assert.assertEquals(ticket.getRequesterName(), "kolo"); + Assert.assertEquals(ticket.getResourceName(), "Resource A"); + Assert.assertEquals(ticket.getScopeName(), "ScopeA"); + Assert.assertTrue(ticket.isGranted()); + } + + @Test + public void testCanRepresentPermissionTicketWithNamesOfResourceOwnedByClient() throws Exception { + resource = addResource("Resource A", getClient(getRealm()).toRepresentation().getId(), true, "ScopeA"); + permission = getAuthzClient().protection().permission(); + createPermissionTicket(); + + List permissionTickets = permission.find(resource.getId(), null, null, null, null, true, null, null); + Assert.assertFalse(permissionTickets.isEmpty()); + Assert.assertEquals(1, permissionTickets.size()); + + PermissionTicketRepresentation ticket = permissionTickets.get(0); + Assert.assertEquals(ticket.getOwnerName(), "resource-server-test"); + Assert.assertEquals(ticket.getRequesterName(), "kolo"); + Assert.assertEquals(ticket.getResourceName(), "Resource A"); + Assert.assertEquals(ticket.getScopeName(), "ScopeA"); + Assert.assertTrue(ticket.isGranted()); + } + + @Test + public void testCanRepresentPolicyResultGrantOfResourceOwnedByUser() throws Exception { + resource = addResource("Resource A", "marta", true, "ScopeA"); + permission = getAuthzClient().protection("marta", "password").permission(); + createPermissionTicket(); + + RealmResource realm = getRealm(); + String resourceServerId = getClient(realm).toRepresentation().getId(); + UserRepresentation user = realm.users().search("kolo").get(0); + + PolicyEvaluationRequest request = new PolicyEvaluationRequest(); + request.setUserId(user.getId()); + request.setClientId(resourceServerId); + request.addResource("Resource A", "ScopeA"); + PolicyEvaluationResponse result = getClient(realm).authorization().policies().evaluate(request); + Assert.assertEquals(result.getStatus(), DecisionEffect.PERMIT); + + List evaluations = result.getResults(); + Assert.assertFalse(evaluations.isEmpty()); + Assert.assertEquals(1, evaluations.size()); + + List policies = evaluations.get(0).getPolicies(); + Assert.assertFalse(evaluations.isEmpty()); + Assert.assertEquals(1, evaluations.size()); + + String description = policies.get(0).getPolicy().getDescription(); + Assert.assertTrue(description.startsWith("Resource owner (marta) grants access")); + } + + @Test + public void testCanRepresentPolicyResultGrantOfResourceOwnedByClient() throws Exception { + resource = addResource("Resource A", getClient(getRealm()).toRepresentation().getId(), true, "ScopeA"); + permission = getAuthzClient().protection().permission(); + createPermissionTicket(); + + RealmResource realm = getRealm(); + String resourceServerId = getClient(realm).toRepresentation().getId(); + UserRepresentation user = realm.users().search("kolo").get(0); + + PolicyEvaluationRequest request = new PolicyEvaluationRequest(); + request.setUserId(user.getId()); + request.setClientId(resourceServerId); + request.addResource("Resource A", "ScopeA"); + PolicyEvaluationResponse result = getClient(realm).authorization().policies().evaluate(request); + Assert.assertEquals(result.getStatus(), DecisionEffect.PERMIT); + + List evaluations = result.getResults(); + Assert.assertFalse(evaluations.isEmpty()); + Assert.assertEquals(1, evaluations.size()); + + List policies = evaluations.get(0).getPolicies(); + Assert.assertFalse(evaluations.isEmpty()); + Assert.assertEquals(1, evaluations.size()); + + String description = policies.get(0).getPolicy().getDescription(); + Assert.assertTrue(description.startsWith("Resource owner (resource-server-test) grants access")); + } + + @Test + public void testCanRepresentResourceBeanOfResourceOwnedByUser() throws Exception { + resource = addResource("Resource A", "marta", true, "ScopeA"); + testingClient.server().run(UmaRepresentationTest::testCanRepresentResourceBeanOfResourceOwnedByUser); + } + + public static void testCanRepresentResourceBeanOfResourceOwnedByUser(KeycloakSession session) { + session.getContext().setRealm(session.realms().getRealmByName("authz-test")); + AuthorizationProvider authorization = session.getProvider(AuthorizationProvider.class); + + AuthorizationBean authorizationBean = new AuthorizationBean(session, null, session.getContext().getUri()); + ClientModel client = session.getContext().getRealm().getClientByClientId("resource-server-test"); + UserModel user = session.userStorageManager().getUserByUsername("marta", session.getContext().getRealm()); + ResourceBean resourceBean = authorizationBean.new ResourceBean( + authorization.getStoreFactory().getResourceStore().findByName( + "Resource A", user.getId(), client.getId() + ) + ); + + Assert.assertEquals("Resource A", resourceBean.getName()); + Assert.assertEquals("marta", resourceBean.getOwnerName()); + Assert.assertNotNull(resourceBean.getUserOwner()); + Assert.assertEquals("marta", resourceBean.getUserOwner().getUsername()); + Assert.assertNull(resourceBean.getClientOwner()); + } + + @Test + public void testCanRepresentResourceBeanOfResourceOwnedByClient() throws Exception { + resource = addResource("Resource A", getClient(getRealm()).toRepresentation().getId(), true, "ScopeA"); + testingClient.server().run(UmaRepresentationTest::testCanRepresentResourceBeanOfResourceOwnedByClient); + } + + public static void testCanRepresentResourceBeanOfResourceOwnedByClient(KeycloakSession session) { + session.getContext().setRealm(session.realms().getRealmByName("authz-test")); + AuthorizationProvider authorization = session.getProvider(AuthorizationProvider.class); + + AuthorizationBean authorizationBean = new AuthorizationBean(session, null, session.getContext().getUri()); + ClientModel client = session.getContext().getRealm().getClientByClientId("resource-server-test"); + ResourceBean resourceBean = authorizationBean.new ResourceBean( + authorization.getStoreFactory().getResourceStore().findByName( + "Resource A", client.getId(), client.getId() + ) + ); + + Assert.assertEquals("Resource A", resourceBean.getName()); + Assert.assertEquals("resource-server-test", resourceBean.getOwnerName()); + Assert.assertNotNull(resourceBean.getClientOwner()); + Assert.assertEquals("resource-server-test", resourceBean.getClientOwner().getClientId()); + Assert.assertNull(resourceBean.getUserOwner()); + } +} diff --git a/themes/src/main/resources/theme/base/account/resources.ftl b/themes/src/main/resources/theme/base/account/resources.ftl index 49795a0827..d86e8bc323 100755 --- a/themes/src/main/resources/theme/base/account/resources.ftl +++ b/themes/src/main/resources/theme/base/account/resources.ftl @@ -262,7 +262,7 @@ <#if resource.displayName??>${resource.displayName}<#else>${resource.name} - <#if resource.owner.email??>${resource.owner.email}<#else>${resource.owner.username} + ${resource.ownerName} <#if resource.resourceServer.baseUri??> @@ -362,7 +362,7 @@ <#if resource.displayName??>${resource.displayName}<#else>${resource.name} - <#if resource.owner.email??>${resource.owner.email}<#else>${resource.owner.username} + ${resource.ownerName}