diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-2.5.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-2.5.0.xml index 4aee29165b..37d02502be 100755 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-2.5.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-2.5.0.xml @@ -20,6 +20,8 @@ + + diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java index ea423352e9..398e03615d 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java @@ -48,6 +48,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import org.keycloak.services.ErrorResponse; /** * @author Bill Burke @@ -138,6 +139,12 @@ public class GroupResource { if (group == null) { throw new NotFoundException("Could not find group by id"); } + + for (GroupModel group : group.getSubGroups()) { + if (group.getName().equals(rep.getName())) { + return ErrorResponse.exists("Parent already contains subgroup named '" + rep.getName() + "'"); + } + } Response.ResponseBuilder builder = Response.status(204); GroupModel child = null; diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java index f670f57413..9d01f35ab1 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java @@ -39,6 +39,7 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.net.URI; import java.util.List; +import org.keycloak.services.ErrorResponse; /** * @author Bill Burke @@ -102,6 +103,12 @@ public class GroupsResource { public Response addTopLevelGroup(GroupRepresentation rep) { auth.requireManage(); + for (GroupModel group : realm.getGroups()) { + if (group.getName().equals(rep.getName())) { + return ErrorResponse.exists("Top level group named '" + rep.getName() + "' already exists."); + } + } + GroupModel child = null; Response.ResponseBuilder builder = Response.status(204); if (rep.getId() != null) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java index b23194a7ac..18277d27ec 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java @@ -1173,7 +1173,9 @@ public class PermissionsTest extends AbstractKeycloakTest { }, Resource.USER, false); invoke(new InvocationWithResponse() { public void invoke(RealmResource realm, AtomicReference response) { - response.set(realm.groups().add(new GroupRepresentation())); + GroupRepresentation group = new GroupRepresentation(); + group.setName("mygroup"); + response.set(realm.groups().add(group)); } }, Resource.USER, true); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java index 91d299f138..79908d8e69 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java @@ -151,6 +151,32 @@ public class GroupTest extends AbstractGroupTest { return group; } + @Test + public void doNotAllowSameGroupNameAtSameLevel() throws Exception { + RealmResource realm = adminClient.realms().realm("test"); + + GroupRepresentation topGroup = new GroupRepresentation(); + topGroup.setName("top"); + topGroup = createGroup(realm, topGroup); + + GroupRepresentation anotherTopGroup = new GroupRepresentation(); + anotherTopGroup.setName("top"); + Response response = realm.groups().add(anotherTopGroup); + assertEquals(409, response.getStatus()); // conflict status 409 - same name not allowed + + GroupRepresentation level2Group = new GroupRepresentation(); + level2Group.setName("level2"); + response = realm.groups().group(topGroup.getId()).subGroup(level2Group); + response.close(); + assertEquals(201, response.getStatus()); // created status + + GroupRepresentation anotherlevel2Group = new GroupRepresentation(); + anotherlevel2Group.setName("level2"); + response = realm.groups().group(topGroup.getId()).subGroup(anotherlevel2Group); + response.close(); + assertEquals(409, response.getStatus()); // conflict status 409 - same name not allowed + } + @Test public void createAndTestGroups() throws Exception { RealmResource realm = adminClient.realms().realm("test"); @@ -179,7 +205,7 @@ public class GroupTest extends AbstractGroupTest { GroupRepresentation topGroup = new GroupRepresentation(); topGroup.setName("top"); topGroup = createGroup(realm, topGroup); - + List roles = new LinkedList<>(); roles.add(topRole); realm.groups().group(topGroup.getId()).roles().realmLevel().add(roles);