From c595e3430e1952b023f6db79d9ae8f0fa6340294 Mon Sep 17 00:00:00 2001 From: Stan Silvert Date: Fri, 31 Mar 2023 15:11:13 -0400 Subject: [PATCH] Add access to full group tree. Fix access for members tab. Add missing (#19423) props to Access object. Fixes #17589 --- .../admin-ui/public/resources/en/groups.json | 3 +- js/apps/admin-ui/src/groups/GroupTable.tsx | 4 +-- js/apps/admin-ui/src/groups/GroupsSection.tsx | 20 ++++++++----- .../src/groups/components/GroupTree.tsx | 21 ++++++++++---- .../admin/ui/rest/GroupsResource.java | 29 ++++++++++++++----- .../permissions/GroupPermissionEvaluator.java | 4 ++- .../admin/permissions/GroupPermissions.java | 18 +++++++++++- 7 files changed, 73 insertions(+), 26 deletions(-) diff --git a/js/apps/admin-ui/public/resources/en/groups.json b/js/apps/admin-ui/public/resources/en/groups.json index 011beedfc5..ada2fdddb2 100644 --- a/js/apps/admin-ui/public/resources/en/groups.json +++ b/js/apps/admin-ui/public/resources/en/groups.json @@ -66,5 +66,6 @@ "groupUpdateError": "Error updating group {{error}}", "roleMapping": "Role mapping", "noRoles": "No roles for this group", - "noRolesInstructions": "You haven't created any roles for this group. Create a role to get started." + "noRolesInstructions": "You haven't created any roles for this group. Create a role to get started.", + "noViewRights": "You do not have rights to view this group." } diff --git a/js/apps/admin-ui/src/groups/GroupTable.tsx b/js/apps/admin-ui/src/groups/GroupTable.tsx index d3c6ddafb2..86934c8cc4 100644 --- a/js/apps/admin-ui/src/groups/GroupTable.tsx +++ b/js/apps/admin-ui/src/groups/GroupTable.tsx @@ -39,7 +39,7 @@ export const GroupTable = ({ const [showDelete, toggleShowDelete] = useToggle(); const [move, setMove] = useState(); - const { subGroups, currentGroup, setSubGroups } = useSubGroups(); + const { currentGroup } = useSubGroups(); const [key, setKey] = useState(0); const refresh = () => setKey(key + 1); @@ -212,7 +212,7 @@ export const GroupTable = ({ setSubGroups([...subGroups, group])} + onClick={() => navigate(toGroups({ realm, id: group.id }))} > {group.name} diff --git a/js/apps/admin-ui/src/groups/GroupsSection.tsx b/js/apps/admin-ui/src/groups/GroupsSection.tsx index e36dbd31b9..713d6c7670 100644 --- a/js/apps/admin-ui/src/groups/GroupsSection.tsx +++ b/js/apps/admin-ui/src/groups/GroupsSection.tsx @@ -69,6 +69,10 @@ export default function GroupsSection() { const canViewDetails = hasAccess("query-groups", "view-users") || hasAccess("manage-users", "query-groups"); + const canViewMembers = + hasAccess("view-users") || + currentGroup()?.access?.viewMembers || + currentGroup()?.access?.manageMembers; useFetch( async () => { @@ -177,13 +181,15 @@ export default function GroupsSection() { canViewDetails={canViewDetails} /> - {t("members")}} - > - - + {canViewMembers && ( + {t("members")}} + > + + + )} (); const [groups, setGroups] = useState([]); @@ -147,7 +152,7 @@ export const GroupTree = ({ group.subGroups && group.subGroups.length > 0 ? group.subGroups.map((g) => mapGroup(g, groups, refresh)) : undefined, - action: canViewDetails && ( + action: (hasAccess("manage-users") || group.access?.manage) && ( ), defaultExpanded: subGroups.map((g) => g.id).includes(group.id), @@ -232,12 +237,16 @@ export const GroupTree = ({ className="keycloak_groups_treeview" onSelect={(_, item) => { setActiveItem(item); - if (canViewDetails) { - const id = item.id?.substring(item.id.lastIndexOf("/") + 1); - const subGroups: GroupRepresentation[] = []; - findGroup(groups, id!, [], subGroups); - setSubGroups(subGroups); + const id = item.id?.substring(item.id.lastIndexOf("/") + 1); + const subGroups: GroupRepresentation[] = []; + findGroup(groups, id!, [], subGroups); + setSubGroups(subGroups); + + if (canViewDetails || subGroups.at(-1)?.access?.view) { navigate(toGroups({ realm, id: item.id })); + } else { + addAlert(t("noViewRights"), AlertVariant.warning); + navigate(toGroups({ realm })); } }} /> 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 index bbfa03664a..ddaf241014 100644 --- 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 @@ -1,5 +1,6 @@ package org.keycloak.admin.ui.rest; +import java.util.List; import static org.keycloak.models.utils.ModelToRepresentation.toRepresentation; import java.util.stream.Collectors; @@ -68,23 +69,35 @@ public class GroupsResource { private GroupRepresentation toGroupHierarchy(GroupModel group, final String search, boolean exact) { GroupRepresentation rep = toRepresentation(group, true); - rep.setAccess(auth.groups().getAccess(group)); rep.setSubGroups(group.getSubGroupsStream().filter(g -> groupMatchesSearchOrIsPathElement( g, search ) - ).map(subGroup -> { - final GroupRepresentation subRep = ModelToRepresentation.toGroupHierarchy( - subGroup, true, search, exact - ); - subRep.setAccess(auth.groups().getAccess(subGroup)); - return subRep; - } + ).map(subGroup -> + ModelToRepresentation.toGroupHierarchy( + subGroup, true, search, exact + ) + ).collect(Collectors.toList())); + setAccess(group, rep); + return rep; } + // set fine-grained access for each group in the tree + private void setAccess(GroupModel groupTree, GroupRepresentation rootGroup) { + if (rootGroup == null) return; + + rootGroup.setAccess(auth.groups().getAccess(groupTree)); + + rootGroup.getSubGroups().stream().forEach(subGroup -> { + GroupModel foundGroupModel = groupTree.getSubGroupsStream().filter(g -> g.getId().equals(subGroup.getId())).findFirst().get(); + setAccess(foundGroupModel, subGroup); + }); + + } + private static boolean groupMatchesSearchOrIsPathElement(GroupModel group, String search) { if (StringUtil.isBlank(search)) { return true; diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java index f82c72ef08..c90eba237d 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java @@ -53,7 +53,9 @@ public interface GroupPermissionEvaluator { boolean canManageMembers(GroupModel group); boolean canManageMembership(GroupModel group); - + + boolean canViewMembers(GroupModel group); + void requireManageMembership(GroupModel group); void requireManageMembers(GroupModel group); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java index 6e5f0699ff..f576faa5a7 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java @@ -343,6 +343,20 @@ class GroupPermissions implements GroupPermissionEvaluator, GroupPermissionManag } } + @Override + public boolean canViewMembers(GroupModel group) { + if (root.users().canView()) return true; + + if (!root.isAdminSameRealm()) { + return false; + } + + ResourceServer server = root.realmResourceServer(); + if (server == null) return false; + + return hasPermission(group, VIEW_MEMBERS_SCOPE); + } + @Override public boolean canManageMembers(GroupModel group) { if (root.users().canManage()) return true; @@ -367,7 +381,7 @@ class GroupPermissions implements GroupPermissionEvaluator, GroupPermissionManag return hasPermission(group, MANAGE_MEMBERSHIP_SCOPE); } - + @Override public void requireManageMembership(GroupModel group) { if (!canManageMembership(group)) { @@ -388,6 +402,8 @@ class GroupPermissions implements GroupPermissionEvaluator, GroupPermissionManag map.put("view", canView(group)); map.put("manage", canManage(group)); map.put("manageMembership", canManageMembership(group)); + map.put("viewMembers", canViewMembers(group)); + map.put("manageMembers", canManageMembers(group)); return map; }