From 73378df52ee288cad17191f8a40cfc23cee22f92 Mon Sep 17 00:00:00 2001 From: Clement Cureau Date: Thu, 16 Jan 2020 15:33:13 +0100 Subject: [PATCH] [KEYCLOAK-11621] Allow user creation via group permissions (Admin API) Problem: Using fine-grained admin permissions on groups, it is not permitted to create new users within a group. Cause: The POST /{realm}/users API does not check permission for each group part of the new user representation Solution: - Change access logic for POST /{realm}/users to require MANAGE_MEMBERS and MANAGE_MEMBERSHIP permissions on each of the incoming groups Tests: Manual API testing performed: 1. admin user from master realm: - POST /{realm}/users without groups => HTTP 201 user created - POST /{realm}/users with groups => HTTP 201 user created 2. user with MANAGE_MEMBERS & MANAGE_MEMBERSHIP permissions on group1 - POST /{realm}/users without groups => HTTP 403 user NOT created - POST /{realm}/users with group1 => HTTP 201 user created - POST /{realm}/users with group1 & group2 => HTTP 403 user NOT created - POST /{realm}/users with group1 & wrong group path => HTTP 400 user NOT created 3. user with MANAGE_MEMBERS permission on group1 - POST /{realm}/users without groups => HTTP 403 user NOT created - POST /{realm}/users with group1 => HTTP 403 user NOT created - POST /{realm}/users with group1 & group2 => HTTP 403 user NOT created - POST /{realm}/users with group1 & wrong group path => HTTP 400 user NOT created --- .../services/resources/admin/UsersResource.java | 17 ++++++++++++++++- .../permissions/GroupPermissionEvaluator.java | 2 ++ .../admin/permissions/GroupPermissions.java | 7 +++++++ .../admin/messages/admin-messages_de.properties | 4 ++-- .../admin/messages/admin-messages_en.properties | 4 ++-- 5 files changed, 29 insertions(+), 5 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index 47d2c1fc89..0381c9a416 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -25,12 +25,14 @@ import org.keycloak.common.util.ObjectUtil; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.models.Constants; +import org.keycloak.models.GroupModel; import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.ModelException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.RepresentationToModel; import org.keycloak.policy.PasswordPolicyNotMetException; @@ -107,7 +109,20 @@ public class UsersResource { @POST @Consumes(MediaType.APPLICATION_JSON) public Response createUser(final UserRepresentation rep) { - auth.users().requireManage(); + // if groups is part of the user rep, check if admin has manage_members and manage_membership on each group + if (rep.getGroups() != null) { + for (String groupPath : rep.getGroups()) { + GroupModel group = KeycloakModelUtils.findGroupByPath(realm, groupPath); + if (group != null) { + auth.groups().requireManageMembers(group); + auth.groups().requireManageMembership(group); + } else { + return ErrorResponse.error(String.format("Group %s not found", groupPath), Response.Status.BAD_REQUEST); + } + } + } else { + auth.users().requireManage(); + } String username = rep.getUsername(); if(realm.isRegistrationEmailAsUsername()) { 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 831e9f0667..f82c72ef08 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 @@ -56,6 +56,8 @@ public interface GroupPermissionEvaluator { void requireManageMembership(GroupModel group); + void requireManageMembers(GroupModel group); + Map getAccess(GroupModel group); Set getGroupsWithViewPermission(); 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 d51dee2b46..3c45a5e575 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 @@ -367,6 +367,13 @@ class GroupPermissions implements GroupPermissionEvaluator, GroupPermissionManag } } + @Override + public void requireManageMembers(GroupModel group) { + if (!canManageMembers(group)) { + throw new ForbiddenException(); + } + } + @Override public Map getAccess(GroupModel group) { Map map = new HashMap<>(); diff --git a/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties b/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties index 9442a19eea..6ed0016338 100644 --- a/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties +++ b/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties @@ -1575,7 +1575,7 @@ permissions-enabled-users.tooltip=Legt fest, ob feingranulare Berechtigungen f\u #manage-permissions-group.tooltip=Fine grain permissions for admins that want to manage this group or the members of this group. #manage-authz-group-scope-description=Policies that decide if an admin can manage this group #view-authz-group-scope-description=Policies that decide if an admin can view this group -#view-members-authz-group-scope-description=Policies that decide if an admin can manage the members of this group +#view-members-authz-group-scope-description=Policies that decide if an admin can view the members of this group #token-exchange-authz-client-scope-description=Policies that decide which clients are allowed exchange tokens for a token that is targeted to this client. #token-exchange-authz-idp-scope-description=Policies that decide which clients are allowed exchange tokens for an external token minted by this identity provider. #manage-authz-client-scope-description=Policies that decide if an admin can manage this client @@ -1591,7 +1591,7 @@ permissions-enabled-users.tooltip=Legt fest, ob feingranulare Berechtigungen f\u #impersonate-authz-users-scope-description=Policies that decide if admin can impersonate other users #map-roles-authz-users-scope-description=Policies that decide if admin can map roles for all users #user-impersonated-authz-users-scope-description=Policies that decide which users can be impersonated. These policies are applied to the user being impersonated. -#manage-membership-authz-group-scope-description=Policies that decide if admin can add or remove users from this group +#manage-membership-authz-group-scope-description=Policies that decide if an admin can add or remove users from this group #manage-members-authz-group-scope-description=Policies that decide if an admin can manage the members of this group # KEYCLOAK-6771 Certificate Bound Token diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties index f3c7b59902..0e5d29f390 100644 --- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties +++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties @@ -1714,7 +1714,7 @@ manage-permissions-client.tooltip=Fine grained permissions for administrators th manage-permissions-group.tooltip=Fine grained permissions for administrators that want to manage this group or the members of this group. manage-authz-group-scope-description=Policies that decide if an administrator can manage this group view-authz-group-scope-description=Policies that decide if an administrator can view this group -view-members-authz-group-scope-description=Policies that decide if an administrator can manage the members of this group +view-members-authz-group-scope-description=Policies that decide if an administrator can view the members of this group token-exchange-authz-client-scope-description=Policies that decide which clients are allowed exchange tokens for a token that is targeted to this client. token-exchange-authz-idp-scope-description=Policies that decide which clients are allowed exchange tokens for an external token minted by this identity provider. manage-authz-client-scope-description=Policies that decide if an administrator can manage this client @@ -1730,7 +1730,7 @@ manage-group-membership-authz-users-scope-description=Policies that decide if an impersonate-authz-users-scope-description=Policies that decide if administrator can impersonate other users map-roles-authz-users-scope-description=Policies that decide if administrator can map roles for all users user-impersonated-authz-users-scope-description=Policies that decide which users can be impersonated. These policies are applied to the user being impersonated. -manage-membership-authz-group-scope-description=Policies that decide if administrator can add or remove users from this group +manage-membership-authz-group-scope-description=Policies that decide if an administrator can add or remove users from this group manage-members-authz-group-scope-description=Policies that decide if an administrator can manage the members of this group # KEYCLOAK-6771 Certificate Bound Token