From b1bcd5d66e148eee4331e2a2f60a8080761ff8ee Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 15 Oct 2021 15:33:19 +0200 Subject: [PATCH] KEYCLOAK-12754 Honor nested composite roles when creating roles via REST API (#7097) * KEYCLOAK-12754 Honor nested composite roles when creating roles via REST API - Validate composite roles when creating roles via REST API --- .../admin/RoleContainerResource.java | 46 ++++++++++++++++ .../testsuite/admin/RoleByIdResourceTest.java | 55 ++++++++++++++++++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java index a718fce918..23a40b2a9a 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java @@ -28,7 +28,10 @@ import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleContainerModel; import org.keycloak.models.RoleModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; +import org.keycloak.models.utils.RoleUtils; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.ManagementPermissionReference; import org.keycloak.representations.idm.RoleRepresentation; @@ -52,10 +55,13 @@ import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.keycloak.services.ErrorResponseException; @@ -148,6 +154,46 @@ public class RoleContainerResource extends RoleResource { adminEvent.resource(ResourceType.REALM_ROLE); } + // Handling of nested composite roles for KEYCLOAK-12754 + if (rep.isComposite() && rep.getComposites() != null) { + RoleRepresentation.Composites composites = rep.getComposites(); + + Set compositeRealmRoles = composites.getRealm(); + if (compositeRealmRoles != null && !compositeRealmRoles.isEmpty()) { + Set realmRoles = new LinkedHashSet<>(); + for (String roleName : compositeRealmRoles) { + RoleModel realmRole = realm.getRole(roleName); + if (realmRole == null) { + return ErrorResponse.error("Realm Role with name " + roleName + " does not exist", Response.Status.NOT_FOUND); + } + realmRoles.add(realmRole); + } + RoleUtils.expandCompositeRoles(realmRoles).forEach(role::addCompositeRole); + } + + Map> compositeClientRoles = composites.getClient(); + if (compositeClientRoles != null && !compositeClientRoles.isEmpty()) { + Set>> entries = compositeClientRoles.entrySet(); + for (Map.Entry> clientIdWithClientRoleNames : entries) { + String clientId = clientIdWithClientRoleNames.getKey(); + List clientRoleNames = clientIdWithClientRoleNames.getValue(); + ClientModel client = realm.getClientByClientId(clientId); + if (client == null) { + continue; + } + Set clientRoles = new LinkedHashSet<>(); + for (String roleName : clientRoleNames) { + RoleModel clientRole = client.getRole(roleName); + if (clientRole == null) { + return ErrorResponse.error("Client Role with name " + roleName + " does not exist", Response.Status.NOT_FOUND); + } + clientRoles.add(clientRole); + } + RoleUtils.expandCompositeRoles(clientRoles).forEach(role::addCompositeRole); + } + } + } + adminEvent.operation(OperationType.CREATE).resourcePath(uriInfo, role.getName()).representation(rep).success(); return Response.created(uriInfo.getAbsolutePathBuilder().path(role.getName()).build()).build(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RoleByIdResourceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RoleByIdResourceTest.java index d1f68efcb9..94ea525b22 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RoleByIdResourceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/RoleByIdResourceTest.java @@ -38,6 +38,8 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; @@ -56,6 +58,9 @@ public class RoleByIdResourceTest extends AbstractAdminTest { private RoleByIdResource resource; private Map ids = new HashMap<>(); + + private String clientId; + private String clientUuid; @Before @@ -63,7 +68,8 @@ public class RoleByIdResourceTest extends AbstractAdminTest { adminClient.realm(REALM_NAME).roles().create(RoleBuilder.create().name("role-a").description("Role A").build()); adminClient.realm(REALM_NAME).roles().create(RoleBuilder.create().name("role-b").description("Role B").build()); - Response response = adminClient.realm(REALM_NAME).clients().create(ClientBuilder.create().clientId("client-a").build()); + clientId = "client-a"; + Response response = adminClient.realm(REALM_NAME).clients().create(ClientBuilder.create().clientId(clientId).build()); clientUuid = ApiUtil.getCreatedId(response); getCleanup().addClientUuid(clientUuid); response.close(); @@ -172,6 +178,53 @@ public class RoleByIdResourceTest extends AbstractAdminTest { } + /** + * see KEYCLOAK-12754 + */ + @Test + public void createNewMixedRealmCompositeRole() { + + RoleRepresentation newRoleComp = RoleBuilder.create().name("role-mixed-comp").composite().realmComposite("role-a").clientComposite(clientId, "role-c").build(); + adminClient.realm(REALM_NAME).roles().create(newRoleComp); + + RoleRepresentation roleMixedComp = adminClient.realm(REALM_NAME).roles().get(newRoleComp.getName()).toRepresentation(); + assertTrue(roleMixedComp.isComposite()); + + Predicate isClientRole = RoleRepresentation::getClientRole; + + Set roleComposites = resource.getRoleComposites(roleMixedComp.getId()); + Set containedRealmRoles = roleComposites.stream().filter(isClientRole.negate()).collect(Collectors.toSet()); + assertFalse(containedRealmRoles.isEmpty()); + assertTrue(containedRealmRoles.stream().anyMatch(r -> r.getName().equals("role-a"))); + + Set containedClientRoles = roleComposites.stream().filter(isClientRole).collect(Collectors.toSet()); + assertFalse(containedClientRoles.isEmpty()); + assertTrue(containedClientRoles.stream().anyMatch(r -> r.getContainerId().equals(clientUuid) && r.getName().equals("role-c"))); + } + + /** + * see KEYCLOAK-12754 + */ + @Test(expected = NotFoundException.class) + public void createNewMixedRealmCompositeRoleWithUnknownRealmRoleShouldThrow() { + + String unknownRealmRole = "realm-role-unknown"; + RoleRepresentation newRoleComp = RoleBuilder.create().name("role-broken-comp1").composite().realmComposite(unknownRealmRole).clientComposite(clientId, "role-c").build(); + + adminClient.realm(REALM_NAME).roles().create(newRoleComp); + } + + /** + * see KEYCLOAK-12754 + */ + @Test(expected = NotFoundException.class) + public void createNewMixedRealmCompositeRoleWithUnknownClientRoleShouldThrow() { + + String unknownClientRole = "client-role-unknown"; + RoleRepresentation newRoleComp = RoleBuilder.create().name("role-broken-comp2").composite().realmComposite("role-a").clientComposite(clientId, unknownClientRole).build(); + adminClient.realm(REALM_NAME).roles().create(newRoleComp); + } + @Test public void attributes() { for (String id : ids.values()) {