From 6d74e6b2895d8b0820d910a818bef8f67d0df129 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Wed, 17 Jan 2024 16:30:49 +0100 Subject: [PATCH] Escape slashes in full group path representation but disabled by default Closes #23900 Signed-off-by: rmartinc --- .../roles-groups/proc-managing-groups.adoc | 9 ++ .../topics/changes/changes-25_0_0.adoc | 15 +- js/apps/admin-ui/src/user/UserGroups.tsx | 5 +- .../models/cache/infinispan/GroupAdapter.java | 6 + .../org/keycloak/models/jpa/GroupAdapter.java | 10 +- .../models/jpa/JpaGroupProviderFactory.java | 26 +++ .../keycloak/models/jpa/JpaRealmProvider.java | 8 +- .../keycloak/models/GroupProviderFactory.java | 4 + .../models/utils/KeycloakModelUtils.java | 90 +++++++++-- .../models/KeycloakModelUtilsTest.java | 150 ++++++++++++++++++ .../java/org/keycloak/models/GroupModel.java | 4 + .../org/keycloak/models/GroupProvider.java | 2 + .../resources/admin/RealmAdminResource.java | 3 +- .../admin/group/GroupMappersTest.java | 47 +++++- .../testsuite/admin/group/GroupTest.java | 35 +++- 15 files changed, 380 insertions(+), 34 deletions(-) diff --git a/docs/documentation/server_admin/topics/roles-groups/proc-managing-groups.adoc b/docs/documentation/server_admin/topics/roles-groups/proc-managing-groups.adoc index bc46613910..b67444c161 100644 --- a/docs/documentation/server_admin/topics/roles-groups/proc-managing-groups.adoc +++ b/docs/documentation/server_admin/topics/roles-groups/proc-managing-groups.adoc @@ -12,6 +12,15 @@ Groups are hierarchical. A group can have multiple subgroups but a group can hav If you have a parent group and a child group, and a user that belongs only to the child group, the user in the child group inherits the attributes and role mappings of both the parent group and the child group. +The hierarchy of a group is sometimes represented using the group path. The path is the complete list of names that represents the hierarchy of a specific group, from top to bottom and separated by slashes `/` (similar to files in a File System). For example a path can be `/top/level1/level2` which means that `top` is a top level group and is parent of `level1`, which in turn is parent of `level2`. This path represents unambiguously the hierarchy for the group `level2`. + +Because of historical reasons {project_name}, does not escape slashes in the group name itself. Therefore a group named `level1/group` under `top` uses the path `/top/level1/group`, which is misleading. {project_name} can be started with the option `--spi-group-jpa-escape-slashes-in-group-path` to `true` and then the slashes in the name are escaped with the character `~`. The escape char marks that the slash is part of the name and has no hierarchical meaning. The previous path example would be `/top/level1~/group` when escaped. + +[source,bash] +---- +bin/kc.[sh|bat] start --spi-group-jpa-escape-slashes-in-group-path=true +---- + The following example includes a top-level *Sales* group and a child *North America* subgroup. To add a group: diff --git a/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc b/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc index 35a37ffe21..871a2b2706 100644 --- a/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-25_0_0.adoc @@ -143,4 +143,17 @@ However, this property is deprecated and will be removed in future releases, so The management interface uses a different HTTP server than the default {project_name} HTTP server, and it is possible to configure them separately. Beware, if no values are supplied for the management interface properties, they are inherited from the default {project_name} HTTP server. -For more details, see https://www.keycloak.org/server/management-interface[Configuring the Management Interface]. \ No newline at end of file +For more details, see https://www.keycloak.org/server/management-interface[Configuring the Management Interface]. + += Escaping slashes in group paths + +{project_name} has never escaped slashes in the group paths. Because of that, a group named `group/slash` child of `top` uses the full path `/top/group/slash`, which is clearly misleading. Starting with this version, the server can be started to perform escaping of those slashes in the name: + +[source,bash] +---- +bin/kc.[sh|bat] start --spi-group-jpa-escape-slashes-in-group-path=true +---- + +The escape char is the tilde character `~`. The previous example results in the path `/top/group~/slash`. The escape marks the last slash is part of the name and not a hierarchy separator. + +The escaping is currently disabled by default because it represents a change in behavior. Nevertheless enabling escaping is recommended and it can be the default in future versions. diff --git a/js/apps/admin-ui/src/user/UserGroups.tsx b/js/apps/admin-ui/src/user/UserGroups.tsx index f2cc2013f4..e4cf323371 100644 --- a/js/apps/admin-ui/src/user/UserGroups.tsx +++ b/js/apps/admin-ui/src/user/UserGroups.tsx @@ -74,7 +74,10 @@ export const UserGroups = ({ user }: UserGroupsProps) => { const indirect: GroupRepresentation[] = []; if (!isDirectMembership) joinedUserGroups.forEach((g) => { - const paths = g.path?.substring(1).split("/").slice(0, -1) || []; + const paths = ( + g.path?.substring(1).match(/((~\/)|[^/])+/g) || [] + ).slice(0, -1); + indirect.push( ...paths.map((p) => ({ name: p, diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java index c7d00a527c..b8ef776260 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java @@ -23,6 +23,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.cache.infinispan.entities.CachedGroup; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RoleUtils; import java.util.HashSet; @@ -284,4 +285,9 @@ public class GroupAdapter implements GroupModel { private GroupModel getGroupModel() { return cacheSession.getGroupDelegate().getGroupById(realm, cached.getId()); } + + @Override + public boolean escapeSlashesInGroupPath() { + return KeycloakModelUtils.escapeSlashesInGroupPath(keycloakSession); + } } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java index 1a9c884fe5..ff7e370e53 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java @@ -48,11 +48,13 @@ import static org.keycloak.utils.StreamsUtil.closing; */ public class GroupAdapter implements GroupModel , JpaModel { + protected final KeycloakSession session; protected GroupEntity group; protected EntityManager em; protected RealmModel realm; - public GroupAdapter(RealmModel realm, EntityManager em, GroupEntity group) { + public GroupAdapter(KeycloakSession session, RealmModel realm, EntityManager em, GroupEntity group) { + this.session = session; this.em = em; this.group = group; this.realm = realm; @@ -313,6 +315,8 @@ public class GroupAdapter implements GroupModel , JpaModel { return getId().hashCode(); } - - + @Override + public boolean escapeSlashesInGroupPath() { + return KeycloakModelUtils.escapeSlashesInGroupPath(session); + } } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaGroupProviderFactory.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaGroupProviderFactory.java index f01b20695a..a80980140e 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaGroupProviderFactory.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaGroupProviderFactory.java @@ -23,22 +23,27 @@ import org.keycloak.models.GroupProvider; import org.keycloak.models.GroupProviderFactory; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.provider.ProviderConfigProperty; import jakarta.persistence.EntityManager; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; import static org.keycloak.models.jpa.JpaRealmProviderFactory.PROVIDER_ID; import static org.keycloak.models.jpa.JpaRealmProviderFactory.PROVIDER_PRIORITY; +import org.keycloak.provider.ProviderConfigurationBuilder; public class JpaGroupProviderFactory implements GroupProviderFactory { private Set groupSearchableAttributes = null; + private boolean escapeSlashesInGroupPath; @Override public void init(Config.Scope config) { + escapeSlashesInGroupPath = config.getBoolean("escapeSlashesInGroupPath", GroupProvider.DEFAULT_ESCAPE_SLASHES); String[] searchableAttrsArr = config.getArray("searchableAttributes"); if (searchableAttrsArr == null) { String s = System.getProperty("keycloak.group.searchableAttributes"); @@ -77,4 +82,25 @@ public class JpaGroupProviderFactory implements GroupProviderFactory { return PROVIDER_PRIORITY; } + @Override + public boolean escapeSlashesInGroupPath() { + return escapeSlashesInGroupPath; + } + + @Override + public List getConfigMetadata() { + return ProviderConfigurationBuilder.create() + .property() + .name("escapeSlashesInGroupPath") + .helpText("If true slashes `/` in group names are escaped with the character `~` when converted to paths.") + .type("boolean") + .defaultValue(false) + .add() + .property() + .name("searchableAttributes") + .helpText("The list of attributes separated by comma that are allowed in client attribute searches.") + .type("string") + .add() + .build(); + } } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java index 50cb0d0c01..3fea3c0438 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -483,7 +483,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc GroupEntity groupEntity = em.find(GroupEntity.class, id); if (groupEntity == null) return null; if (!groupEntity.getRealm().equals(realm.getId())) return null; - GroupAdapter adapter = new GroupAdapter(realm, em, groupEntity); + GroupAdapter adapter = new GroupAdapter(session, realm, em, groupEntity); return adapter; } @@ -650,7 +650,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc Stream results = paginateQuery(query, firstResult, maxResults).getResultStream(); return closing(results - .map(g -> (GroupModel) new GroupAdapter(realm, em, g)) + .map(g -> (GroupModel) new GroupAdapter(session, realm, em, g)) .sorted(GroupModel.COMPARE_BY_NAME)); } @@ -733,7 +733,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc em.persist(groupEntity); em.flush(); - return new GroupAdapter(realm, em, groupEntity); + return new GroupAdapter(session, realm, em, groupEntity); } @Override @@ -1169,7 +1169,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc TypedQuery query = em.createQuery(queryBuilder); return closing(paginateQuery(query, firstResult, maxResults).getResultStream()) - .map(g -> new GroupAdapter(realm, em, g)); + .map(g -> new GroupAdapter(session, realm, em, g)); } @Override diff --git a/server-spi-private/src/main/java/org/keycloak/models/GroupProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/models/GroupProviderFactory.java index 7409a7acf8..7d0d3ac609 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/GroupProviderFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/models/GroupProviderFactory.java @@ -20,4 +20,8 @@ package org.keycloak.models; import org.keycloak.provider.ProviderFactory; public interface GroupProviderFactory extends ProviderFactory { + + default boolean escapeSlashesInGroupPath() { + return GroupProvider.DEFAULT_ESCAPE_SLASHES; + } } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java index bf704d43ed..d73444cafe 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java @@ -64,6 +64,7 @@ import java.security.KeyPair; import java.security.PrivateKey; import java.security.PublicKey; import java.security.cert.X509Certificate; +import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -73,8 +74,10 @@ import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; +import java.util.regex.Pattern; import org.keycloak.models.AccountRoles; +import org.keycloak.models.GroupProviderFactory; import org.keycloak.provider.Provider; import org.keycloak.provider.ProviderFactory; import org.keycloak.sessions.AuthenticationSessionModel; @@ -97,6 +100,7 @@ public final class KeycloakModelUtils { public static final String AUTH_TYPE_CLIENT_SECRET_JWT = "client-secret-jwt"; public static final String GROUP_PATH_SEPARATOR = "/"; + public static final String GROUP_PATH_ESCAPE = "~"; private static final char CLIENT_ROLE_SEPARATOR = '.'; private KeycloakModelUtils() { @@ -692,10 +696,22 @@ public final class KeycloakModelUtils { return segments; } + /** + * Helper to get from the session if group path slashes should be escaped or not. + * @param session The session + * @return true or false + */ + public static boolean escapeSlashesInGroupPath(KeycloakSession session) { + GroupProviderFactory fact = (GroupProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(GroupProvider.class); + return fact.escapeSlashesInGroupPath(); + } + /** * Finds group by path. Path is separated by '/' character. For example: /group/subgroup/subsubgroup *

* The method takes into consideration also groups with '/' in their name. For example: /group/sub/group/subgroup + * This method allows escaping of slashes for example: /parent\/group/child which + * is a two level path for ["parent/group", "child"]. * * @param session Keycloak session * @param realm The realm @@ -707,13 +723,7 @@ public final class KeycloakModelUtils { if (path == null) { return null; } - if (path.startsWith(GROUP_PATH_SEPARATOR)) { - path = path.substring(1); - } - if (path.endsWith(GROUP_PATH_SEPARATOR)) { - path = path.substring(0, path.length() - 1); - } - String[] split = path.split(GROUP_PATH_SEPARATOR); + String[] split = splitPath(path, escapeSlashesInGroupPath(session)); if (split.length == 0) return null; return getGroupModel(session.groups(), realm, null, split, 0); } @@ -752,22 +762,76 @@ public final class KeycloakModelUtils { return null; } - private static void buildGroupPath(StringBuilder sb, String groupName, GroupModel parent) { - if (parent != null) { - buildGroupPath(sb, parent.getName(), parent.getParent()); + /** + * Splits a group path than can be escaped for slashes. + * @param path The group path + * @param escapedSlashes true if slashes are escaped in the path + * @return + */ + public static String[] splitPath(String path, boolean escapedSlashes) { + if (path == null) { + return null; } - sb.append(GROUP_PATH_SEPARATOR).append(groupName); + if (path.startsWith(GROUP_PATH_SEPARATOR)) { + path = path.substring(1); + } + if (path.endsWith(GROUP_PATH_SEPARATOR)) { + path = path.substring(0, path.length() - 1); + } + // just split by slashed that are not escaped + return escapedSlashes + ? Arrays.stream(path.split("(? values) { + } + + @Override + public void removeAttribute(String name) { + } + + @Override + public String getFirstAttribute(String name) { + return null; + } + + @Override + public Stream getAttributeStream(String name) { + return null; + } + + @Override + public Map> getAttributes() {return null; + } + + @Override + public GroupModel getParent() { + return parent; + } + + @Override + public String getParentId() { + return parent.getId(); + } + + @Override + public Stream getSubGroupsStream() { + return Stream.empty(); + } + + @Override + public void setParent(GroupModel group) { + this.parent = group; + } + + @Override + public void addChild(GroupModel subGroup) { + } + + @Override + public void removeChild(GroupModel subGroup) { + } + + @Override + public Stream getRealmRoleMappingsStream() { + return Stream.empty(); + } + + @Override + public Stream getClientRoleMappingsStream(ClientModel app) { + return Stream.empty(); + } + + @Override + public boolean hasRole(RoleModel role) { + return false; + } + + @Override + public void grantRole(RoleModel role) { + } + + @Override + public Stream getRoleMappingsStream() { + return Stream.empty(); + } + + @Override + public void deleteRoleMapping(RoleModel role) { + } + + @Override + public boolean escapeSlashesInGroupPath() { + return escapeSlashes; + } + } } diff --git a/server-spi/src/main/java/org/keycloak/models/GroupModel.java b/server-spi/src/main/java/org/keycloak/models/GroupModel.java index 243f47b45f..14600eaa50 100755 --- a/server-spi/src/main/java/org/keycloak/models/GroupModel.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupModel.java @@ -173,4 +173,8 @@ public interface GroupModel extends RoleMapperModel { * @param subGroup */ void removeChild(GroupModel subGroup); + + default boolean escapeSlashesInGroupPath() { + return GroupProvider.DEFAULT_ESCAPE_SLASHES; + } } diff --git a/server-spi/src/main/java/org/keycloak/models/GroupProvider.java b/server-spi/src/main/java/org/keycloak/models/GroupProvider.java index a3880a9ce6..e6ed5835b8 100644 --- a/server-spi/src/main/java/org/keycloak/models/GroupProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupProvider.java @@ -30,6 +30,8 @@ import java.util.stream.Stream; */ public interface GroupProvider extends Provider, GroupLookupProvider { + static boolean DEFAULT_ESCAPE_SLASHES = false; + /** * Returns groups for the given realm. * diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java index c061612968..3f240cbfe4 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java @@ -1084,8 +1084,7 @@ public class RealmAdminResource { @Produces(MediaType.APPLICATION_JSON) @Tag(name = KeycloakOpenAPI.Admin.Tags.REALMS_ADMIN) @Operation() - public GroupRepresentation getGroupByPath(@PathParam("path") List pathSegments) { - String[] path = pathSegments.stream().map(PathSegment::getPath).toArray(String[]::new); + public GroupRepresentation getGroupByPath(@PathParam("path") String path) { GroupModel found = KeycloakModelUtils.findGroupByPath(session, realm, path); if (found == null) { throw new NotFoundException("Group path does not exist"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java index 1a5587b2f8..d178846f97 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java @@ -17,11 +17,16 @@ package org.keycloak.testsuite.admin.group; +import jakarta.ws.rs.core.Response; +import java.util.*; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; +import org.keycloak.admin.client.resource.ProtocolMappersResource; import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.models.GroupProvider; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.ProtocolMapperUtils; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.mappers.GroupMembershipMapper; @@ -30,11 +35,12 @@ import org.keycloak.protocol.oidc.mappers.UserAttributeMapper; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; - -import java.util.*; +import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.util.ProtocolMapperUtil; /** * @author Marko Strukelj @@ -130,4 +136,41 @@ public class GroupMappersTest extends AbstractGroupTest { Assert.assertEquals("true", token.getOtherClaims().get("level2Attribute")); } } + + @Test + public void testGroupMappersWithSlash() throws Exception { + RealmResource realm = adminClient.realms().realm("test"); + GroupRepresentation topGroup = realm.getGroupByPath("/topGroup"); + Assert.assertNotNull(topGroup); + GroupRepresentation childSlash = new GroupRepresentation(); + childSlash.setName("child/slash"); + try (Response response = realm.groups().group(topGroup.getId()).subGroup(childSlash)) { + Assert.assertEquals(Response.Status.CREATED.getStatusCode(), response.getStatus()); + childSlash.setId(ApiUtil.getCreatedId(response)); + } + List users = realm.users().search("level2GroupUser", true); + Assert.assertEquals(1, users.size()); + UserRepresentation user = users.iterator().next(); + realm.users().get(user.getId()).joinGroup(childSlash.getId()); + + ProtocolMappersResource protocolMappers = ApiUtil.findClientResourceByClientId(realm, "test-app").getProtocolMappers(); + ProtocolMapperRepresentation groupsMapper = ProtocolMapperUtil.getMapperByNameAndProtocol( + protocolMappers, OIDCLoginProtocol.LOGIN_PROTOCOL, "groups"); + groupsMapper.getConfig().put("full.path", Boolean.TRUE.toString()); + protocolMappers.update(groupsMapper.getId(), groupsMapper); + + try { + AccessToken token = login(user.getUsername(), "test-app", "password", user.getId()); + Assert.assertNotNull(token.getOtherClaims().get("groups")); + Map> groups = (Map>) token.getOtherClaims().get("groups"); + MatcherAssert.assertThat(groups.get("groups"), Matchers.containsInAnyOrder( + KeycloakModelUtils.buildGroupPath(GroupProvider.DEFAULT_ESCAPE_SLASHES, "topGroup", "level2group"), + KeycloakModelUtils.buildGroupPath(GroupProvider.DEFAULT_ESCAPE_SLASHES, "topGroup", "child/slash"))); + } finally { + realm.users().get(user.getId()).leaveGroup(childSlash.getId()); + realm.groups().group(childSlash.getId()).remove(); + groupsMapper.getConfig().remove("full.path"); + protocolMappers.update(groupsMapper.getId(), groupsMapper); + } + } } 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 5aa9b6b381..17879f683a 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 @@ -31,6 +31,7 @@ import org.keycloak.admin.client.resource.UsersResource; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.models.Constants; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.CredentialRepresentation; @@ -87,6 +88,7 @@ import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import static org.hamcrest.MatcherAssert.assertThat; +import org.keycloak.models.GroupProvider; import static org.keycloak.testsuite.Assert.assertNames; import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot; @@ -1370,14 +1372,13 @@ public class GroupTest extends AbstractGroupTest { assertTrue(searchResultGroups.isEmpty()); } - @Test - public void testGroupsWithSpaces() { + public void testParentAndChildGroup(String parentName, String childName) { RealmResource realm = adminClient.realms().realm("test"); GroupRepresentation parentGroup = new GroupRepresentation(); - parentGroup.setName("parent space"); + parentGroup.setName(parentName); parentGroup = createGroup(realm, parentGroup); GroupRepresentation childGroup = new GroupRepresentation(); - childGroup.setName("child space"); + childGroup.setName(childName); try (Response response = realm.groups().group(parentGroup.getId()).subGroup(childGroup)) { assertEquals(201, response.getStatus()); // created status childGroup.setId(ApiUtil.getCreatedId(response)); @@ -1385,20 +1386,28 @@ public class GroupTest extends AbstractGroupTest { assertAdminEvents.assertEvent(testRealmId, OperationType.CREATE, AdminEventPaths.groupSubgroupsPath(parentGroup.getId()), childGroup, ResourceType.GROUP); - List groupsFound = realm.groups().groups("parent space", true, 0, 1, true); + List groupsFound = realm.groups().groups(parentGroup.getName(), true, 0, 1, true); Assert.assertEquals(1, groupsFound.size()); Assert.assertEquals(parentGroup.getId(), groupsFound.iterator().next().getId()); Assert.assertEquals(0, groupsFound.iterator().next().getSubGroups().size()); - groupsFound = realm.groups().groups("child space", true, 0, 1, true); + parentGroup = groupsFound.iterator().next(); + Assert.assertEquals(KeycloakModelUtils.buildGroupPath(GroupProvider.DEFAULT_ESCAPE_SLASHES, parentName), + parentGroup.getPath()); + + groupsFound = realm.groups().groups(childGroup.getName(), true, 0, 1, true); Assert.assertEquals(1, groupsFound.size()); Assert.assertEquals(parentGroup.getId(), groupsFound.iterator().next().getId()); Assert.assertEquals(1, groupsFound.iterator().next().getSubGroups().size()); Assert.assertEquals(childGroup.getId(), groupsFound.iterator().next().getSubGroups().iterator().next().getId()); + childGroup = groupsFound.iterator().next().getSubGroups().iterator().next(); + Assert.assertEquals(KeycloakModelUtils.normalizeGroupPath( + KeycloakModelUtils.buildGroupPath(GroupProvider.DEFAULT_ESCAPE_SLASHES, parentName, childName)), + childGroup.getPath()); - GroupRepresentation groupFound = realm.getGroupByPath(parentGroup.getName()); + GroupRepresentation groupFound = realm.getGroupByPath(parentGroup.getPath()); Assert.assertNotNull(groupFound); Assert.assertEquals(parentGroup.getId(), groupFound.getId()); - groupFound = realm.getGroupByPath("/" + parentGroup.getName() + "/" + childGroup.getName()); + groupFound = realm.getGroupByPath(childGroup.getPath()); Assert.assertNotNull(groupFound); Assert.assertEquals(childGroup.getId(), groupFound.getId()); @@ -1408,6 +1417,16 @@ public class GroupTest extends AbstractGroupTest { assertAdminEvents.assertEvent(testRealmId, OperationType.DELETE, AdminEventPaths.groupPath(parentGroup.getId()), ResourceType.GROUP); } + @Test + public void testGroupsWithSpaces() { + testParentAndChildGroup("parent space", "child space"); + } + + @Test + public void testGroupsWithSlashes() { + testParentAndChildGroup("parent/slash", "child/slash"); + } + /** * 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