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 1f1bfd8c62..1555d99b96 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 @@ -156,6 +156,10 @@ public class GroupResource { if (ObjectUtil.isBlank(groupName)) { return ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST); } + boolean childExists = group.getSubGroupsStream().anyMatch(s -> Objects.equals(s.getName(), groupName)); + if (childExists) { + return ErrorResponse.exists("Sibling group named '" + groupName + "' already exists."); + } Response.ResponseBuilder builder = Response.status(204); GroupModel child = null; 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 318d0e3f79..03bc270ad5 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 @@ -33,6 +33,7 @@ import org.keycloak.models.Constants; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.CredentialRepresentation; +import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.MappingsRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -167,6 +168,7 @@ public class GroupTest extends AbstractGroupTest { } @Test + // KEYCLOAK-16888 Error messages for groups with same name in the same level public void doNotAllowSameGroupNameAtSameLevel() throws Exception { RealmResource realm = adminClient.realms().realm("test"); @@ -177,19 +179,20 @@ public class GroupTest extends AbstractGroupTest { GroupRepresentation anotherTopGroup = new GroupRepresentation(); anotherTopGroup.setName("top"); Response response = realm.groups().add(anotherTopGroup); - assertEquals(409, response.getStatus()); // conflict status 409 - same name not allowed + assertSameNameNotAllowed(response,"Top level group named 'top' already exists."); + response.close(); GroupRepresentation level2Group = new GroupRepresentation(); level2Group.setName("level2"); response = realm.groups().group(topGroup.getId()).subGroup(level2Group); - response.close(); assertEquals(201, response.getStatus()); // created status + response.close(); GroupRepresentation anotherlevel2Group = new GroupRepresentation(); anotherlevel2Group.setName("level2"); response = realm.groups().group(topGroup.getId()).subGroup(anotherlevel2Group); + assertSameNameNotAllowed(response,"Sibling group named 'level2' already exists."); response.close(); - assertEquals(409, response.getStatus()); // conflict status 409 - same name not allowed } @Test @@ -212,7 +215,7 @@ public class GroupTest extends AbstractGroupTest { Assert.fail("Expected ClientErrorException"); } catch (ClientErrorException e) { // conflict status 409 - same name not allowed - assertEquals("HTTP 409 Conflict", e.getMessage()); + assertSameNameNotAllowed(e.getResponse(),"Sibling group named 'top1' already exists."); } GroupRepresentation level2Group = new GroupRepresentation(); @@ -230,7 +233,7 @@ public class GroupTest extends AbstractGroupTest { Assert.fail("Expected ClientErrorException"); } catch (ClientErrorException e) { // conflict status 409 - same name not allowed - assertEquals("HTTP 409 Conflict", e.getMessage()); + assertSameNameNotAllowed(e.getResponse(),"Sibling group named 'level2-1' already exists."); } } @@ -1271,4 +1274,14 @@ public class GroupTest extends AbstractGroupTest { searchResultGroups.remove(0); assertTrue(searchResultGroups.isEmpty()); } + + /** + * Assert that when you create/move/update a group name, the response is not Http 409 Conflict and the message does not + * correspond to the returned user-friendly message in such cases + */ + private void assertSameNameNotAllowed(Response response, String expectedErrorMessage) { + assertEquals(409, response.getStatus()); + ErrorRepresentation error = response.readEntity(ErrorRepresentation.class); + Assert.assertEquals(expectedErrorMessage, error.getErrorMessage()); + } }