diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index 0381c9a416..955a8ba7ad 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -109,20 +109,28 @@ public class UsersResource { @POST @Consumes(MediaType.APPLICATION_JSON) public Response createUser(final UserRepresentation rep) { - // if groups is part of the user rep, check if admin has manage_members and manage_membership on each group - if (rep.getGroups() != null) { - for (String groupPath : rep.getGroups()) { - GroupModel group = KeycloakModelUtils.findGroupByPath(realm, groupPath); - if (group != null) { - auth.groups().requireManageMembers(group); - auth.groups().requireManageMembership(group); - } else { - return ErrorResponse.error(String.format("Group %s not found", groupPath), Response.Status.BAD_REQUEST); - } - } - } else { + // first check if user has manage rights + try { auth.users().requireManage(); } + catch (ForbiddenException exception) { + // if user does not have manage rights, fallback to fine grain admin permissions per group + if (rep.getGroups() != null) { + // if groups is part of the user rep, check if admin has manage_members and manage_membership on each group + for (String groupPath : rep.getGroups()) { + GroupModel group = KeycloakModelUtils.findGroupByPath(realm, groupPath); + if (group != null) { + auth.groups().requireManageMembers(group); + auth.groups().requireManageMembership(group); + } else { + return ErrorResponse.error(String.format("Group %s not found", groupPath), Response.Status.BAD_REQUEST); + } + } + } else { + // propagate exception if no group specified + throw exception; + } + } String username = rep.getUsername(); if(realm.isRegistrationEmailAsUsername()) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java index aabc69b482..f8c8086e48 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java @@ -497,16 +497,21 @@ public abstract class AbstractKeycloakTest { return ApiUtil.createUserWithAdminClient(adminClient.realm(realm), homer); } - public static UserRepresentation createUserRepresentation(String username, String email, String firstName, String lastName, boolean enabled) { + public static UserRepresentation createUserRepresentation(String username, String email, String firstName, String lastName, List groups, boolean enabled) { UserRepresentation user = new UserRepresentation(); user.setUsername(username); user.setEmail(email); user.setFirstName(firstName); user.setLastName(lastName); + user.setGroups(groups); user.setEnabled(enabled); return user; } + public static UserRepresentation createUserRepresentation(String username, String email, String firstName, String lastName, boolean enabled) { + return createUserRepresentation(username, email, firstName, lastName, null, enabled); + } + public static UserRepresentation createUserRepresentation(String username, String email, String firstName, String lastName, boolean enabled, String password) { UserRepresentation user = createUserRepresentation(username, email, firstName, lastName, enabled); setPasswordFor(user, password); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java index b65dc9ff40..99d85b3347 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java @@ -59,6 +59,7 @@ import org.keycloak.testsuite.util.AdminClientUtil; import org.keycloak.testsuite.utils.tls.TLSUtils; import javax.ws.rs.ClientErrorException; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; import java.util.ArrayList; import java.util.Arrays; @@ -260,8 +261,8 @@ public class FineGrainAdminUnitTest extends AbstractKeycloakTest { groupManagerRep.addUser("noMapperGroupManager"); ResourceServer server = permissions.realmResourceServer(); Policy groupManagerPolicy = permissions.authz().getStoreFactory().getPolicyStore().create(groupManagerRep, server); - Policy groupManagerPermission = permissions.groups().manageMembersPermission(group); - groupManagerPermission.addAssociatedPolicy(groupManagerPolicy); + permissions.groups().manageMembersPermission(group).addAssociatedPolicy(groupManagerPolicy); + permissions.groups().manageMembershipPermission(group).addAssociatedPolicy(groupManagerPolicy); permissions.groups().viewPermission(group).addAssociatedPolicy(groupManagerPolicy); UserModel clientMapper = session.users().addUser(realm, "clientMapper"); @@ -372,6 +373,7 @@ public class FineGrainAdminUnitTest extends AbstractKeycloakTest { Assert.assertFalse(permissionsForAdmin.users().canView(user)); UserModel member = session.users().getUserByUsername("groupMember", realm); Assert.assertTrue(permissionsForAdmin.users().canManage(member)); + Assert.assertTrue(permissionsForAdmin.users().canManageGroupMembership(member)); Assert.assertTrue(permissionsForAdmin.users().canView(member)); Assert.assertTrue(permissionsForAdmin.roles().canMapRole(realmRole)); Assert.assertTrue(permissionsForAdmin.roles().canMapRole(clientRole)); @@ -626,6 +628,45 @@ public class FineGrainAdminUnitTest extends AbstractKeycloakTest { } } } + + // KEYCLOAK-11261 : user creation via fine grain admin + + { + try (Keycloak realmClient = AdminClientUtil.createAdminClient(suiteContext.isAdapterCompatTesting(), + TEST, "noMapperGroupManager", "password", Constants.ADMIN_CLI_CLIENT_ID, null)) { + // Should only return the list of users that belong to "top" group + List queryUsers = realmClient.realm(TEST).users().list(); + Assert.assertEquals(1, queryUsers.size()); + + UserRepresentation newGroupMemberWithoutGroup = createUserRepresentation("new-group-member", + "new-group-member@keycloak.org", "New", "Member", true); + try { + ApiUtil.createUserWithAdminClient(realmClient.realm(TEST), newGroupMemberWithoutGroup); + Assert.fail("should fail with HTTP response code 403 Forbidden"); + } catch (WebApplicationException e) { + Assert.assertEquals(403, e.getResponse().getStatus()); + } + + UserRepresentation newGroupMemberWithAWrongGroup = createUserRepresentation("new-group-member", + "new-group-member@keycloak.org", "New", "Member", + Arrays.asList("wrong-group"),true); + try { + ApiUtil.createUserWithAdminClient(realmClient.realm(TEST), newGroupMemberWithAWrongGroup); + Assert.fail("should fail with HTTP response code 400 Bad Request"); + } catch (WebApplicationException e) { + Assert.assertEquals(400, e.getResponse().getStatus()); + } + + UserRepresentation newGroupMember = createUserRepresentation("new-group-member", + "new-group-member@keycloak.org", "New", "Member", + Arrays.asList("top"), true); + ApiUtil.createUserWithAdminClient(realmClient.realm(TEST), newGroupMember); + + // Should only return the list of users that belong to "top" group + the new one + queryUsers = realmClient.realm(TEST).users().list(); + Assert.assertEquals(2, queryUsers.size()); + } + } } @Test