diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProviderFactory.java index 2fd3f5b9cd..77e7e3247b 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProviderFactory.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProviderFactory.java @@ -164,7 +164,7 @@ public class GroupPolicyProviderFactory implements PolicyProviderFactory topLevelGroups = authorization.getRealm().getTopLevelGroupsStream().collect(Collectors.toList()); + List topLevelGroups = authorization.getKeycloakSession().groups().getTopLevelGroupsStream(authorization.getRealm()).collect(Collectors.toList()); for (GroupPolicyRepresentation.GroupDefinition definition : groups) { GroupModel group = null; diff --git a/core/src/main/java/org/keycloak/representations/idm/GroupRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/GroupRepresentation.java index 9c96970970..e83fab0ecc 100755 --- a/core/src/main/java/org/keycloak/representations/idm/GroupRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/GroupRepresentation.java @@ -17,23 +17,35 @@ package org.keycloak.representations.idm; +import com.fasterxml.jackson.annotation.JsonInclude; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; /** * @author Bill Burke * @version $Revision: 1 $ */ +@JsonInclude(JsonInclude.Include.NON_EMPTY) public class GroupRepresentation { + // For an individual group these are the sufficient minimum fields + // to identify a group and operate on it in a basic way protected String id; protected String name; protected String path; + protected String parentId; + protected Long subGroupCount; + // For navigating a hierarchy of groups, we can also include a minimum representation of subGroups + // These aren't populated by default and are only included as-needed + protected List subGroups; protected Map> attributes; protected List realmRoles; protected Map> clientRoles; - protected List subGroups; + private Map access; public String getId() { @@ -60,6 +72,22 @@ public class GroupRepresentation { this.path = path; } + public String getParentId() { + return parentId; + } + + public void setParentId(String parentId) { + this.parentId = parentId; + } + + public Long getSubGroupCount() { + return subGroupCount; + } + + public void setSubGroupCount(Long subGroupCount) { + this.subGroupCount = subGroupCount; + } + public List getRealmRoles() { return realmRoles; } @@ -92,6 +120,9 @@ public class GroupRepresentation { } public List getSubGroups() { + if(subGroups == null) { + subGroups = new ArrayList<>(); + } return subGroups; } @@ -106,4 +137,49 @@ public class GroupRepresentation { public void setAccess(Map access) { this.access = access; } + + public void merge(GroupRepresentation g) { + merge(this, g); + } + + private void merge(GroupRepresentation g1, GroupRepresentation g2) { + if(g1.equals(g2)) { + Map g1Children = g1.getSubGroups().stream().collect(Collectors.toMap(GroupRepresentation::getId, g -> g)); + Map g2Children = g2.getSubGroups().stream().collect(Collectors.toMap(GroupRepresentation::getId, g -> g)); + + g2Children.forEach((key, value) -> { + if (g1Children.containsKey(key)) { + merge(g1Children.get(key), value); + } else { + g1Children.put(key, value); + } + }); + g1.setSubGroups(new ArrayList<>(g1Children.values())); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + GroupRepresentation that = (GroupRepresentation) o; + boolean isEqual = Objects.equals(id, that.id) && Objects.equals(parentId, that.parentId); + if(isEqual) { + return true; + } else { + return Objects.equals(name, that.name) && Objects.equals(path, that.path); + } + } + + @Override + public int hashCode() { + if(id == null) { + return Objects.hash(name, path); + } + return Objects.hash(id, parentId); + } } diff --git a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc index cbc4ea218a..6cdc4a83d7 100644 --- a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc +++ b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc @@ -107,3 +107,31 @@ bin/kc.sh start --db postgres --db-username keycloak --db-url "jdbc:postgresql:/ The form action `RegistrationProfile` (displayed in the UI of authentication flows as `Profile Validation`) was removed from the codebase and also from all authentication flows. By default, it was in the built-in registration flow of every realm. The validation of user attributes as well as creation of the user including all that user's attributes is handled by `RegistrationUserCreation` form action and hence `RegistrationProfile` is not needed anymore. There is usually no further action needed in relation to this change, unless you used `RegistrationProfile` class in your own providers. + += Deprecated methods from data providers and models + +* `RealmModel#getTopLevelGroupsStream()` and overloaded methods are now deprecated + += `GroupProvider` changes + +A new method has been added to allow for searching and paging through top level groups. +If you implement this interface you will need to implement the following method: +[source,java] +---- +Stream getTopLevelGroupsStream(RealmModel realm, + String search, + Boolean exact, + Integer firstResult, + Integer maxResults) +---- + += `GroupRepresentation` changes + +* new field `subGroupCount` added to inform client how many subgroups are on any given group +* `subGroups` list is now only populated on queries that request hierarchy data + * This field is populated from the "bottom up" so can't be relied on for getting all subgroups for a group. Use a `GroupProvider` or request the subgroups from `GET {keycloak server}/realms/{realm}/groups/{group_id}/children` + += New endpoint for Group Admin API + +Endpoint `GET {keycloak server}/realms/{realm}/groups/{group_id}/children` added as a way to get subgroups of specific groups that support pagination + diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java index 04ede149e8..b72f6a658a 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java @@ -814,7 +814,8 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements if (parentGroup == null) { parentGroup = getKcGroupsPathGroup(realm); } - return parentGroup == null ? realm.getTopLevelGroupsStream() : parentGroup.getSubGroupsStream(); + return parentGroup == null ? session.groups().getTopLevelGroupsStream(realm) : + parentGroup.getSubGroupsStream(); } /** diff --git a/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/GroupResource.java b/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/GroupResource.java index 1075f0486d..00ee40e3f5 100755 --- a/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/GroupResource.java +++ b/integration/admin-client-jee/src/main/java/org/keycloak/admin/client/resource/GroupResource.java @@ -85,6 +85,18 @@ public interface GroupResource { @DELETE void remove(); + /** + * Get the paginated list of subgroups belonging to this group + * + * @param first + * @param max + * @param full + */ + @GET + @Path("children") + @Produces(jakarta.ws.rs.core.MediaType.APPLICATION_JSON) + @Consumes(jakarta.ws.rs.core.MediaType.APPLICATION_JSON) + List getSubGroups(@QueryParam("first") Integer first, @QueryParam("max") Integer max, @QueryParam("briefRepresentation") Boolean briefRepresentation); /** * Set or create child. This will just set the parent if it exists. Create it and set the parent diff --git a/js/apps/admin-ui/cypress/e2e/sessions_test.spec.ts b/js/apps/admin-ui/cypress/e2e/sessions_test.spec.ts index 19705897ca..ee23e4ae78 100644 --- a/js/apps/admin-ui/cypress/e2e/sessions_test.spec.ts +++ b/js/apps/admin-ui/cypress/e2e/sessions_test.spec.ts @@ -3,15 +3,15 @@ import SidebarPage from "../support/pages/admin-ui/SidebarPage"; import SessionsPage from "../support/pages/admin-ui/manage/sessions/SessionsPage"; import CommonPage from "../support/pages/CommonPage"; import ListingPage from "../support/pages/admin-ui/ListingPage"; -import GroupPage from "../support/pages/admin-ui/manage/groups/GroupPage"; import { keycloakBefore } from "../support/util/keycloak_hooks"; +import PageObject from "../support/pages/admin-ui/components/PageObject"; const loginPage = new LoginPage(); const sidebarPage = new SidebarPage(); const sessionsPage = new SessionsPage(); const commonPage = new CommonPage(); const listingPage = new ListingPage(); -const groupPage = new GroupPage(); +const page = new PageObject(); describe("Sessions test", () => { const admin = "admin"; @@ -42,12 +42,12 @@ describe("Sessions test", () => { it("search existing session", () => { listingPage.searchItem(admin, false); listingPage.itemExist(admin, true); - groupPage.assertNoSearchResultsMessageExist(false); + page.assertEmptyStateExist(false); }); it("search non-existant session", () => { listingPage.searchItem("non-existant-session", false); - groupPage.assertNoSearchResultsMessageExist(true); + page.assertEmptyStateExist(true); }); }); diff --git a/js/apps/admin-ui/cypress/support/pages/admin-ui/components/PageObject.ts b/js/apps/admin-ui/cypress/support/pages/admin-ui/components/PageObject.ts index de4dc7691f..43079dd45f 100644 --- a/js/apps/admin-ui/cypress/support/pages/admin-ui/components/PageObject.ts +++ b/js/apps/admin-ui/cypress/support/pages/admin-ui/components/PageObject.ts @@ -329,7 +329,7 @@ export default class PageObject { return this; } - protected assertEmptyStateExist(exist: boolean) { + assertEmptyStateExist(exist: boolean) { if (exist) { cy.get(this.#emptyStateDiv).should("exist").should("be.visible"); } else { diff --git a/js/apps/admin-ui/cypress/support/pages/admin-ui/manage/groups/GroupPage.ts b/js/apps/admin-ui/cypress/support/pages/admin-ui/manage/groups/GroupPage.ts index 5dae7b544e..8813b54c32 100644 --- a/js/apps/admin-ui/cypress/support/pages/admin-ui/manage/groups/GroupPage.ts +++ b/js/apps/admin-ui/cypress/support/pages/admin-ui/manage/groups/GroupPage.ts @@ -17,7 +17,7 @@ export default class GroupPage extends PageObject { protected actionDrpDwnButton = "action-dropdown"; #searchField = "[data-testid='group-search']"; - public openCreateGroupModal(emptyState: boolean) { + openCreateGroupModal(emptyState: boolean) { if (emptyState) { cy.findByTestId(this.createGroupEmptyStateBtn).click(); } else { @@ -26,7 +26,7 @@ export default class GroupPage extends PageObject { return this; } - public createGroup(groupName: string, emptyState: boolean) { + createGroup(groupName: string, emptyState: boolean) { this.openCreateGroupModal(emptyState); groupModal .assertCreateGroupModalVisible(true) @@ -42,12 +42,23 @@ export default class GroupPage extends PageObject { return this; } - protected search(searchField: string, searchValue: string, wait: boolean) { + protected search( + searchField: string, + searchValue: string, + wait: boolean, + exact = true, + ) { if (wait) { const searchUrl = `/admin/realms/master/**/*${searchValue}*`; cy.intercept(searchUrl).as("search"); } + if (exact) { + cy.findByTestId("exact-search").check(); + } else { + cy.findByTestId("exact-search").uncheck(); + } + cy.get(searchField + " input").clear(); if (searchValue) { cy.get(searchField + " input").type(searchValue); @@ -62,26 +73,26 @@ export default class GroupPage extends PageObject { } } - public goToGroupChildGroupsTab(groupName: string) { + goToGroupChildGroupsTab(groupName: string) { listingPage.goToItemDetails(groupName); cy.intercept("GET", "*/admin/realms/master/groups/*").as("get"); sidebarPage.waitForPageLoad(); return this; } - public selectGroupItemCheckbox(items: string[]) { + selectGroupItemCheckbox(items: string[]) { for (const item of items) { listingPage.clickItemCheckbox(item); } return this; } - public selectGroupItemCheckboxAllRows() { + selectGroupItemCheckboxAllRows() { listingPage.clickTableHeaderItemCheckboxAllRows(); return this; } - public deleteSelectedGroups(confirmModal = true) { + deleteSelectedGroups(confirmModal = true) { this.clickToolbarAction("Delete"); if (confirmModal) { groupModal.confirmModal(); @@ -89,12 +100,12 @@ export default class GroupPage extends PageObject { return this; } - public showDeleteSelectedGroupsDialog() { + showDeleteSelectedGroupsDialog() { this.clickToolbarAction("Delete"); return this; } - public deleteGroupItem(groupName: string, confirmModal = true) { + deleteGroupItem(groupName: string, confirmModal = true) { listingPage.deleteItem(groupName); if (confirmModal) { groupModal.confirmModal(); @@ -102,10 +113,7 @@ export default class GroupPage extends PageObject { return this; } - public moveGroupItemAction( - groupName: string, - destinationGroupName: string[], - ) { + moveGroupItemAction(groupName: string, destinationGroupName: string[]) { listingPage.clickRowDetails(groupName); listingPage.clickDetailMenu("Move to"); moveGroupModal @@ -124,66 +132,68 @@ export default class GroupPage extends PageObject { return this; } - public clickBreadcrumbItem(groupName: string) { + clickBreadcrumbItem(groupName: string) { super.clickBreadcrumbItem(groupName); return this; } - public assertGroupItemExist(groupName: string, exist: boolean) { + assertGroupItemExist(groupName: string, exist: boolean) { listingPage.itemExist(groupName, exist); return this; } - public assertNoGroupsInThisRealmEmptyStateMessageExist(exist: boolean) { + assertNoGroupsInThisRealmEmptyStateMessageExist(exist: boolean) { this.assertEmptyStateExist(exist); return this; } - public assertGroupItemsEqual(number: number) { + assertGroupItemsEqual(number: number) { listingPage.itemsEqualTo(number); return this; } - public assertNoSearchResultsMessageExist(exist: boolean) { - super.assertEmptyStateExist(exist); + assertNoSearchResultsMessageExist(exist: boolean) { + if (!exist) { + cy.get("keycloak_groups_treeview").should("be.visible"); + } else { + cy.get("keycloak_groups_treeview").should("not.exist"); + } return this; } - public assertNotificationGroupDeleted() { + assertNotificationGroupDeleted() { masthead.checkNotificationMessage("Group deleted"); return this; } - public assertNotificationGroupsDeleted() { + assertNotificationGroupsDeleted() { masthead.checkNotificationMessage("Groups deleted"); return this; } - public assertNotificationGroupCreated() { + assertNotificationGroupCreated() { masthead.checkNotificationMessage("Group created"); return this; } - public assertNotificationGroupMoved() { + assertNotificationGroupMoved() { masthead.checkNotificationMessage("Group moved"); return this; } - public assertNotificationGroupUpdated() { + assertNotificationGroupUpdated() { masthead.checkNotificationMessage("Group updated"); return this; } - public assertNotificationCouldNotCreateGroupWithEmptyName() { + assertNotificationCouldNotCreateGroupWithEmptyName() { masthead.checkNotificationMessage( "Could not create group Group name is missing", ); return this; } - public assertNotificationCouldNotCreateGroupWithDuplicatedName( - groupName: string, - ) { + assertNotificationCouldNotCreateGroupWithDuplicatedName(groupName: string) { masthead.checkNotificationMessage( "Could not create group Top level group named '" + groupName + @@ -192,7 +202,7 @@ export default class GroupPage extends PageObject { return this; } - public goToGroupActions(groupName: string) { + goToGroupActions(groupName: string) { listingPage.clickRowDetails(groupName); return this; diff --git a/js/apps/admin-ui/src/components/group/GroupPickerDialog.tsx b/js/apps/admin-ui/src/components/group/GroupPickerDialog.tsx index 087959e589..514ae40b5b 100644 --- a/js/apps/admin-ui/src/components/group/GroupPickerDialog.tsx +++ b/js/apps/admin-ui/src/components/group/GroupPickerDialog.tsx @@ -1,4 +1,8 @@ import type GroupRepresentation from "@keycloak/keycloak-admin-client/lib/defs/groupRepresentation"; +import { + GroupQuery, + SubGroupQuery, +} from "@keycloak/keycloak-admin-client/lib/resources/groups"; import { Breadcrumb, BreadcrumbItem, @@ -17,7 +21,6 @@ import { AngleRightIcon } from "@patternfly/react-icons"; import { Fragment, useState } from "react"; import { useTranslation } from "react-i18next"; import { adminClient } from "../../admin-client"; -import { fetchAdminUI } from "../../context/auth/admin-ui-endpoint"; import { useFetch } from "../../utils/useFetch"; import { ListEmptyState } from "../list-empty-state/ListEmptyState"; import { PaginatingTableToolbar } from "../table-toolbar/PaginatingTableToolbar"; @@ -73,24 +76,31 @@ export const GroupPickerDialog = ({ let group; let groups; let existingUserGroups; + if (!groupId) { - groups = await fetchAdminUI( - "ui-ext/groups", - Object.assign( - { - first: `${first}`, - max: `${max + 1}`, - global: "false", - }, - isSearching ? { search: filter, global: "true" } : null, - ), - ); - } else if (!navigation.map(({ id }) => id).includes(groupId)) { - group = await adminClient.groups.findOne({ id: groupId }); - if (!group) { - throw new Error(t("notFound")); + const args: GroupQuery = { + first: first, + max: max + 1, + }; + if (isSearching) { + args.search = filter; + } + groups = await adminClient.groups.find(args); + } else { + if (!navigation.map(({ id }) => id).includes(groupId)) { + group = await adminClient.groups.findOne({ id: groupId }); + if (!group) { + throw new Error(t("common:notFound")); + } + } + if (group?.id) { + const args: SubGroupQuery = { + first: first, + max: max + 1, + parentId: group.id, + }; + groups = await adminClient.groups.listSubGroups(args); } - groups = group.subGroups!; } if (id) { diff --git a/js/apps/admin-ui/src/groups/GroupTable.tsx b/js/apps/admin-ui/src/groups/GroupTable.tsx index a1914d36d6..c2f7204707 100644 --- a/js/apps/admin-ui/src/groups/GroupTable.tsx +++ b/js/apps/admin-ui/src/groups/GroupTable.tsx @@ -1,4 +1,8 @@ import type GroupRepresentation from "@keycloak/keycloak-admin-client/lib/defs/groupRepresentation"; +import { + GroupQuery, + SubGroupQuery, +} from "@keycloak/keycloak-admin-client/lib/resources/groups"; import { SearchInput, ToolbarItem } from "@patternfly/react-core"; import { useState } from "react"; import { useTranslation } from "react-i18next"; @@ -7,7 +11,6 @@ import { Link, useLocation } from "react-router-dom"; import { ListEmptyState } from "../components/list-empty-state/ListEmptyState"; import { KeycloakDataTable } from "../components/table-toolbar/KeycloakDataTable"; import { useAccess } from "../context/access/Access"; -import { fetchAdminUI } from "../context/auth/admin-ui-endpoint"; import useToggle from "../utils/useToggle"; import { GroupsModal } from "./GroupsModal"; import { useSubGroups } from "./SubGroupsContext"; @@ -15,6 +18,7 @@ import { DeleteGroup } from "./components/DeleteGroup"; import { GroupToolbar } from "./components/GroupToolbar"; import { MoveDialog } from "./components/MoveDialog"; import { getLastId } from "./groupIdUtils"; +import { adminClient } from "../admin-client"; type GroupTableProps = { refresh: () => void; @@ -47,23 +51,21 @@ export const GroupTable = ({ const isManager = hasAccess("manage-users") || currentGroup()?.access?.manage; const loader = async (first?: number, max?: number) => { - const params: Record = { - search: search || "", - first: first?.toString() || "", - max: max?.toString() || "", - }; - let groupsData = undefined; if (id) { - groupsData = await fetchAdminUI( - "ui-ext/groups/subgroup", - { ...params, id }, - ); + const args: SubGroupQuery = { + first: first, + max: max, + parentId: id, + }; + groupsData = await adminClient.groups.listSubGroups(args); } else { - groupsData = await fetchAdminUI("ui-ext/groups", { - ...params, - global: "false", - }); + const args: GroupQuery = { + search: search || "", + first: first || undefined, + max: max || undefined, + }; + groupsData = await adminClient.groups.find(args); } return groupsData; diff --git a/js/apps/admin-ui/src/groups/GroupsSection.tsx b/js/apps/admin-ui/src/groups/GroupsSection.tsx index 38205e6250..ec1e74dcd9 100644 --- a/js/apps/admin-ui/src/groups/GroupsSection.tsx +++ b/js/apps/admin-ui/src/groups/GroupsSection.tsx @@ -23,7 +23,7 @@ import { GroupBreadCrumbs } from "../components/bread-crumb/GroupBreadCrumbs"; import { PermissionsTab } from "../components/permission-tab/PermissionTab"; import { ViewHeader } from "../components/view-header/ViewHeader"; import { useAccess } from "../context/access/Access"; -import { fetchAdminUI } from "../context/auth/admin-ui-endpoint"; +import { adminClient } from "../admin-client"; import { useRealm } from "../context/realm-context/RealmContext"; import helpUrls from "../help-urls"; import { useFetch } from "../utils/useFetch"; @@ -84,12 +84,12 @@ export default function GroupsSection() { if (isNavigationStateInValid) { const groups: GroupRepresentation[] = []; for (const i of ids!) { - const group = - i !== "search" - ? await fetchAdminUI( - "ui-ext/groups/" + i, - ) - : { name: t("searchGroups"), id: "search" }; + let group = undefined; + if (i !== "search") { + group = await adminClient.groups.findOne({ id: i }); + } else { + group = { name: t("searchGroups"), id: "search" }; + } if (group) { groups.push(group); } else { diff --git a/js/apps/admin-ui/src/groups/Members.tsx b/js/apps/admin-ui/src/groups/Members.tsx index 01cba9a1dc..b6e148e4df 100644 --- a/js/apps/admin-ui/src/groups/Members.tsx +++ b/js/apps/admin-ui/src/groups/Members.tsx @@ -1,5 +1,6 @@ import type GroupRepresentation from "@keycloak/keycloak-admin-client/lib/defs/groupRepresentation"; import type UserRepresentation from "@keycloak/keycloak-admin-client/lib/defs/userRepresentation"; +import { SubGroupQuery } from "@keycloak/keycloak-admin-client/lib/resources/groups"; import { AlertVariant, Button, @@ -41,7 +42,7 @@ const MemberOfRenderer = (member: MembersOf) => { <> {member.membership.map((group, index) => ( <> - + {member.membership[index + 1] ? ", " : ""} ))} @@ -87,30 +88,51 @@ export const Members = () => { const getMembership = async (id: string) => await adminClient.users.listGroups({ id: id! }); - const getSubGroups = (groups: GroupRepresentation[]) => { - let subGroups: GroupRepresentation[] = []; - for (const group of groups!) { - subGroups.push(group); - const subs = getSubGroups(group.subGroups!); - subGroups = subGroups.concat(subs); + // this queries the subgroups using the new search paradigm but doesn't + // account for pagination and therefore isn't going to scale well + const getSubGroups = async (groupId?: string, count = 0) => { + let nestedGroups: GroupRepresentation[] = []; + if (!count || !groupId) { + return nestedGroups; } - return subGroups; + const args: SubGroupQuery = { + parentId: groupId, + first: 0, + max: count, + }; + const subGroups: GroupRepresentation[] = + await adminClient.groups.listSubGroups(args); + nestedGroups = nestedGroups.concat(subGroups); + + await Promise.all( + subGroups.map((g) => getSubGroups(g.id, g.subGroupCount)), + ).then((values: GroupRepresentation[][]) => { + values.forEach((groups) => (nestedGroups = nestedGroups.concat(groups))); + }); + return nestedGroups; }; const loader = async (first?: number, max?: number) => { + if (!id) { + return []; + } + let members = await adminClient.groups.listMembers({ id: id!, first, max, }); - if (includeSubGroup) { - const subGroups = getSubGroups(currentGroup?.subGroups || []); - for (const group of subGroups) { - members = members.concat( - await adminClient.groups.listMembers({ id: group.id! }), - ); - } + if (includeSubGroup && currentGroup?.subGroupCount && currentGroup.id) { + const subGroups = await getSubGroups( + currentGroup.id, + currentGroup.subGroupCount, + ); + await Promise.all( + subGroups.map((g) => adminClient.groups.listMembers({ id: g.id! })), + ).then((values: UserRepresentation[][]) => { + values.forEach((users) => (members = members.concat(users))); + }); members = uniqBy(members, (member) => member.username); } diff --git a/js/apps/admin-ui/src/groups/components/GroupTree.tsx b/js/apps/admin-ui/src/groups/components/GroupTree.tsx index 8a9ddc77bd..107bdeaa71 100644 --- a/js/apps/admin-ui/src/groups/components/GroupTree.tsx +++ b/js/apps/admin-ui/src/groups/components/GroupTree.tsx @@ -183,7 +183,7 @@ export const GroupTree = ({ useFetch( async () => { const groups = await fetchAdminUI( - "ui-ext/groups", + "groups", Object.assign( { first: `${first}`, @@ -197,9 +197,8 @@ export const GroupTree = ({ let subGroups: GroupRepresentation[] = []; if (activeItem) { subGroups = await fetchAdminUI( - "ui-ext/groups/subgroup", + `groups/${activeItem.id}/children`, { - id: activeItem.id!, first: `${firstSub}`, max: `${SUBGROUP_COUNT}`, }, diff --git a/js/libs/keycloak-admin-client/README.md b/js/libs/keycloak-admin-client/README.md index 24100331aa..5ec42492e5 100644 --- a/js/libs/keycloak-admin-client/README.md +++ b/js/libs/keycloak-admin-client/README.md @@ -220,6 +220,7 @@ Demo code: https://github.com/keycloak/keycloak/blob/main/js/libs/keycloak-admin - Count (`GET /{realm}/groups/count`) - List members (`GET /{realm}/groups/{id}/members`) - Set or create child (`POST /{realm}/groups/{id}/children`) +- Get children (`GET /{realm}/groups/{id}/children`) ### Group role-mapping diff --git a/js/libs/keycloak-admin-client/src/defs/groupRepresentation.ts b/js/libs/keycloak-admin-client/src/defs/groupRepresentation.ts index 7ebee981c3..691b35e857 100644 --- a/js/libs/keycloak-admin-client/src/defs/groupRepresentation.ts +++ b/js/libs/keycloak-admin-client/src/defs/groupRepresentation.ts @@ -6,6 +6,7 @@ export default interface GroupRepresentation { id?: string; name?: string; path?: string; + subGroupCount?: number; subGroups?: GroupRepresentation[]; // optional in response diff --git a/js/libs/keycloak-admin-client/src/resources/groups.ts b/js/libs/keycloak-admin-client/src/resources/groups.ts index 4587fc2c55..7d1655a76c 100644 --- a/js/libs/keycloak-admin-client/src/resources/groups.ts +++ b/js/libs/keycloak-admin-client/src/resources/groups.ts @@ -7,13 +7,26 @@ import type { RoleMappingPayload } from "../defs/roleRepresentation.js"; import type UserRepresentation from "../defs/userRepresentation.js"; import Resource from "./resource.js"; -export interface GroupQuery { +interface Query { + search?: string; + exact?: boolean; +} + +interface PaginatedQuery { first?: number; max?: number; - search?: string; +} + +interface SummarizedQuery { briefRepresentation?: boolean; } +export type GroupQuery = Query & PaginatedQuery & SummarizedQuery; +export type SubGroupQuery = PaginatedQuery & + SummarizedQuery & { + parentId: string; + }; + export interface GroupCountQuery { search?: string; top?: boolean; @@ -22,6 +35,7 @@ export interface GroupCountQuery { export class Groups extends Resource<{ realm?: string }> { public find = this.makeRequest({ method: "GET", + queryParamKeys: ["search", "exact", "briefRepresentation", "first", "max"], }); public create = this.makeRequest({ @@ -112,6 +126,19 @@ export class Groups extends Resource<{ realm?: string }> { urlParamKeys: ["id"], }); + /** + * Finds all subgroups on the specified parent group matching the provided parameters. + */ + public listSubGroups = this.makeRequest( + { + method: "GET", + path: "/{parentId}/children", + urlParamKeys: ["parentId"], + queryParamKeys: ["first", "max", "briefRepresentation"], + catchNotFound: true, + }, + ); + /** * Members */ diff --git a/js/libs/keycloak-admin-client/test/groups.spec.ts b/js/libs/keycloak-admin-client/test/groups.spec.ts index 5d91e620f6..92ed4c0c4e 100644 --- a/js/libs/keycloak-admin-client/test/groups.spec.ts +++ b/js/libs/keycloak-admin-client/test/groups.spec.ts @@ -6,6 +6,7 @@ import type ClientRepresentation from "../src/defs/clientRepresentation.js"; import type GroupRepresentation from "../src/defs/groupRepresentation.js"; import type RoleRepresentation from "../src/defs/roleRepresentation.js"; import { credentials } from "./constants.js"; +import { SubGroupQuery } from "../src/resources/groups.js"; const expect = chai.expect; @@ -93,11 +94,20 @@ describe("Groups", () => { const group = (await kcAdminClient.groups.findOne({ id: groupId!, }))!; - expect(group.subGroups![0]).to.deep.include({ - id: childGroup.id, - name: groupName, - path: `/${group.name}/${groupName}`, - }); + expect(group).to.be.ok; + }); + + it("list subgroups", async () => { + if (currentGroup.id) { + const args: SubGroupQuery = { + parentId: currentGroup!.id, + first: 0, + max: 10, + briefRepresentation: false, + }; + const groups = await kcAdminClient.groups.listSubGroups(args); + expect(groups.length).to.equal(1); + } }); /** 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 0493b668d2..b0719131e1 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 @@ -237,7 +237,29 @@ public class GroupAdapter implements GroupModel { return subGroups.stream().sorted(GroupModel.COMPARE_BY_NAME); } + @Override + public Stream getSubGroupsStream(String search, Integer firstResult, Integer maxResults) { + if (isUpdated()) return updated.getSubGroupsStream(search, firstResult, maxResults); + return modelSupplier.get().getSubGroupsStream(search, firstResult, maxResults); + } + @Override + public Stream getSubGroupsStream(Integer firstResult, Integer maxResults) { + if (isUpdated()) return updated.getSubGroupsStream(firstResult, maxResults); + return modelSupplier.get().getSubGroupsStream(firstResult, maxResults); + } + + @Override + public Stream getSubGroupsStream(String search, Boolean exact, Integer firstResult, Integer maxResults) { + if (isUpdated()) return updated.getSubGroupsStream(search, exact, firstResult, maxResults); + return modelSupplier.get().getSubGroupsStream(search, exact, firstResult, maxResults); + } + + @Override + public Long getSubGroupsCount() { + if (isUpdated()) return updated.getSubGroupsCount(); + return modelSupplier.get().getSubGroupsCount(); + } @Override public void setParent(GroupModel group) { diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java index 329b160baa..49ae627a33 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java @@ -989,7 +989,6 @@ public class RealmCacheSession implements CacheRealmProvider { public Long getGroupsCount(RealmModel realm, Boolean onlyTopGroups) { return getGroupDelegate().getGroupsCount(realm, onlyTopGroups); } - @Override public long getClientsCount(RealmModel realm) { return getClientDelegate().getClientsCount(realm); @@ -1006,49 +1005,12 @@ public class RealmCacheSession implements CacheRealmProvider { } @Override - public Stream getTopLevelGroupsStream(RealmModel realm) { - String cacheKey = getTopGroupsQueryCacheKey(realm.getId()); - boolean queryDB = invalidations.contains(cacheKey) || listInvalidations.contains(realm.getId()); + public Stream getTopLevelGroupsStream(RealmModel realm, String search, Boolean exact, Integer first, Integer max) { + String cacheKey = getTopGroupsQueryCacheKey(realm.getId() + search + first + max); + boolean queryDB = invalidations.contains(cacheKey) || listInvalidations.contains(cacheKey) + || listInvalidations.contains(realm.getId()); if (queryDB) { - return getGroupDelegate().getTopLevelGroupsStream(realm); - } - - GroupListQuery query = cache.get(cacheKey, GroupListQuery.class); - if (query != null) { - logger.tracev("getTopLevelGroups cache hit: {0}", realm.getName()); - } - - if (query == null) { - Long loaded = cache.getCurrentRevision(cacheKey); - List model = getGroupDelegate().getTopLevelGroupsStream(realm).collect(Collectors.toList()); - if (model.isEmpty()) return Stream.empty(); - Set ids = new HashSet<>(); - for (GroupModel client : model) ids.add(client.getId()); - query = new GroupListQuery(loaded, cacheKey, realm, ids); - logger.tracev("adding realm getTopLevelGroups cache miss: realm {0} key {1}", realm.getName(), cacheKey); - cache.addRevisioned(query, startupRevision); - return model.stream(); - } - List list = new LinkedList<>(); - for (String id : query.getGroups()) { - GroupModel group = session.groups().getGroupById(realm, id); - if (group == null) { - invalidations.add(cacheKey); - return getGroupDelegate().getTopLevelGroupsStream(realm); - } - list.add(group); - } - - return list.stream().sorted(GroupModel.COMPARE_BY_NAME); - } - - @Override - public Stream getTopLevelGroupsStream(RealmModel realm, Integer first, Integer max) { - String cacheKey = getTopGroupsQueryCacheKey(realm.getId() + first + max); - boolean queryDB = invalidations.contains(cacheKey) || listInvalidations.contains(realm.getId() + first + max) - || listInvalidations.contains(realm.getId()); - if (queryDB) { - return getGroupDelegate().getTopLevelGroupsStream(realm, first, max); + return getGroupDelegate().getTopLevelGroupsStream(realm, search, exact, first, max); } GroupListQuery query = cache.get(cacheKey, GroupListQuery.class); @@ -1058,7 +1020,7 @@ public class RealmCacheSession implements CacheRealmProvider { if (Objects.isNull(query)) { Long loaded = cache.getCurrentRevision(cacheKey); - List model = getGroupDelegate().getTopLevelGroupsStream(realm, first, max).collect(Collectors.toList()); + List model = getGroupDelegate().getTopLevelGroupsStream(realm, search, exact, first, max).collect(Collectors.toList()); if (model.isEmpty()) return Stream.empty(); Set ids = new HashSet<>(); for (GroupModel client : model) ids.add(client.getId()); 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 b9471712ff..1a9c884fe5 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 @@ -20,6 +20,7 @@ package org.keycloak.models.jpa; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.models.ClientModel; import org.keycloak.models.GroupModel; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.jpa.entities.GroupAttributeEntity; @@ -38,6 +39,7 @@ import java.util.Objects; import java.util.stream.Stream; import jakarta.persistence.LockModeType; +import static org.keycloak.models.jpa.PaginationUtils.paginateQuery; import static org.keycloak.utils.StreamsUtil.closing; /** @@ -121,10 +123,35 @@ public class GroupAdapter implements GroupModel , JpaModel { @Override public Stream getSubGroupsStream() { - TypedQuery query = em.createNamedQuery("getGroupIdsByParent", String.class); - query.setParameter("realm", group.getRealm()); - query.setParameter("parent", group.getId()); - return closing(query.getResultStream().map(realm::getGroupById).filter(Objects::nonNull)); + return getSubGroupsStream("", false, -1, -1); + } + + @Override + public Stream getSubGroupsStream(String search, Boolean exact, Integer firstResult, Integer maxResults) { + TypedQuery query; + if (Boolean.TRUE.equals(exact)) { + query = em.createNamedQuery("getGroupIdsByParentAndName", String.class); + } else { + query = em.createNamedQuery("getGroupIdsByParentAndNameContaining", String.class); + } + query.setParameter("realm", realm.getId()) + .setParameter("parent", group.getId()) + .setParameter("search", search == null ? "" : search); + + return closing(paginateQuery(query, firstResult, maxResults).getResultStream() + .map(realm::getGroupById) + // In concurrent tests, the group might be deleted in another thread, therefore, skip those null values. + .filter(Objects::nonNull) + .sorted(GroupModel.COMPARE_BY_NAME) + ); + } + + @Override + public Long getSubGroupsCount() { + return em.createNamedQuery("getGroupCountByParent", Long.class) + .setParameter("realm", realm.getId()) + .setParameter("parent", group.getId()) + .getSingleResult(); } @Override 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 56d32acf0b..936d9e5852 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 @@ -21,15 +21,6 @@ import static org.keycloak.common.util.StackUtil.getShortStackTrace; import static org.keycloak.models.jpa.PaginationUtils.paginateQuery; import static org.keycloak.utils.StreamsUtil.closing; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.function.Function; -import java.util.stream.Collectors; -import java.util.stream.Stream; import jakarta.persistence.EntityManager; import jakarta.persistence.LockModeType; import jakarta.persistence.TypedQuery; @@ -39,7 +30,15 @@ import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; - +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.hibernate.Session; import org.jboss.logging.Logger; import org.keycloak.common.util.Time; @@ -71,8 +70,9 @@ import org.keycloak.models.jpa.entities.GroupEntity; import org.keycloak.models.jpa.entities.RealmEntity; import org.keycloak.models.jpa.entities.RealmLocalizationTextsEntity; import org.keycloak.models.jpa.entities.RoleEntity; -import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; + /** * @author Bill Burke @@ -183,7 +183,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc session.clientScopes().removeClientScopes(adapter); session.roles().removeRoles(adapter); - adapter.getTopLevelGroupsStream().forEach(adapter::removeGroup); + session.groups().getTopLevelGroupsStream(adapter).forEach(adapter::removeGroup); num = em.createNamedQuery("removeClientInitialAccessByRealm") .setParameter("realm", realm).executeUpdate(); @@ -437,8 +437,8 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc @Override public GroupModel getGroupByName(RealmModel realm, GroupModel parent, String name) { - TypedQuery query = em.createNamedQuery("getGroupIdByNameAndParent", String.class); - query.setParameter("name", name); + TypedQuery query = em.createNamedQuery("getGroupIdsByParentAndName", String.class); + query.setParameter("search", name); query.setParameter("realm", realm.getId()); query.setParameter("parent", parent != null ? parent.getId() : GroupEntity.TOP_PARENT_ID); List entities = query.getResultList(); @@ -566,7 +566,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc @Override public Long getGroupsCount(RealmModel realm, Boolean onlyTopGroups) { if(Objects.equals(onlyTopGroups, Boolean.TRUE)) { - return em.createNamedQuery("getTopLevelGroupCount", Long.class) + return em.createNamedQuery("getGroupCountByParent", Long.class) .setParameter("realm", realm.getId()) .setParameter("parent", GroupEntity.TOP_PARENT_ID) .getSingleResult(); @@ -603,21 +603,23 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc } @Override - public Stream getTopLevelGroupsStream(RealmModel realm) { - return getTopLevelGroupsStream(realm, null, null); - } + public Stream getTopLevelGroupsStream(RealmModel realm, String search, Boolean exact, Integer firstResult, Integer maxResults) { + TypedQuery groupsQuery; + if(Boolean.TRUE.equals(exact)) { + groupsQuery = em.createNamedQuery("getGroupIdsByParentAndName", String.class); + } else { + groupsQuery = em.createNamedQuery("getGroupIdsByParentAndNameContaining", String.class); + } - @Override - public Stream getTopLevelGroupsStream(RealmModel realm, Integer first, Integer max) { - TypedQuery groupsQuery = em.createNamedQuery("getTopLevelGroupIds", String.class) - .setParameter("realm", realm.getId()) - .setParameter("parent", GroupEntity.TOP_PARENT_ID); + groupsQuery.setParameter("realm", realm.getId()) + .setParameter("parent", GroupEntity.TOP_PARENT_ID) + .setParameter("search", search); - return closing(paginateQuery(groupsQuery, first, max).getResultStream() - .map(realm::getGroupById) - // In concurrent tests, the group might be deleted in another thread, therefore, skip those null values. - .filter(Objects::nonNull) - .sorted(GroupModel.COMPARE_BY_NAME) + return closing(paginateQuery(groupsQuery, firstResult, maxResults).getResultStream() + .map(realm::getGroupById) + // In concurrent tests, the group might be deleted in another thread, therefore, skip those null values. + .filter(Objects::nonNull) + .sorted(GroupModel.COMPARE_BY_NAME) ); } @@ -648,6 +650,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc session.users().preRemove(realm, group); realm.removeDefaultGroup(group); + group.getSubGroupsStream().forEach(realm::removeGroup); GroupEntity groupEntity = em.find(GroupEntity.class, group.getId(), LockModeType.PESSIMISTIC_WRITE); @@ -1015,14 +1018,9 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc .setParameter("search", search); Stream groups = paginateQuery(query, first, max).getResultStream(); - return closing(groups.map(id -> { - GroupModel groupById = session.groups().getGroupById(realm, id); - while (Objects.nonNull(groupById.getParentId())) { - groupById = session.groups().getGroupById(realm, groupById.getParentId()); - } - return groupById; - }).sorted(GroupModel.COMPARE_BY_NAME).distinct()); + return closing(groups.map(id -> session.groups().getGroupById(realm, id)).sorted(GroupModel.COMPARE_BY_NAME).distinct()); } + @Override public Stream searchGroupsByAttributes(RealmModel realm, Map attributes, Integer firstResult, Integer maxResults) { Map filteredAttributes = groupSearchableAttributes == null || groupSearchableAttributes.isEmpty() @@ -1057,7 +1055,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc TypedQuery query = em.createQuery(queryBuilder); return closing(paginateQuery(query, firstResult, maxResults).getResultStream()) - .map(g -> session.groups().getGroupById(realm, g.getId())); + .map(g -> new GroupAdapter(realm, em, g)); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java index 59f81266ac..b2d8928aba 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java @@ -29,16 +29,16 @@ import java.util.LinkedList; */ @NamedQueries({ @NamedQuery(name="getGroupIdsByParent", query="select u.id from GroupEntity u where u.realm = :realm and u.parentId = :parent order by u.name ASC"), + @NamedQuery(name="getGroupIdsByParentAndName", query="select u.id from GroupEntity u where u.realm = :realm and u.parentId = :parent and u.name = :search order by u.name ASC"), + @NamedQuery(name="getGroupIdsByParentAndNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and u.parentId = :parent and lower(u.name) like lower(concat('%',:search,'%')) order by u.name ASC"), @NamedQuery(name="getGroupIdsByRealm", query="select u.id from GroupEntity u where u.realm = :realm order by u.name ASC"), - @NamedQuery(name="getGroupIdsByNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and u.name like concat('%',:search,'%') order by u.name ASC"), + @NamedQuery(name="getGroupIdsByNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and lower(u.name) like lower(concat('%',:search,'%')) order by u.name ASC"), @NamedQuery(name="getGroupIdsByNameContainingFromIdList", query="select u.id from GroupEntity u where u.realm = :realm and lower(u.name) like lower(concat('%',:search,'%')) and u.id in :ids order by u.name ASC"), @NamedQuery(name="getGroupIdsByName", query="select u.id from GroupEntity u where u.realm = :realm and u.name = :search order by u.name ASC"), @NamedQuery(name="getGroupIdsFromIdList", query="select u.id from GroupEntity u where u.realm = :realm and u.id in :ids order by u.name ASC"), @NamedQuery(name="getGroupCountByNameContainingFromIdList", query="select count(u) from GroupEntity u where u.realm = :realm and lower(u.name) like lower(concat('%',:search,'%')) and u.id in :ids"), - @NamedQuery(name="getTopLevelGroupIds", query="select u.id from GroupEntity u where u.parentId = :parent and u.realm = :realm order by u.name ASC"), @NamedQuery(name="getGroupCount", query="select count(u) from GroupEntity u where u.realm = :realm"), - @NamedQuery(name="getTopLevelGroupCount", query="select count(u) from GroupEntity u where u.realm = :realm and u.parentId = :parent"), - @NamedQuery(name="getGroupIdByNameAndParent", query="select u.id from GroupEntity u where u.realm = :realm and u.parentId = :parent and u.name = :name") + @NamedQuery(name="getGroupCountByParent", query="select count(u) from GroupEntity u where u.realm = :realm and u.parentId = :parent") }) @Entity @Table(name="KEYCLOAK_GROUP", diff --git a/model/legacy-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java b/model/legacy-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java index 0f7bdc6efd..ed1f11de59 100755 --- a/model/legacy-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java +++ b/model/legacy-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java @@ -105,7 +105,7 @@ public class ExportUtils { // Groups and Roles if (options.isGroupsAndRolesIncluded()) { - ModelToRepresentation.exportGroups(realm, rep); + ModelToRepresentation.exportGroups(session, realm, rep); Map> clientRolesReps = new HashMap<>(); diff --git a/model/legacy-private/src/main/java/org/keycloak/storage/GroupStorageManager.java b/model/legacy-private/src/main/java/org/keycloak/storage/GroupStorageManager.java index 98d9d75570..05c9874a9c 100644 --- a/model/legacy-private/src/main/java/org/keycloak/storage/GroupStorageManager.java +++ b/model/legacy-private/src/main/java/org/keycloak/storage/GroupStorageManager.java @@ -16,6 +16,8 @@ */ package org.keycloak.storage; +import java.util.Map; +import java.util.stream.Stream; import org.keycloak.models.GroupModel; import org.keycloak.models.GroupProvider; import org.keycloak.models.KeycloakSession; @@ -26,8 +28,6 @@ import org.keycloak.storage.group.GroupStorageProvider; import org.keycloak.storage.group.GroupStorageProviderFactory; import org.keycloak.storage.group.GroupStorageProviderModel; -import java.util.Map; -import java.util.stream.Stream; public class GroupStorageManager extends AbstractStorageManager implements GroupProvider { @@ -85,6 +85,7 @@ public class GroupStorageManager extends AbstractStorageManager getTopLevelGroupsStream(RealmModel realm) { - return localStorage().getTopLevelGroupsStream(realm); - } - - @Override - public Stream getTopLevelGroupsStream(RealmModel realm, Integer firstResult, Integer maxResults) { - return localStorage().getTopLevelGroupsStream(realm, firstResult, maxResults); + public Stream getTopLevelGroupsStream(RealmModel realm, String search, Boolean exact, Integer firstResult, Integer maxResults) { + return localStorage().getTopLevelGroupsStream(realm, search, exact, firstResult, maxResults); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java index 056a7f4b70..da301d99da 100644 --- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java @@ -17,6 +17,11 @@ package org.keycloak.models.map.group; +import java.util.Map; +import java.util.Objects; +import java.util.function.Function; +import java.util.function.UnaryOperator; +import java.util.stream.Stream; import org.jboss.logging.Logger; import org.keycloak.models.GroupModel; import org.keycloak.models.GroupModel.SearchableFields; @@ -28,18 +33,13 @@ import org.keycloak.models.RoleModel; import org.keycloak.models.map.common.DeepCloner; import org.keycloak.models.map.common.HasRealmId; import org.keycloak.models.map.storage.MapStorage; - import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator; import org.keycloak.models.map.storage.QueryParameters; - import org.keycloak.models.map.storage.criteria.DefaultModelCriteria; import org.keycloak.models.utils.KeycloakModelUtils; -import java.util.Map; -import java.util.Objects; -import java.util.function.Function; -import java.util.function.UnaryOperator; -import java.util.stream.Stream; + + import static org.keycloak.common.util.StackUtil.getShortStackTrace; import static org.keycloak.models.map.common.AbstractMapProviderFactory.MapProviderObjectType.GROUP_AFTER_REMOVE; @@ -135,8 +135,7 @@ public class MapGroupProvider implements GroupProvider { } return storeWithRealm(realm).read(queryParameters) - .map(entityToAdapterFunc(realm)) - ; + .map(entityToAdapterFunc(realm)); } @Override @@ -168,7 +167,14 @@ public class MapGroupProvider implements GroupProvider { @Override public Long getGroupsCountByNameContaining(RealmModel realm, String search) { - return searchForGroupByNameStream(realm, search, false, null, null).count(); + LOG.tracef("getGroupsCountByNameContaining(%s, %s, %s)%s", realm, session, search, getShortStackTrace()); + + DefaultModelCriteria mcb = criteria(); + + mcb = mcb.compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId()) + .compare(SearchableFields.NAME, Operator.ILIKE, "%" + search + "%"); + + return storeWithRealm(realm).read(withCriteria(mcb).orderBy(SearchableFields.NAME, ASCENDING)).count(); } @Override @@ -181,21 +187,21 @@ public class MapGroupProvider implements GroupProvider { } @Override - public Stream getTopLevelGroupsStream(RealmModel realm) { - LOG.tracef("getTopLevelGroupsStream(%s)%s", realm, getShortStackTrace()); - return getGroupsStreamInternal(realm, - (DefaultModelCriteria mcb) -> mcb.compare(SearchableFields.PARENT_ID, Operator.NOT_EXISTS), - null - ); - } + public Stream getTopLevelGroupsStream(RealmModel realm, String search, Boolean exact, Integer firstResult, Integer maxResults) { + LOG.tracef("getTopLevelGroupsStream(%s, %s,%s, %s,%s)%s", realm, search, exact, firstResult, maxResults, getShortStackTrace()); - @Override - public Stream getTopLevelGroupsStream(RealmModel realm, Integer firstResult, Integer maxResults) { - LOG.tracef("getTopLevelGroupsStream(%s, %s, %s)%s", realm, firstResult, maxResults, getShortStackTrace()); - return getGroupsStreamInternal(realm, - (DefaultModelCriteria mcb) -> mcb.compare(SearchableFields.PARENT_ID, Operator.NOT_EXISTS), - qp -> qp.offset(firstResult).limit(maxResults) - ); + DefaultModelCriteria mcb = criteria(); + mcb = mcb.compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId()) + .compare(SearchableFields.PARENT_ID, Operator.NOT_EXISTS); + if(Boolean.TRUE.equals(exact)) { + mcb.compare(SearchableFields.NAME, Operator.EQ,search); + } else { + mcb.compare(SearchableFields.NAME, Operator.ILIKE, "%" + search + "%"); + } + + + return storeWithRealm(realm).read(withCriteria(mcb).pagination(firstResult, maxResults, SearchableFields.NAME)) + .map(entityToAdapterFunc(realm)); } @Override @@ -214,14 +220,7 @@ public class MapGroupProvider implements GroupProvider { return storeWithRealm(realm).read(withCriteria(mcb).pagination(firstResult, maxResults, SearchableFields.NAME)) - .map(MapGroupEntity::getId) - .map(id -> { - GroupModel groupById = session.groups().getGroupById(realm, id); - while (Objects.nonNull(groupById.getParentId())) { - groupById = session.groups().getGroupById(realm, groupById.getParentId()); - } - return groupById; - }).sorted(GroupModel.COMPARE_BY_NAME).distinct(); + .map(entityToAdapterFunc(realm)); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java index a2d95eb1ef..cf9dff744e 100644 --- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java @@ -61,7 +61,14 @@ public class MapGroupProviderFactory extends AbstractMapProviderFactory create(session).removeGroup(realm, subGroup)); + + // TODO: Should the batch size be a config option? + // batch and remove subgroups to avoid grinding server to a halt at scale + long batches = (long) Math.ceil(group.getSubGroupsCount() / 1000.0); + for(int i = 0; i < batches; i++) { + group.getSubGroupsStream(i * 1000, 1000) + .forEach(subGroup -> create(session).removeGroup(realm, subGroup)); + } } else if (type == GROUP_AFTER_REMOVE) { session.getKeycloakSessionFactory().publish(new GroupModel.GroupRemovedEvent() { @Override public RealmModel getRealm() { return (RealmModel) params[0]; } diff --git a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/AdminExtResource.java b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/AdminExtResource.java index ba1374e90a..d0a700028a 100644 --- a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/AdminExtResource.java +++ b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/AdminExtResource.java @@ -40,11 +40,6 @@ public final class AdminExtResource { return new EffectiveRoleMappingResource(session, realm, auth); } - @Path("/groups") - public GroupsResource groups() { - return new GroupsResource(session, realm, auth); - } - @Path("/sessions") public SessionsResource sessions() { return new SessionsResource(session, realm, auth); diff --git a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/GroupsResource.java b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/GroupsResource.java deleted file mode 100644 index c7882a6b95..0000000000 --- a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/GroupsResource.java +++ /dev/null @@ -1,135 +0,0 @@ -package org.keycloak.admin.ui.rest; - - -import java.util.Objects; -import java.util.stream.Stream; -import jakarta.ws.rs.Consumes; -import jakarta.ws.rs.DefaultValue; -import jakarta.ws.rs.GET; -import jakarta.ws.rs.Path; -import jakarta.ws.rs.PathParam; -import jakarta.ws.rs.Produces; -import jakarta.ws.rs.QueryParam; - -import org.eclipse.microprofile.openapi.annotations.Operation; -import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; -import org.eclipse.microprofile.openapi.annotations.media.Content; -import org.eclipse.microprofile.openapi.annotations.media.Schema; -import org.eclipse.microprofile.openapi.annotations.responses.APIResponse; -import org.keycloak.models.GroupModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.RealmModel; -import org.keycloak.models.utils.ModelToRepresentation; -import org.keycloak.representations.idm.GroupRepresentation; -import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator; -import org.keycloak.services.resources.admin.permissions.GroupPermissionEvaluator; -import org.keycloak.utils.GroupUtils; - -import static org.keycloak.models.utils.ModelToRepresentation.toRepresentation; - -public class GroupsResource { - private final KeycloakSession session; - private final RealmModel realm; - private final AdminPermissionEvaluator auth; - - public GroupsResource(KeycloakSession session, RealmModel realm, AdminPermissionEvaluator auth) { - super(); - this.realm = realm; - this.auth = auth; - this.session = session; - } - - @GET - @Consumes({"application/json"}) - @Produces({"application/json"}) - @Operation( - summary = "List all groups with fine grained authorisation", - description = "This endpoint returns a list of groups with fine grained authorisation" - ) - @APIResponse( - responseCode = "200", - description = "", - content = {@Content( - schema = @Schema( - implementation = GroupRepresentation.class, - type = SchemaType.ARRAY - ) - )} - ) - public final Stream listGroups(@QueryParam("search") @DefaultValue("") final String search, @QueryParam("first") - @DefaultValue("0") int first, @QueryParam("max") @DefaultValue("10") int max, @QueryParam("global") @DefaultValue("true") boolean global, - @QueryParam("exact") @DefaultValue("false") boolean exact) { - GroupPermissionEvaluator groupsEvaluator = auth.groups(); - groupsEvaluator.requireList(); - final Stream stream; - if (global) { - stream = session.groups().searchForGroupByNameStream(realm, search.trim(), exact, first, max); - } else { - stream = this.realm.getTopLevelGroupsStream().filter(g -> g.getName().contains(search)).skip(first).limit(max); - } - - boolean canViewGlobal = groupsEvaluator.canView(); - return stream.filter(group -> canViewGlobal || groupsEvaluator.canView(group)) - .map(group -> GroupUtils.toGroupHierarchy(groupsEvaluator, group, search, exact, "".equals(search))); - } - - @GET - @Path("/subgroup") - @Consumes({"application/json"}) - @Produces({"application/json"}) - @Operation( - summary = "List all sub groups with fine grained authorisation and pagination", - description = "This endpoint returns a list of groups with fine grained authorisation" - ) - @APIResponse( - responseCode = "200", - description = "", - content = {@Content( - schema = @Schema( - implementation = GroupRepresentation.class, - type = SchemaType.ARRAY - ) - )} - ) - public final Stream subgroups(@QueryParam("id") final String groupId, @QueryParam("search") - @DefaultValue("") final String search, @QueryParam("first") @DefaultValue("0") int first, @QueryParam("max") @DefaultValue("10") int max) { - GroupPermissionEvaluator groupsEvaluator = auth.groups(); - groupsEvaluator.requireList(); - GroupModel group = realm.getGroupById(groupId); - if (group == null) { - return Stream.empty(); - } - - return group.getSubGroupsStream().filter(g -> g.getName().contains(search)) - .map(g -> GroupUtils.toGroupHierarchy(groupsEvaluator, g, search, false, true)).skip(first).limit(max); - } - - @GET - @Path("{id}") - @Consumes({"application/json"}) - @Produces({"application/json"}) - @Operation( - summary = "Find a specific group with no subgroups", - description = "This endpoint returns a group by id with no subgroups" - ) - @APIResponse( - responseCode = "200", - description = "", - content = {@Content( - schema = @Schema( - implementation = GroupRepresentation.class, - type = SchemaType.OBJECT - ) - )} - ) - public GroupRepresentation findGroupById(@PathParam("id") String id) { - GroupModel group = realm.getGroupById(id); - this.auth.groups().requireView(group); - - GroupRepresentation rep = toRepresentation(group, true); - - rep.setAccess(auth.groups().getAccess(group)); - - return rep; - } -} diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index e0c5acecd5..e48cbd9b81 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -30,6 +30,7 @@ import org.keycloak.authorization.policy.provider.PolicyProviderFactory; import org.keycloak.authorization.store.PolicyStore; import org.keycloak.authorization.store.StoreFactory; import org.keycloak.common.Profile; +import org.keycloak.common.Profile.Feature; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.common.util.Time; import org.keycloak.component.ComponentModel; @@ -48,7 +49,6 @@ import org.keycloak.representations.idm.*; import org.keycloak.representations.idm.authorization.*; import org.keycloak.storage.StorageId; import org.keycloak.util.JsonSerialization; -import org.keycloak.utils.StreamsUtil; import org.keycloak.utils.StringUtil; import java.io.IOException; @@ -131,6 +131,7 @@ public class ModelToRepresentation { rep.setId(group.getId()); rep.setName(group.getName()); rep.setPath(buildGroupPath(group)); + rep.setParentId(group.getParentId()); if (!full) return rep; // Role mappings Set roles = group.getRoleMappingsStream().collect(Collectors.toSet()); @@ -153,70 +154,17 @@ public class ModelToRepresentation { return rep; } - public static Stream searchGroupsByAttributes(KeycloakSession session, RealmModel realm, boolean full, boolean populateHierarchy, Map attributes, Integer first, Integer max) { - Stream groups = searchGroupModelsByAttributes(session, realm, full, populateHierarchy, attributes, first, max); - // and then turn the result into GroupRepresentations creating whole hierarchy of child groups for each root group - return groups.map(g -> toGroupHierarchy(g, full, attributes)); - + public static Stream searchGroupModelsByAttributes(KeycloakSession session, RealmModel realm, Map attributes, Integer first, Integer max) { + return session.groups().searchGroupsByAttributes(realm, attributes, first, max); } - public static Stream searchGroupModelsByAttributes(KeycloakSession session, RealmModel realm, boolean full, boolean populateHierarchy, Map attributes, Integer first, Integer max) { - Stream groups = session.groups().searchGroupsByAttributes(realm, attributes, first, max); - if(populateHierarchy) { - groups = groups - // We need to return whole group hierarchy when any child group fulfills the attribute search, - // therefore for each group from the result, we need to find root group - .map(group -> { - while (Objects.nonNull(group.getParentId())) { - group = group.getParent(); - } - return group; - }) - - // More child groups of one root can fulfill the search, so we need to filter duplicates - .filter(StreamsUtil.distinctByKey(GroupModel::getId)); - } - return groups; - } - - public static Stream searchForGroupByName(KeycloakSession session, RealmModel realm, boolean full, String search, Boolean exact, Integer first, Integer max) { - return searchForGroupModelByName(session, realm, full, search, exact, first, max) - .map(g -> toGroupHierarchy(g, full, search, exact)); - } - - public static Stream searchForGroupModelByName(KeycloakSession session, RealmModel realm, boolean full, String search, Boolean exact, Integer first, Integer max) { - return session.groups().searchForGroupByNameStream(realm, search, exact, first, max); - } - - public static Stream searchForGroupByName(UserModel user, boolean full, String search, Integer first, Integer max) { - return user.getGroupsStream(search, first, max) - .map(group -> toRepresentation(group, full)); - } - - public static Stream toGroupHierarchy(RealmModel realm, boolean full, Integer first, Integer max) { - return toGroupModelHierarchy(realm, full, first, max) - .map(g -> toGroupHierarchy(g, full)); - } - - public static Stream toGroupModelHierarchy(RealmModel realm, boolean full, Integer first, Integer max) { - return realm.getTopLevelGroupsStream(first, max); - } - - public static Stream toGroupHierarchy(UserModel user, boolean full, Integer first, Integer max) { - return user.getGroupsStream(null, first, max) - .map(group -> toRepresentation(group, full)); - } - - public static Stream toGroupHierarchy(RealmModel realm, boolean full) { - return realm.getTopLevelGroupsStream() + @Deprecated + public static Stream toGroupHierarchy(KeycloakSession session, RealmModel realm, boolean full) { + return session.groups().getTopLevelGroupsStream(realm, null, null) .map(g -> toGroupHierarchy(g, full)); } - public static Stream toGroupHierarchy(UserModel user, boolean full) { - return user.getGroupsStream() - .map(group -> toRepresentation(group, full)); - } - + @Deprecated public static GroupRepresentation toGroupHierarchy(GroupModel group, boolean full) { return toGroupHierarchy(group, full, (String) null); } @@ -226,6 +174,11 @@ public class ModelToRepresentation { return toGroupHierarchy(group, full, search, false); } + @Deprecated + /** + * @deprecated This function is left in place to serve mostly for a full export of all groups. + * There is a GroupUtil class in the keycloak-services module to handle normal search operations + */ public static GroupRepresentation toGroupHierarchy(GroupModel group, boolean full, String search, Boolean exact) { GroupRepresentation rep = toRepresentation(group, full); List subGroups = group.getSubGroupsStream() @@ -235,14 +188,6 @@ public class ModelToRepresentation { return rep; } - public static GroupRepresentation toGroupHierarchy(GroupModel group, boolean full, Map attributes) { - GroupRepresentation rep = toRepresentation(group, full); - List subGroups = group.getSubGroupsStream() - .map(subGroup -> toGroupHierarchy(subGroup, full, attributes)).collect(Collectors.toList()); - rep.setSubGroups(subGroups); - return rep; - } - private static boolean groupMatchesSearchOrIsPathElement(GroupModel group, String search, Boolean exact) { if (StringUtil.isBlank(search)) { return true; @@ -554,7 +499,7 @@ public class ModelToRepresentation { if (internal) { exportAuthenticationFlows(session, realm, rep); exportRequiredActions(realm, rep); - exportGroups(realm, rep); + exportGroups(session, realm, rep); } session.clientPolicy().updateRealmRepresentationFromModel(realm, rep); @@ -586,9 +531,8 @@ public class ModelToRepresentation { return a; } - - public static void exportGroups(RealmModel realm, RealmRepresentation rep) { - rep.setGroups(toGroupHierarchy(realm, true).collect(Collectors.toList())); + public static void exportGroups(KeycloakSession session, RealmModel realm, RealmRepresentation rep) { + rep.setGroups(toGroupHierarchy(session, realm, true).collect(Collectors.toList())); } public static void exportAuthenticationFlows(KeycloakSession session, RealmModel realm, RealmRepresentation rep) { 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 0665adf11c..09930a8ab5 100755 --- a/server-spi/src/main/java/org/keycloak/models/GroupModel.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupModel.java @@ -110,6 +110,69 @@ public interface GroupModel extends RoleMapperModel { */ Stream getSubGroupsStream(); + /** + * Returns all sub groups for the parent group matching the fuzzy search as a stream, paginated. + * Stream is sorted by the group name. + * + * @param search searched string. If empty or {@code null} all subgroups are returned. + * @return Stream of {@link GroupModel}. Never returns {@code null}. + */ + default Stream getSubGroupsStream(String search, Integer firstResult, Integer maxResults) { + return getSubGroupsStream(search, false, firstResult, maxResults); + } + + /** + * Returns all sub groups for the parent group as a stream, paginated. + * + * @param firstResult First result to return. Ignored if negative or {@code null}. + * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. + * @return + */ + default Stream getSubGroupsStream(Integer firstResult, Integer maxResults) { + return getSubGroupsStream(null, firstResult, maxResults); + } + + /** + * Returns all subgroups for the parent group matching the search as a stream, paginated. + * Stream is sorted by the group name. + * + * @param search search string. If empty or {@code null} all subgroups are returned. + * @param exact toggles fuzzy searching + * @param firstResult First result to return. Ignored if negative or {@code null}. + * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. + * @return Stream of {@link GroupModel}. Never returns {@code null}. + */ + default Stream getSubGroupsStream(String search, Boolean exact, Integer firstResult, Integer maxResults) { + Stream allSubgorupsGroups = getSubGroupsStream().filter(group -> { + if (search == null || search.isEmpty()) return true; + if (Boolean.TRUE.equals(exact)) { + return group.getName().equals(search); + } else { + return group.getName().toLowerCase().contains(search.toLowerCase()); + } + }); + + // Copied over from StreamsUtil from server-spi-private which is not available here + if (firstResult != null && firstResult > 0) { + allSubgorupsGroups = allSubgorupsGroups.skip(firstResult); + } + + if (maxResults != null && maxResults >= 0) { + allSubgorupsGroups = allSubgorupsGroups.limit(maxResults); + } + + return allSubgorupsGroups; + } + + /** + * Returns the number of groups contained beneath this group. + * + * @return The number of groups beneath this group. Never returns {@code null}. + */ + default Long getSubGroupsCount() { + return getSubGroupsStream().count(); + } + /** * You must also call addChild on the parent group, addChild on RealmModel if there is no parent group * 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 fd84e1701e..7951fd9a37 100644 --- a/server-spi/src/main/java/org/keycloak/models/GroupProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupProvider.java @@ -125,7 +125,9 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @param realm Realm. * @return Stream of all top level groups in the realm. Never returns {@code null}. */ - Stream getTopLevelGroupsStream(RealmModel realm); + default Stream getTopLevelGroupsStream(RealmModel realm) { + return getTopLevelGroupsStream(realm, "", false, null, null); + } /** * Returns top level groups (i.e. groups without parent group) for the given realm. @@ -135,7 +137,20 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. * @return Stream of top level groups in the realm. Never returns {@code null}. */ - Stream getTopLevelGroupsStream(RealmModel realm, Integer firstResult, Integer maxResults); + default Stream getTopLevelGroupsStream(RealmModel realm, Integer firstResult, Integer maxResults) { + return getTopLevelGroupsStream(realm, "", false, firstResult, maxResults); + } + + /** + * Returns top level groups (i.e. groups without parent group) for the given realm. + * + * @param realm Realm. + * @param firstResult First result to return. Ignored if negative or {@code null}. + * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. + * @param search The name that should be matched + * @return Stream of top level groups in the realm. Never returns {@code null}. + */ + Stream getTopLevelGroupsStream(RealmModel realm, String search, Boolean exact, Integer firstResult, Integer maxResults); /** * Creates a new group with the given name in the given realm. diff --git a/server-spi/src/main/java/org/keycloak/models/RealmModel.java b/server-spi/src/main/java/org/keycloak/models/RealmModel.java index b2eb909301..b57c4313d1 100755 --- a/server-spi/src/main/java/org/keycloak/models/RealmModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RealmModel.java @@ -653,13 +653,17 @@ public interface RealmModel extends RoleContainerModel { Long getGroupsCount(Boolean onlyTopGroups); Long getGroupsCountByNameContaining(String search); + @Deprecated /** + * @deprecated It is now preferable to use {@link GroupProvider} from a {@link KeycloakSession} * Returns top level groups as a stream. * @return Stream of {@link GroupModel}. Never returns {@code null}. */ Stream getTopLevelGroupsStream(); + @Deprecated /** + * @deprecated It is now preferable to use {@link GroupProvider} from a {@link KeycloakSession} * Returns top level groups as a stream. * @param first {@code Integer} Index of the first desired group. Ignored if negative or {@code null}. * @param max {@code Integer} Maximum number of returned groups. Ignored if negative or {@code null}. diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java b/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java index f39fe66250..a48c7fbae2 100755 --- a/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java +++ b/services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java @@ -92,6 +92,7 @@ import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.EventAuditingAttributeChangeListener; import org.keycloak.userprofile.ValidationException; import org.keycloak.userprofile.ValidationException.Error; +import org.keycloak.utils.GroupUtils; import org.keycloak.validate.Validators; /** @@ -459,9 +460,10 @@ public class AccountRestService { @GET @NoCache @Produces(MediaType.APPLICATION_JSON) + //TODO GROUPS this isn't paginated public Stream groupMemberships(@QueryParam("briefRepresentation") @DefaultValue("true") boolean briefRepresentation) { auth.require(AccountRoles.VIEW_GROUPS); - return ModelToRepresentation.toGroupHierarchy(user, !briefRepresentation); + return user.getGroupsStream().map(g -> ModelToRepresentation.toRepresentation(g, !briefRepresentation)); } @Path("/applications") 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 4420a3978c..d9d7b750a5 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 @@ -16,6 +16,7 @@ */ package org.keycloak.services.resources.admin; +import jakarta.ws.rs.DefaultValue; import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.extensions.Extension; import org.eclipse.microprofile.openapi.annotations.parameters.Parameter; @@ -36,7 +37,6 @@ import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.ManagementPermissionReference; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.services.ErrorResponse; -import org.keycloak.services.Urls; import org.keycloak.services.resources.KeycloakOpenAPI; import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator; import org.keycloak.services.resources.admin.permissions.AdminPermissionManagement; @@ -59,6 +59,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Stream; +import org.keycloak.utils.GroupUtils; /** * @resource Groups @@ -94,11 +95,11 @@ public class GroupResource { public GroupRepresentation getGroup() { this.auth.groups().requireView(group); - GroupRepresentation rep = ModelToRepresentation.toGroupHierarchy(group, true); + GroupRepresentation rep = GroupUtils.toRepresentation(this.auth.groups(), group, true); rep.setAccess(auth.groups().getAccess(group)); - return rep; + return GroupUtils.populateSubGroupCount(group, rep); } /** @@ -134,7 +135,7 @@ public class GroupResource { private Stream siblings() { if (group.getParentId() == null) { - return realm.getTopLevelGroupsStream(); + return session.groups().getTopLevelGroupsStream(realm); } else { return group.getParent().getSubGroupsStream(); } @@ -150,6 +151,21 @@ public class GroupResource { adminEvent.operation(OperationType.DELETE).resourcePath(session.getContext().getUri()).success(); } + @GET + @Path("children") + @NoCache + @Produces(MediaType.APPLICATION_JSON) + @Tag(name = KeycloakOpenAPI.Admin.Tags.GROUPS) + @Operation( summary = "Return a paginated list of subgroups that have a parent group corresponding to the group on the URL") + public Stream getSubGroups(@QueryParam("first") @DefaultValue("0") Integer first, + @QueryParam("max") @DefaultValue("10") Integer max, + @QueryParam("briefRepresentation") @DefaultValue("false") Boolean full) { + this.auth.groups().requireView(group); + boolean canViewGlobal = auth.groups().canView(); + return group.getSubGroupsStream(first, max) + .filter(g -> canViewGlobal || auth.groups().canView(g)) + .map(g -> GroupUtils.populateSubGroupCount(g, GroupUtils.toRepresentation(auth.groups(), g, full))); + } /** * Set or create child. This will just set the parent if it exists. Create it and set the parent @@ -201,7 +217,7 @@ public class GroupResource { } adminEvent.resourcePath(session.getContext().getUri()).representation(rep).success(); - GroupRepresentation childRep = ModelToRepresentation.toGroupHierarchy(child, true); + GroupRepresentation childRep = GroupUtils.toRepresentation(auth.groups(), child, true); return builder.type(MediaType.APPLICATION_JSON_TYPE).entity(childRep).build(); } catch (ModelDuplicateException e) { throw ErrorResponse.exists("Sibling group named '" + groupName + "' already exists."); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java index 74bf445e68..115b330716 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java @@ -16,12 +16,26 @@ */ package org.keycloak.services.resources.admin; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.DefaultValue; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.QueryParam; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import java.net.URI; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Stream; import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.extensions.Extension; import org.eclipse.microprofile.openapi.annotations.tags.Tag; import org.jboss.resteasy.annotations.cache.NoCache; -import jakarta.ws.rs.NotFoundException; - import org.keycloak.common.util.ObjectUtil; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; @@ -38,21 +52,7 @@ import org.keycloak.services.resources.admin.permissions.GroupPermissionEvaluato import org.keycloak.utils.GroupUtils; import org.keycloak.utils.SearchQueryUtils; -import jakarta.ws.rs.Consumes; -import jakarta.ws.rs.DefaultValue; -import jakarta.ws.rs.GET; -import jakarta.ws.rs.POST; -import jakarta.ws.rs.Path; -import jakarta.ws.rs.PathParam; -import jakarta.ws.rs.Produces; -import jakarta.ws.rs.QueryParam; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; -import java.net.URI; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; -import java.util.stream.Stream; + /** * @resource Groups @@ -94,21 +94,25 @@ public class GroupsResource { GroupPermissionEvaluator groupsEvaluator = auth.groups(); groupsEvaluator.requireList(); - Stream stream = null; + Stream stream; if (Objects.nonNull(searchQuery)) { Map attributes = SearchQueryUtils.getFields(searchQuery); - stream = ModelToRepresentation.searchGroupModelsByAttributes(session, realm, !briefRepresentation, populateHierarchy, attributes, firstResult, maxResults); + stream = ModelToRepresentation.searchGroupModelsByAttributes(session, realm, attributes, firstResult, maxResults); } else if (Objects.nonNull(search)) { - stream = ModelToRepresentation.searchForGroupModelByName(session, realm, !briefRepresentation, search.trim(), exact, firstResult, maxResults); + stream = session.groups().searchForGroupByNameStream(realm, search.trim(), exact, firstResult, maxResults); } else if(Objects.nonNull(firstResult) && Objects.nonNull(maxResults)) { - stream = ModelToRepresentation.toGroupModelHierarchy(realm, !briefRepresentation, firstResult, maxResults); + stream = session.groups().getTopLevelGroupsStream(realm, firstResult, maxResults); } else { - stream = realm.getTopLevelGroupsStream(); + stream = session.groups().getTopLevelGroupsStream(realm); } + if(populateHierarchy) { + return GroupUtils.populateGroupHierarchyFromSubGroups(session, realm, stream, !briefRepresentation, groupsEvaluator); + } boolean canViewGlobal = groupsEvaluator.canView(); - return stream.filter(group -> canViewGlobal || groupsEvaluator.canView(group)) - .map(group -> GroupUtils.toGroupHierarchy(groupsEvaluator, group, search, exact, !briefRepresentation, false)); + return stream + .filter(g -> canViewGlobal || groupsEvaluator.canView(g)) + .map(g -> GroupUtils.populateSubGroupCount(g, GroupUtils.toRepresentation(groupsEvaluator, g, !briefRepresentation))); } /** @@ -139,6 +143,8 @@ public class GroupsResource { @Operation( summary = "Returns the groups counts.") public Map getGroupCount(@QueryParam("search") String search, @QueryParam("top") @DefaultValue("false") boolean onlyTopGroups) { + GroupPermissionEvaluator groupsEvaluator = auth.groups(); + groupsEvaluator.requireList(); Long results; Map map = new HashMap<>(); if (Objects.nonNull(search)) { 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 ddf75343c1..c7090623e4 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 @@ -117,6 +117,7 @@ import org.keycloak.services.resources.admin.permissions.AdminPermissions; import org.keycloak.storage.DatastoreProvider; import org.keycloak.storage.ExportImportManager; import org.keycloak.storage.LegacyStoreSyncEvent; +import org.keycloak.utils.GroupUtils; import org.keycloak.utils.ProfileHelper; import org.keycloak.utils.ReservedCharValidator; @@ -1079,7 +1080,7 @@ public class RealmAdminResource { } auth.groups().requireView(found); - return ModelToRepresentation.toGroupHierarchy(found, true); + return ModelToRepresentation.toRepresentation(found, true); } /** diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index 89e53d933e..eebbc1cc41 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java @@ -89,6 +89,7 @@ import org.keycloak.userprofile.AttributeValidatorMetadata; import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.UserProfileProvider; import org.keycloak.userprofile.ValidationException; +import org.keycloak.utils.GroupUtils; import org.keycloak.utils.ProfileHelper; import jakarta.ws.rs.BadRequestException; @@ -982,12 +983,7 @@ public class UserResource { @QueryParam("max") Integer maxResults, @QueryParam("briefRepresentation") @DefaultValue("true") boolean briefRepresentation) { auth.users().requireView(user); - - if (Objects.nonNull(search)) { - return ModelToRepresentation.searchForGroupByName(user, !briefRepresentation, search.trim(), firstResult, maxResults); - } else { - return ModelToRepresentation.toGroupHierarchy(user, !briefRepresentation, firstResult, maxResults); - } + return user.getGroupsStream(search, firstResult, maxResults).map(g -> ModelToRepresentation.toRepresentation(g, !briefRepresentation)); } @GET diff --git a/services/src/main/java/org/keycloak/utils/GroupUtils.java b/services/src/main/java/org/keycloak/utils/GroupUtils.java index bc1c51e564..8a63af9e6b 100644 --- a/services/src/main/java/org/keycloak/utils/GroupUtils.java +++ b/services/src/main/java/org/keycloak/utils/GroupUtils.java @@ -1,56 +1,88 @@ package org.keycloak.utils; - -import java.util.Collections; -import java.util.stream.Collectors; - -import org.keycloak.common.Profile; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Stream; import org.keycloak.models.GroupModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.services.resources.admin.permissions.GroupPermissionEvaluator; + + public class GroupUtils { - // Moved out from org.keycloak.admin.ui.rest.GroupsResource - public static GroupRepresentation toGroupHierarchy(GroupPermissionEvaluator groupsEvaluator, GroupModel group, final String search, boolean exact, boolean lazy) { - return toGroupHierarchy(groupsEvaluator, group, search, exact, true, lazy); + + /** + * This method takes the provided groups and attempts to load their parents all the way to the root group while maintaining the hierarchy data + * for each GroupRepresentation object. Each resultant GroupRepresentation object in the stream should contain relevant subgroups to the originally + * provided groups + * @param session The active keycloak session + * @param realm The realm to operate on + * @param groups The groups that we want to populate the hierarchy for + * @return A stream of groups that contain all relevant groups from the root down with no extra siblings + */ + public static Stream populateGroupHierarchyFromSubGroups(KeycloakSession session, RealmModel realm, Stream groups, boolean full, GroupPermissionEvaluator groupEvaluator) { + Map groupIdToGroups = new HashMap<>(); + groups.forEach(group -> { + //TODO GROUPS do permissions work in such a way that if you can view the children you can definitely view the parents? + if(!groupEvaluator.canView() && !groupEvaluator.canView(group)) return; + + GroupRepresentation currGroup = toRepresentation(groupEvaluator, group, full); + populateSubGroupCount(group, currGroup); + groupIdToGroups.putIfAbsent(currGroup.getId(), currGroup); + + while(currGroup.getParentId() != null) { + GroupModel parentModel = session.groups().getGroupById(realm, currGroup.getParentId()); + + //TODO GROUPS not sure if this is even necessary but if somehow you can't view the parent we need to remove the child and move on + if(!groupEvaluator.canView() && !groupEvaluator.canView(parentModel)) { + groupIdToGroups.remove(currGroup.getId()); + break; + } + + GroupRepresentation parent = groupIdToGroups.computeIfAbsent(currGroup.getParentId(), + id -> toRepresentation(groupEvaluator, parentModel, full)); + populateSubGroupCount(parentModel, parent); + GroupRepresentation finalCurrGroup = currGroup; + + // check the parent for existing subgroups that match the group we're currently operating on and merge them if needed + Optional duplicateGroup = parent.getSubGroups() == null ? + Optional.empty() : parent.getSubGroups().stream().filter(g -> g.equals(finalCurrGroup)).findFirst(); + if(duplicateGroup.isPresent()) { + duplicateGroup.get().merge(currGroup); + } else { + parent.getSubGroups().add(currGroup); + } + groupIdToGroups.remove(currGroup.getId()); + currGroup = parent; + } + }); + return groupIdToGroups.values().stream().sorted(Comparator.comparing(GroupRepresentation::getName)); } - public static GroupRepresentation toGroupHierarchy(GroupPermissionEvaluator groupsEvaluator, GroupModel group, final String search, boolean exact, boolean full, boolean lazy) { - GroupRepresentation rep = ModelToRepresentation.toRepresentation(group, full); - if (!lazy) { - rep.setSubGroups(group.getSubGroupsStream().filter(g -> - groupMatchesSearchOrIsPathElement( - g, search - ) - ).map(subGroup -> - ModelToRepresentation.toGroupHierarchy( - subGroup, full, search, exact - ) - - ).collect(Collectors.toList())); - } else { - rep.setSubGroups(Collections.emptyList()); - } - - if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) { - setAccess(groupsEvaluator, group, rep); - } - - return rep; + /** + * This method's purpose is to look up the subgroup count of a Group and populate it on the representation. This has been kept separate from + * {@link #toRepresentation} in order to keep database lookups separate from a function that aims to only convert objects + * A way of cohesively ensuring that a GroupRepresentation always has a group count should be considered + * + * @param group model + * @param representation group representation + * @return + */ + public static GroupRepresentation populateSubGroupCount(GroupModel group, GroupRepresentation representation) { + representation.setSubGroupCount(group.getSubGroupsCount()); + return representation; } //From org.keycloak.admin.ui.rest.GroupsResource // set fine-grained access for each group in the tree - private static void setAccess(GroupPermissionEvaluator groupsEvaluator, GroupModel groupTree, GroupRepresentation rootGroup) { - if (rootGroup == null) return; - - rootGroup.setAccess(groupsEvaluator.getAccess(groupTree)); - - rootGroup.getSubGroups().stream().forEach(subGroup -> { - GroupModel foundGroupModel = groupTree.getSubGroupsStream().filter(g -> g.getId().equals(subGroup.getId())).findFirst().get(); - setAccess(groupsEvaluator, foundGroupModel, subGroup); - }); - + public static GroupRepresentation toRepresentation(GroupPermissionEvaluator groupsEvaluator, GroupModel groupTree, boolean full) { + GroupRepresentation rep = ModelToRepresentation.toRepresentation(groupTree, full); + rep.setAccess(groupsEvaluator.getAccess(groupTree)); + return rep; } private static boolean groupMatchesSearchOrIsPathElement(GroupModel group, String search) { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java index 7ac4d3e963..b86a99005b 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/admin/ApiUtil.java @@ -20,6 +20,8 @@ import org.jboss.logging.Logger; import org.keycloak.admin.client.resource.AuthorizationResource; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientScopeResource; +import org.keycloak.admin.client.resource.GroupResource; +import org.keycloak.admin.client.resource.GroupsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RoleResource; import org.keycloak.admin.client.resource.UserResource; @@ -256,9 +258,9 @@ public class ApiUtil { } } - public static boolean groupContainsSubgroup(GroupRepresentation group, GroupRepresentation subgroup) { + public static boolean groupContainsSubgroup(GroupResource groupsResource, GroupRepresentation subgroup) { boolean contains = false; - for (GroupRepresentation sg : group.getSubGroups()) { + for (GroupRepresentation sg : groupsResource.getSubGroups(null,null, true)) { if (subgroup.getId().equals(sg.getId())) { contains = true; break; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java index a26687b541..375657c1f7 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java @@ -17,6 +17,8 @@ package org.keycloak.testsuite.admin.concurrency; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import org.junit.Test; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.ClientResource; @@ -233,11 +235,18 @@ public class ConcurrencyTest extends AbstractConcurrencyTest { c = realm.groups().group(id).toRepresentation(); assertNotNull(c); - assertTrue("Group " + name + " [" + id + "] " + " not found in group list", - realm.groups().groups().stream() - .map(GroupRepresentation::getName) - .filter(Objects::nonNull) - .anyMatch(name::equals)); + + boolean retry = true; + int i = 0; + do { + List groups = realm.groups().groups().stream() + .map(GroupRepresentation::getName) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + retry = !groups.contains(name); + i++; + } while(retry && i < 3); + assertFalse("Group " + name + " [" + id + "] " + " not found in group list", retry); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupSearchTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupSearchTest.java index 7d02798929..b8b126c15b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupSearchTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupSearchTest.java @@ -58,6 +58,7 @@ public class GroupSearchTest extends AbstractGroupTest { GroupRepresentation group3; GroupRepresentation parentGroup; GroupRepresentation childGroup; + GroupRepresentation secondChildGroup; @Before public void init() { @@ -66,6 +67,7 @@ public class GroupSearchTest extends AbstractGroupTest { group3 = new GroupRepresentation(); parentGroup = new GroupRepresentation(); childGroup = new GroupRepresentation(); + secondChildGroup = new GroupRepresentation(); group1.setAttributes(new HashMap<>() {{ put(ATTR_ORG_NAME, Collections.singletonList(ATTR_ORG_VAL)); @@ -82,7 +84,7 @@ public class GroupSearchTest extends AbstractGroupTest { put(ATTR_QUOTES_NAME, Collections.singletonList(ATTR_QUOTES_VAL)); }}); - childGroup.setAttributes(new HashMap<>() {{ + parentGroup.setAttributes(new HashMap<>() {{ put(ATTR_ORG_NAME, Collections.singletonList("parentOrg")); }}); @@ -95,6 +97,7 @@ public class GroupSearchTest extends AbstractGroupTest { group3.setName(GROUP3); parentGroup.setName(PARENT_GROUP); childGroup.setName(CHILD_GROUP); + secondChildGroup.setName(CHILD_GROUP + "2"); } public RealmResource testRealmResource() { 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 2d747ae62e..44a01db55a 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 @@ -40,7 +40,6 @@ import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.admin.ApiUtil; -import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.updaters.Creator; import org.keycloak.testsuite.util.AdminEventPaths; import org.keycloak.testsuite.util.ClientBuilder; @@ -83,7 +82,6 @@ import org.keycloak.models.AdminRoles; import org.keycloak.models.GroupModel; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; -import org.keycloak.models.utils.KeycloakModelUtils; import static org.hamcrest.MatcherAssert.assertThat; import static org.keycloak.testsuite.Assert.assertNames; @@ -426,15 +424,15 @@ public class GroupTest extends AbstractGroupTest { topGroup = realm.getGroupByPath("/top"); assertEquals(1, topGroup.getRealmRoles().size()); assertTrue(topGroup.getRealmRoles().contains("topRole")); - assertEquals(1, topGroup.getSubGroups().size()); + assertEquals(1, realm.groups().group(topGroup.getId()).getSubGroups(0, null, false).size()); - level2Group = topGroup.getSubGroups().get(0); + level2Group = realm.getGroupByPath("/top/level2"); assertEquals("level2", level2Group.getName()); assertEquals(1, level2Group.getRealmRoles().size()); assertTrue(level2Group.getRealmRoles().contains("level2Role")); - assertEquals(1, level2Group.getSubGroups().size()); + assertEquals(1, realm.groups().group(level2Group.getId()).getSubGroups(0, null, false).size()); - level3Group = level2Group.getSubGroups().get(0); + level3Group = realm.getGroupByPath("/top/level2/level3"); assertEquals("level3", level3Group.getName()); assertEquals(1, level3Group.getRealmRoles().size()); assertTrue(level3Group.getRealmRoles().contains("level3Role")); @@ -559,10 +557,11 @@ public class GroupTest extends AbstractGroupTest { response.close(); // Assert "mygroup2" was moved - group1 = realm.groups().group(group1.getId()).toRepresentation(); - group2 = realm.groups().group(group2.getId()).toRepresentation(); - assertNames(group1.getSubGroups(), "mygroup2"); - assertEquals("/mygroup1/mygroup2", group2.getPath()); + List group1Children = realm.groups().group(group1.getId()).getSubGroups(0, 10, false); + List group2Children = realm.groups().group(group2.getId()).getSubGroups(0, 10, false); + + assertNames(group1Children, "mygroup2"); + assertEquals("/mygroup1/mygroup2", realm.groups().group(group2.getId()).toRepresentation().getPath()); assertAdminEvents.clear(); @@ -583,10 +582,10 @@ public class GroupTest extends AbstractGroupTest { response.close(); // Assert "mygroup2" was moved - group1 = realm.groups().group(group1.getId()).toRepresentation(); - group2 = realm.groups().group(group2.getId()).toRepresentation(); - assertTrue(group1.getSubGroups().isEmpty()); - assertEquals("/mygroup2", group2.getPath()); + group1Children = realm.groups().group(group1.getId()).getSubGroups(0, 10, false); + group2Children = realm.groups().group(group2.getId()).getSubGroups(0, 10, false); + assertEquals(0, group1Children.size()); + assertEquals("/mygroup2", realm.groups().group(group2.getId()).toRepresentation().getPath()); } @Test @@ -1160,7 +1159,19 @@ public class GroupTest extends AbstractGroupTest { assertNotNull(group0); assertEquals(2,group0.getSubGroups().size()); assertThat(group0.getSubGroups().stream().map(GroupRepresentation::getName).collect(Collectors.toList()), Matchers.containsInAnyOrder("group1111", "group111111")); - assertEquals(new Long(search.size()), realm.groups().count("group11").get("count")); + assertEquals(countLeafGroups(search), realm.groups().count("group11").get("count")); + } + + private Long countLeafGroups(List search) { + long counter = 0; + for(GroupRepresentation group : search) { + if(group.getSubGroups().isEmpty()) { + counter += 1; + continue; + } + counter += countLeafGroups(group.getSubGroups()); + } + return counter; } @Test @@ -1192,7 +1203,7 @@ public class GroupTest extends AbstractGroupTest { Comparator compareByName = Comparator.comparing(GroupRepresentation::getName); // Assert that all groups are returned in order - List allGroups = realm.groups().groups(); + List allGroups = realm.groups().groups(0, 100); assertEquals(40, allGroups.size()); assertTrue(Comparators.isInStrictOrder(allGroups, compareByName)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupNamePolicyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupNamePolicyTest.java index cbff66c605..aab876d2cf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupNamePolicyTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupNamePolicyTest.java @@ -35,6 +35,7 @@ import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.authorization.client.AuthorizationDeniedException; import org.keycloak.authorization.client.AuthzClient; +import org.keycloak.authorization.model.Resource; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.mappers.GroupMembershipMapper; import org.keycloak.protocol.oidc.mappers.OIDCAttributeMapperHelper; @@ -259,7 +260,7 @@ public class GroupNamePolicyTest extends AbstractAuthzTest { continue; } - GroupRepresentation group = getGroup(part, parent.getSubGroups()); + GroupRepresentation group = getGroup(part, realm.groups().group(parent.getId()).getSubGroups(0, 10, true)); if (path.endsWith(group.getName())) { return group; @@ -272,12 +273,13 @@ public class GroupNamePolicyTest extends AbstractAuthzTest { } private GroupRepresentation getGroup(String name, List groups) { + RealmResource realm = getRealm(); for (GroupRepresentation group : groups) { if (name.equals(group.getName())) { return group; } - GroupRepresentation child = getGroup(name, group.getSubGroups()); + GroupRepresentation child = getGroup(name, realm.groups().group(group.getId()).getSubGroups(0, 10, true)); if (child != null && name.equals(child.getName())) { return child; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupPathPolicyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupPathPolicyTest.java index c94447ff92..1452a1058a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupPathPolicyTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/GroupPathPolicyTest.java @@ -235,7 +235,7 @@ public class GroupPathPolicyTest extends AbstractAuthzTest { continue; } - GroupRepresentation group = getGroup(part, parent.getSubGroups()); + GroupRepresentation group = getGroup(part, realm.groups().group(parent.getId()).getSubGroups(0, 10, true)); if (path.endsWith(group.getName())) { return group; @@ -248,12 +248,13 @@ public class GroupPathPolicyTest extends AbstractAuthzTest { } private GroupRepresentation getGroup(String name, List groups) { + RealmResource realm = getRealm(); for (GroupRepresentation group : groups) { if (name.equals(group.getName())) { return group; } - GroupRepresentation child = getGroup(name, group.getSubGroups()); + GroupRepresentation child = getGroup(name, realm.groups().group(group.getId()).getSubGroups(0, 10, true)); if (child != null && name.equals(child.getName())) { return child; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cluster/GroupInvalidationClusterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cluster/GroupInvalidationClusterTest.java index d6eb0f962d..613f3f588b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cluster/GroupInvalidationClusterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cluster/GroupInvalidationClusterTest.java @@ -4,6 +4,7 @@ import org.apache.commons.lang.RandomStringUtils; import org.junit.Before; import org.keycloak.admin.client.resource.GroupResource; import org.keycloak.admin.client.resource.GroupsResource; +import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.ContainerInfo; @@ -45,6 +46,10 @@ public class GroupInvalidationClusterTest extends AbstractInvalidationClusterTes return getAdminClientFor(node).realm(testRealmName).groups(); } + protected RealmResource realm(ContainerInfo node) { + return getAdminClientFor(node).realm(testRealmName); + } + @Override protected GroupResource entityResource(GroupRepresentation group, ContainerInfo node) { return entityResource(group.getId(), node); @@ -129,7 +134,7 @@ public class GroupInvalidationClusterTest extends AbstractInvalidationClusterTes parentGroup = readEntityOnCurrentFailNode(parentGroup); group = readEntityOnCurrentFailNode(group); - assertTrue(ApiUtil.groupContainsSubgroup(parentGroup, group)); + assertTrue(ApiUtil.groupContainsSubgroup(entityResourceOnCurrentFailNode(parentGroup), group)); assertEquals(parentGroup.getPath() + "/" + group.getName(), group.getPath()); verifyEntityUpdateDuringFailover(group, backendFailover); @@ -149,8 +154,8 @@ public class GroupInvalidationClusterTest extends AbstractInvalidationClusterTes // Verify same child groups on both nodes GroupRepresentation parentGroupOnOtherNode = readEntityOnCurrentFailNode(parentGroup); - assertNames(parentGroup.getSubGroups(), group.getName(), "childGroup2"); - assertNames(parentGroupOnOtherNode.getSubGroups(), group.getName(), "childGroup2"); + assertNames(entityResourceOnCurrentFailNode(parentGroup).getSubGroups(0, 20, true), group.getName(), "childGroup2"); + assertNames(entityResourceOnCurrentFailNode(parentGroupOnOtherNode).getSubGroups(0, 20, true), group.getName(), "childGroup2"); // Remove childGroup2 deleteEntityOnCurrentFailNode(childGroup2);