diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RoleResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RoleResource.java index 53ca36df06..f5604c51af 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RoleResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/RoleResource.java @@ -102,31 +102,37 @@ public interface RoleResource { void deleteComposites(List rolesToRemove); /** - * Get role members - *

- * Returns users that have the given role - * + * Get role members. + *

+ * Returns users that have the given role, sorted by username ascending. + *

+ *

+ * Note: This method just returns the first 100 users. In order to retrieve all users, use paging (see + * {@link #getUserMembers(Integer, Integer)}). + *

+ * * @return a list of users with the given role */ @GET @Path("users") @Produces(MediaType.APPLICATION_JSON) - Set getRoleUserMembers(); + List getUserMembers(); /** - * Get role members + * Get role members. *

- * Returns users that have the given role, paginated according to the query parameters + * Returns users that have the given role, sorted by username ascending, paginated according to the query + * parameters. * * @param firstResult Pagination offset - * @param maxResults Pagination size + * @param maxResults Pagination size * @return a list of users with the given role */ @GET @Path("users") @Produces(MediaType.APPLICATION_JSON) - Set getRoleUserMembers(@QueryParam("first") Integer firstResult, - @QueryParam("max") Integer maxResults); + List getUserMembers(@QueryParam("first") Integer firstResult, + @QueryParam("max") Integer maxResults); /** * Get role groups @@ -154,4 +160,37 @@ public interface RoleResource { @Produces(MediaType.APPLICATION_JSON) Set getRoleGroupMembers(@QueryParam("first") Integer firstResult, @QueryParam("max") Integer maxResults); + + /** + * Get role members + *

+ * Returns users that have the given role + * + * @return a set of users with the given role + * + * @deprecated please use {@link #getUserMembers()} + */ + @GET + @Path("users") + @Produces(MediaType.APPLICATION_JSON) + @Deprecated + Set getRoleUserMembers(); + + /** + * Get role members + *

+ * Returns users that have the given role, paginated according to the query parameters + * + * @param firstResult Pagination offset + * @param maxResults Pagination size + * @return a set of users with the given role + * + * @deprecated please use {@link #getUserMembers(Integer, Integer)} + */ + @GET + @Path("users") + @Produces(MediaType.APPLICATION_JSON) + @Deprecated + Set getRoleUserMembers(@QueryParam("first") Integer firstResult, + @QueryParam("max") Integer maxResults); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java index a75076fa60..bc8768ba58 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java @@ -34,7 +34,7 @@ import java.io.Serializable; * @version $Revision: 1 $ */ @NamedQueries({ - @NamedQuery(name="usersInRole", query="select u from UserRoleMappingEntity m, UserEntity u where m.roleId=:roleId and u.id=m.user"), + @NamedQuery(name="usersInRole", query="select u from UserRoleMappingEntity m, UserEntity u where m.roleId=:roleId and u.id=m.user order by u.username"), @NamedQuery(name="userHasRole", query="select m from UserRoleMappingEntity m where m.user = :user and m.roleId = :roleId"), @NamedQuery(name="userRoleMappings", query="select m from UserRoleMappingEntity m where m.user = :user"), @NamedQuery(name="userRoleMappingIds", query="select m.roleId from UserRoleMappingEntity m where m.user = :user"), diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientRolesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientRolesTest.java index 80a70ea13e..c4b0d64488 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientRolesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientRolesTest.java @@ -31,14 +31,14 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.util.AdminEventPaths; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -216,38 +216,40 @@ public class ClientRolesTest extends AbstractClientTest { List roleToAdd = Collections.singletonList(rolesRsc.get(roleName).toRepresentation()); //create users and assign test role - Set users = new HashSet<>(); - for (int i = 0; i < 10; i++) { - String userName = "user" + i; + List usernames = createUsernames(0, 10); + usernames.forEach(username -> { UserRepresentation user = new UserRepresentation(); - user.setUsername(userName); + user.setUsername(username); testRealmResource().users().create(user); - user = getFullUserRep(userName); + user = getFullUserRep(username); testRealmResource().users().get(user.getId()).roles().clientLevel(clientID).add(roleToAdd); - users.add(user); - } + }); // check if users have test role assigned RoleResource roleResource = rolesRsc.get(roleName); - Set usersInRole = roleResource.getRoleUserMembers(); - assertEquals(users.size(), usersInRole.size()); - for (UserRepresentation user : users) { - Optional result = usersInRole.stream().filter(u -> user.getUsername().equals(u.getUsername())).findAny(); - assertTrue(result.isPresent()); - } + List usersInRole = roleResource.getUserMembers(); + assertEquals(usernames, extractUsernames(usersInRole)); // pagination - Set usersInRole1 = roleResource.getRoleUserMembers(0, 5); - assertEquals(5, usersInRole1.size()); - Set usersInRole2 = roleResource.getRoleUserMembers(5, 10); - assertEquals(5, usersInRole2.size()); - for (UserRepresentation user : users) { - Optional result1 = usersInRole1.stream().filter(u -> user.getUsername().equals(u.getUsername())).findAny(); - Optional result2 = usersInRole2.stream().filter(u -> user.getUsername().equals(u.getUsername())).findAny(); - assertTrue((result1.isPresent() || result2.isPresent()) && !(result1.isPresent() && result2.isPresent())); - } + List usersInRole1 = roleResource.getUserMembers(0, 5); + assertEquals(createUsernames(0, 5), extractUsernames(usersInRole1)); + List usersInRole2 = roleResource.getUserMembers(5, 10); + assertEquals(createUsernames(5, 10), extractUsernames(usersInRole2)); } - + + private static List createUsernames(int startIndex, int endIndex) { + List usernames = new ArrayList<>(); + for (int i = startIndex; i < endIndex; i++) { + String userName = "user" + i; + usernames.add(userName); + } + return usernames; + } + + private static List extractUsernames(Collection users) { + return users.stream().map(UserRepresentation::getUsername).collect(Collectors.toList()); + } + @Test public void testSearchForRoles() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmRolesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmRolesTest.java index 3136a8a13e..4d5e554cf8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmRolesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmRolesTest.java @@ -39,9 +39,9 @@ import javax.ws.rs.NotFoundException; import javax.ws.rs.core.Response; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -52,7 +52,6 @@ import javax.ws.rs.ClientErrorException; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; @@ -237,7 +236,7 @@ public class RealmRolesTest extends AbstractAdminTest { public void testUsersInRole() { RoleResource role = resource.get("role-with-users"); - List users = adminClient.realm(REALM_NAME).users().search("test-role-member", null, null, null, null, null); + List users = adminClient.realm(REALM_NAME).users().search("test-role-member"); assertEquals(1, users.size()); UserResource user = adminClient.realm(REALM_NAME).users().get(users.get(0).getId()); UserRepresentation userRep = user.toRepresentation(); @@ -248,12 +247,12 @@ public class RealmRolesTest extends AbstractAdminTest { adminClient.realm(REALM_NAME).users().get(userRep.getId()).roles().realmLevel().add(rolesToAdd); roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName()); - roleResource.getRoleUserMembers(); - //roleResource.getRoleUserMembers().stream().forEach((member) -> log.infof("Found user {}", member.getUsername())); - assertEquals(1, roleResource.getRoleUserMembers().size()); - + assertEquals(Collections.singletonList("test-role-member"), extractUsernames(roleResource.getUserMembers())); } + private static List extractUsernames(Collection users) { + return users.stream().map(UserRepresentation::getUsername).collect(Collectors.toList()); + } /** * KEYCLOAK-2035 Verifies that Role with no users assigned is being properly retrieved without members in API endpoint for role membership @@ -263,9 +262,7 @@ public class RealmRolesTest extends AbstractAdminTest { RoleResource role = resource.get("role-without-users"); role = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName()); - role.getRoleUserMembers(); - assertEquals(0, role.getRoleUserMembers().size()); - + assertThat(role.getUserMembers(), is(empty())); } @@ -321,13 +318,10 @@ public class RealmRolesTest extends AbstractAdminTest { adminClient.realm(REALM_NAME).users().get(userRep.getId()).roles().realmLevel().add(rolesToAdd); roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName()); - roleResource.getRoleUserMembers(); - assertEquals(1, roleResource.getRoleUserMembers().size()); + assertEquals(Collections.singletonList("test-role-member"), extractUsernames(roleResource.getUserMembers())); adminClient.realm(REALM_NAME).users().delete(userRep.getId()); - roleResource.getRoleUserMembers(); - assertEquals(0, roleResource.getRoleUserMembers().size()); - + assertThat(roleResource.getUserMembers(), is(empty())); } @Test @@ -355,20 +349,16 @@ public class RealmRolesTest extends AbstractAdminTest { } RoleResource roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName()); - Set roleUserMembers = roleResource.getRoleUserMembers(0, 1); - Set expectedMembers = new HashSet<>(); + List roleUserMembers = roleResource.getUserMembers(0, 1); + assertEquals(Collections.singletonList("test-role-member"), extractUsernames(roleUserMembers)); + + roleUserMembers = roleResource.getUserMembers(1, 1); assertThat(roleUserMembers, hasSize(1)); - expectedMembers.add(roleUserMembers.iterator().next().getUsername()); + assertEquals(Collections.singletonList("test-role-member2"), extractUsernames(roleUserMembers)); - roleUserMembers = roleResource.getRoleUserMembers(1, 1); - assertThat(roleUserMembers, hasSize(1)); - expectedMembers.add(roleUserMembers.iterator().next().getUsername()); - - roleUserMembers = roleResource.getRoleUserMembers(2, 1); + roleUserMembers = roleResource.getUserMembers(2, 1); assertThat(roleUserMembers, is(empty())); - - assertThat(expectedMembers, containsInAnyOrder("test-role-member", "test-role-member2")); } // issue #9587