From facdd586a33d486bf565fd62223f4bce18162087 Mon Sep 17 00:00:00 2001 From: Stan Silvert Date: Tue, 1 Nov 2016 16:08:30 -0400 Subject: [PATCH 1/4] KEYCLOAK-2720: Should not allow two groups with the same path. --- .../keycloak/services/resources/admin/GroupResource.java | 7 +++++++ .../keycloak/services/resources/admin/GroupsResource.java | 7 +++++++ 2 files changed, 14 insertions(+) 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 2d5476b613..5088c93893 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) { From 3035cbc5db57bc654f506d9c6b1523f2e60fa970 Mon Sep 17 00:00:00 2001 From: Stan Silvert Date: Fri, 4 Nov 2016 13:11:06 -0400 Subject: [PATCH 2/4] KEYCLOAK-2720: Unit test --- .../testsuite/admin/group/GroupTest.java | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) 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 7e61f511db..24b96e5fff 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 @@ -148,6 +148,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"); @@ -176,7 +202,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); From 3efd103712e0cd04bc66361df679c99c9f458f15 Mon Sep 17 00:00:00 2001 From: Stan Silvert Date: Fri, 4 Nov 2016 19:05:32 -0400 Subject: [PATCH 3/4] KEYCLOAK-2720: Fix PermissionsTest --- .../java/org/keycloak/testsuite/admin/PermissionsTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 29252d98d4..81d4fb2b07 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 @@ -1171,7 +1171,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); From 80b071024f2114a41b89631ebc62e424cf92c452 Mon Sep 17 00:00:00 2001 From: Stan Silvert Date: Tue, 8 Nov 2016 15:05:19 -0500 Subject: [PATCH 4/4] KEYCLOAK-2720: Add unique constraint --- .../META-INF/jpa-changelog-2.4.0.xml | 25 +++++++++++++++++++ .../META-INF/jpa-changelog-master.xml | 1 + 2 files changed, 26 insertions(+) create mode 100755 model/jpa/src/main/resources/META-INF/jpa-changelog-2.4.0.xml diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-2.4.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-2.4.0.xml new file mode 100755 index 0000000000..caf1f2cbe4 --- /dev/null +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-2.4.0.xml @@ -0,0 +1,25 @@ + + + + + + + + + + \ No newline at end of file diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml index 8990fc438c..194b2d51ca 100755 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml @@ -37,4 +37,5 @@ +