libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Return a user friendly message when a group name already exists on the same level.

Browse source 
Closes #16888
This commit is contained in:
Klajdi Paja 2023-03-23 13:15:22 +01:00 committed by Marek Posolda
parent a48db930fe
commit cf61a65198
2 changed files with 22 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java
View file

@ -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;

23
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java
View file

@ -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());
}
}