KEYCLOAK-11412 Display more nice error message when creating top level group with same name

This commit is contained in:
mposolda 2020-03-16 11:18:33 +01:00 committed by Marek Posolda
parent d7688f6b12
commit 56d1ab19a8
2 changed files with 38 additions and 19 deletions

View file

@ -23,6 +23,7 @@ import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType; import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.GroupModel; import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.GroupRepresentation;
@ -141,28 +142,27 @@ public class GroupsResource {
public Response addTopLevelGroup(GroupRepresentation rep) { public Response addTopLevelGroup(GroupRepresentation rep) {
auth.groups().requireManage(); auth.groups().requireManage();
List<GroupRepresentation> search = ModelToRepresentation.searchForGroupByName(realm, false, rep.getName(), 0, 1);
if (search != null && !search.isEmpty() && Objects.equals(search.get(0).getName(), rep.getName())) {
return ErrorResponse.exists("Top level group named '" + rep.getName() + "' already exists.");
}
GroupModel child; GroupModel child;
Response.ResponseBuilder builder = Response.status(204); Response.ResponseBuilder builder = Response.status(204);
if (rep.getId() != null) { try {
child = realm.getGroupById(rep.getId()); if (rep.getId() != null) {
if (child == null) { child = realm.getGroupById(rep.getId());
throw new NotFoundException("Could not find child by id"); if (child == null) {
} throw new NotFoundException("Could not find child by id");
adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()); }
} else { adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri());
child = realm.createGroup(rep.getName()); } else {
GroupResource.updateGroup(rep, child); child = realm.createGroup(rep.getName());
URI uri = session.getContext().getUri().getAbsolutePathBuilder() GroupResource.updateGroup(rep, child);
.path(child.getId()).build(); URI uri = session.getContext().getUri().getAbsolutePathBuilder()
builder.status(201).location(uri); .path(child.getId()).build();
builder.status(201).location(uri);
rep.setId(child.getId()); rep.setId(child.getId());
adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), child.getId()); adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), child.getId());
}
} catch (ModelDuplicateException mde) {
return ErrorResponse.exists("Top level group named '" + rep.getName() + "' already exists.");
} }
adminEvent.representation(rep).success(); adminEvent.representation(rep).success();

View file

@ -248,17 +248,36 @@ public class GroupTest extends AbstractGroupTest {
GroupRepresentation topGroup = new GroupRepresentation(); GroupRepresentation topGroup = new GroupRepresentation();
topGroup.setName("test-group"); topGroup.setName("test-group");
topGroup = createGroup(realm, topGroup); topGroup = createGroup(realm, topGroup);
getCleanup().addGroupId(topGroup.getId());
// creating "/test-group/test-group" // creating "/test-group/test-group"
GroupRepresentation childGroup = new GroupRepresentation(); GroupRepresentation childGroup = new GroupRepresentation();
childGroup.setName("test-group"); childGroup.setName("test-group");
try (Response response = realm.groups().group(topGroup.getId()).subGroup(childGroup)) { try (Response response = realm.groups().group(topGroup.getId()).subGroup(childGroup)) {
assertEquals(201, response.getStatus()); assertEquals(201, response.getStatus());
getCleanup().addGroupId(ApiUtil.getCreatedId(response));
} }
assertNotNull(realm.getGroupByPath("/test-group/test-group")); assertNotNull(realm.getGroupByPath("/test-group/test-group"));
} }
@Test
public void doNotAllowSameGroupNameAtTopLevel() throws Exception {
RealmResource realm = adminClient.realms().realm("test");
// creating "/test-group"
GroupRepresentation topGroup = new GroupRepresentation();
topGroup.setName("test-group");
topGroup = createGroup(realm, topGroup);
getCleanup().addGroupId(topGroup.getId());
GroupRepresentation group2 = new GroupRepresentation();
group2.setName("test-group");
try (Response response = realm.groups().add(group2)) {
assertEquals(Status.CONFLICT.getStatusCode(), response.getStatus());
}
}
@Test @Test
@UncaughtServerErrorExpected @UncaughtServerErrorExpected
public void doNotAllowSameGroupNameAtTopLevelInDatabase() throws Exception { public void doNotAllowSameGroupNameAtTopLevelInDatabase() throws Exception {