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

KEYCLOAK-17581 Prevent empty group names

Browse source 
Create / Update operations in `GroupResource ` and `GroupsResource#addTopLevelGroup`
did not validate the given group name. This allowed the creation of groups with empty names.

We now prevent the creation of groups with empty names.
This commit is contained in:
Thomas Darimont 2021-03-25 10:12:55 +01:00 committed by Pedro Igor
parent a36fafe04e
commit 7ec6a54e22
3 changed files with 81 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -19,6 +19,7 @@ package org.keycloak.services.resources.admin;
import org.jboss.resteasy.annotations.cache.NoCache;
import javax.ws.rs.NotFoundException;
import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.Constants;
@ -100,10 +101,15 @@ public class GroupResource {
public Response updateGroup(GroupRepresentation rep) {
this.auth.groups().requireManage(group);
String groupName = rep.getName();
if (ObjectUtil.isBlank(groupName)) {
return ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
}
boolean exists = siblings().filter(s -> !Objects.equals(s.getId(), group.getId()))
.anyMatch(s -> Objects.equals(s.getName(), rep.getName()));
.anyMatch(s -> Objects.equals(s.getName(), groupName));
if (exists) {
return ErrorResponse.exists("Sibling group named '" + rep.getName() + "' already exists.");
return ErrorResponse.exists("Sibling group named '" + groupName + "' already exists.");
}
updateGroup(rep, group);
@ -143,6 +149,11 @@ public class GroupResource {
public Response addChild(GroupRepresentation rep) {
this.auth.groups().requireManage(group);
String groupName = rep.getName();
if (ObjectUtil.isBlank(groupName)) {
return ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
}
Response.ResponseBuilder builder = Response.status(204);
GroupModel child = null;
if (rep.getId() != null) {
@ -153,7 +164,7 @@ public class GroupResource {
realm.moveGroup(child, group);
adminEvent.operation(OperationType.UPDATE);
} else {
child = realm.createGroup(rep.getName(), group);
child = realm.createGroup(groupName, group);
updateGroup(rep, child);
URI uri = session.getContext().getUri().getBaseUriBuilder()
.path(session.getContext().getUri().getMatchedURIs().get(2))

11
services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java
View file

@ -19,6 +19,7 @@ package org.keycloak.services.resources.admin;
import org.jboss.resteasy.annotations.cache.NoCache;
import javax.ws.rs.NotFoundException;
import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.GroupModel;
@ -140,6 +141,12 @@ public class GroupsResource {
GroupModel child;
Response.ResponseBuilder builder = Response.status(204);
String groupName = rep.getName();
if (ObjectUtil.isBlank(groupName)) {
return ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
}
try {
if (rep.getId() != null) {
child = realm.getGroupById(rep.getId());
@ -149,7 +156,7 @@ public class GroupsResource {
realm.moveGroup(child, null);
adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri());
} else {
child = realm.createGroup(rep.getName());
child = realm.createGroup(groupName);
GroupResource.updateGroup(rep, child);
URI uri = session.getContext().getUri().getAbsolutePathBuilder()
.path(child.getId()).build();
@ -159,7 +166,7 @@ public class GroupsResource {
adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), child.getId());
}
} catch (ModelDuplicateException mde) {
return ErrorResponse.exists("Top level group named '" + rep.getName() + "' already exists.");
return ErrorResponse.exists("Top level group named '" + groupName + "' already exists.");
}
adminEvent.representation(rep).success();

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

@ -300,6 +300,64 @@ public class GroupTest extends AbstractGroupTest {
});
}
// KEYCLOAK-17581
@Test
public void createGroupWithEmptyNameShouldFail() {
RealmResource realm = adminClient.realms().realm("test");
GroupRepresentation group = new GroupRepresentation();
group.setName("");
try (Response response = realm.groups().add(group)){
if (response.getStatus() != 400) {
Assert.fail("Creating a group with empty name should fail");
}
} catch (Exception expected) {
Assert.assertNotNull(expected);
}
group.setName(null);
try (Response response = realm.groups().add(group)){
if (response.getStatus() != 400) {
Assert.fail("Creating a group with null name should fail");
}
} catch (Exception expected) {
Assert.assertNotNull(expected);
}
}
// KEYCLOAK-17581
@Test
public void updatingGroupWithEmptyNameShouldFail() {
RealmResource realm = adminClient.realms().realm("test");
GroupRepresentation group = new GroupRepresentation();
group.setName("groupWithName");
String groupId = null;
try (Response response = realm.groups().add(group)) {
groupId = ApiUtil.getCreatedId(response);
}
try {
group.setName("");
realm.groups().group(groupId).update(group);
Assert.fail("Updating a group with empty name should fail");
} catch(Exception expected) {
Assert.assertNotNull(expected);
}
try {
group.setName(null);
realm.groups().group(groupId).update(group);
Assert.fail("Updating a group with null name should fail");
} catch(Exception expected) {
Assert.assertNotNull(expected);
}
}
@Test
public void createAndTestGroups() throws Exception {
RealmResource realm = adminClient.realms().realm("test");