mirror of
https://github.com/keycloak/keycloak.git
synced 2026-05-28 04:13:22 -04:00
Fix FGAP deny evaluation for manage-group-membership
Add the missing Users FGAP scope alias from manage-group-membership to Groups manage-membership so deny permissions on group members apply during user membership updates. Add a regression test covering a protected group member, an unrelated user, and the no-mutation postcondition after a forbidden request. Closes keycloak#46693
This commit is contained in:
parent
db44c8a38c
commit
24a819eabc
4 changed files with 58 additions and 32 deletions
|
|
@ -114,3 +114,10 @@ bin/kc.sh start --log="console,file" --log-file-rotation-enabled=false
|
|||
----
|
||||
|
||||
For more information, see the https://www.keycloak.org/server/logging/file[File logging] guide.
|
||||
|
||||
= Fine-Grained Admin Permissions new Groups scope for user membership changes
|
||||
|
||||
Fine-Grained Admin Permissions (FGAP) now includes a new `Groups` scope: `manage-membership-of-members`.
|
||||
|
||||
This scope is now used as the group-side bridge for evaluating user-side `manage-group-membership` permissions based on a user's current group memberships.
|
||||
The existing `manage-membership` scope keeps its current behavior for target group membership management operations.
|
||||
|
|
|
|||
|
|
@ -86,7 +86,7 @@ set of scopes:
|
|||
|
||||
| *view* | Defines if a realm administrator can view users. This scope should be set whenever you want to make users available from queries. | `view-members`
|
||||
| *manage* | Defines if a realm administrator can manage users. | `manage-members`
|
||||
| *manage-group-membership* | Defines if a realm administrator can assign or unassign users to/from groups. | None
|
||||
| *manage-group-membership* | Defines if a realm administrator can assign or unassign users to/from groups. | `manage-membership-of-members`
|
||||
| *map-roles* | Defines if a realm administrator can assign or unassign roles to/from users. | None
|
||||
| *impersonate* | Defines if a realm administrator can impersonate other users. | `impersonate-members`
|
||||
| *reset-password* | Defines if a realm administrator can reset user passwords. If no permission with `reset-password` is found, it falls back to check `manage` scope. | None
|
||||
|
|
@ -97,6 +97,11 @@ The user resource type has a strong relationship with some of the permissions yo
|
|||
users are members of groups and granting access to `view-members` or `manage-members` of a group should also allow
|
||||
a realm administrator to `view` and `manage` members of that group.
|
||||
|
||||
To assign or unassign a specific user to or from a group, both sides of the relationship are required: `manage-group-membership`
|
||||
on the user and `manage-membership` on the target group.
|
||||
The `manage-membership-of-members` scope can be used to grant or deny `manage-group-membership` based on the groups that
|
||||
the target user currently belongs to.
|
||||
|
||||
[NOTE]
|
||||
====
|
||||
This feature does not support enforcing access to federated resource, however, this limitation is being considered
|
||||
|
|
@ -125,6 +130,7 @@ set of management operations:
|
|||
This operation applies to any child group in the group hierarchy.
|
||||
This can be prevented by explicitly denying permission for specific subgroups.
|
||||
| *manage-membership* | Defines if a realm administrator can add or remove members from groups.
|
||||
| *manage-membership-of-members* | Defines if a realm administrator can grant or deny `manage-group-membership` for members of a group.
|
||||
|===
|
||||
|
||||
===== Clients Resource Type
|
||||
|
|
@ -374,7 +380,7 @@ if a respective admin role is assigned to a realm administrator, permission eval
|
|||
It does not grant the ability to *view* clients.
|
||||
| *view-users* | A realm administrator can *view* all users and groups in the realm.
|
||||
| *manage-users* | A realm administrator can *view*, *map-roles*, *manage-group-membership* and *manage* all users in the realm,
|
||||
as well as *view*, *manage-membership* and *manage* groups in the realm.
|
||||
as well as *view*, *manage-membership*, *manage-membership-of-members* and *manage* groups in the realm.
|
||||
| *impersonation* | A realm administrator can *impersonate* all users in the realm.
|
||||
| *view-clients* | A realm administrator can *view* all clients in the realm.
|
||||
| *manage-clients* | A realm administrator can *view* and *manage* all clients and client scopes in the realm.
|
||||
|
|
|
|||
|
|
@ -85,6 +85,7 @@ public class AdminPermissionsSchema extends AuthorizationSchema {
|
|||
|
||||
// group specific scopes
|
||||
public static final String MANAGE_MEMBERSHIP = "manage-membership";
|
||||
public static final String MANAGE_MEMBERSHIP_OF_MEMBERS = "manage-membership-of-members";
|
||||
public static final String MANAGE_MEMBERS = "manage-members";
|
||||
public static final String VIEW_MEMBERS = "view-members";
|
||||
public static final String IMPERSONATE_MEMBERS = "impersonate-members";
|
||||
|
|
@ -101,9 +102,9 @@ public class AdminPermissionsSchema extends AuthorizationSchema {
|
|||
public static final String MANAGE_GROUP_MEMBERSHIP = "manage-group-membership";
|
||||
|
||||
public static final ResourceType CLIENTS = new ResourceType(CLIENTS_RESOURCE_TYPE, Set.of(MANAGE, MAP_ROLES, MAP_ROLES_CLIENT_SCOPE, MAP_ROLES_COMPOSITE, VIEW));
|
||||
public static final ResourceType GROUPS = new ResourceType(GROUPS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, MANAGE_MEMBERSHIP, MANAGE_MEMBERS, VIEW_MEMBERS, IMPERSONATE_MEMBERS));
|
||||
public static final ResourceType GROUPS = new ResourceType(GROUPS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, MANAGE_MEMBERSHIP, MANAGE_MEMBERSHIP_OF_MEMBERS, MANAGE_MEMBERS, VIEW_MEMBERS, IMPERSONATE_MEMBERS));
|
||||
public static final ResourceType ROLES = new ResourceType(ROLES_RESOURCE_TYPE, Set.of(MAP_ROLE, MAP_ROLE_CLIENT_SCOPE, MAP_ROLE_COMPOSITE));
|
||||
public static final ResourceType USERS = new ResourceType(USERS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, IMPERSONATE, MAP_ROLES, MANAGE_GROUP_MEMBERSHIP, RESET_PASSWORD), Map.of(VIEW, Set.of(VIEW_MEMBERS), MANAGE, Set.of(MANAGE_MEMBERS), IMPERSONATE, Set.of(IMPERSONATE_MEMBERS)), GROUPS.getType());
|
||||
public static final ResourceType USERS = new ResourceType(USERS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, IMPERSONATE, MAP_ROLES, MANAGE_GROUP_MEMBERSHIP, RESET_PASSWORD), Map.of(VIEW, Set.of(VIEW_MEMBERS), MANAGE, Set.of(MANAGE_MEMBERS), IMPERSONATE, Set.of(IMPERSONATE_MEMBERS), MANAGE_GROUP_MEMBERSHIP, Set.of(MANAGE_MEMBERSHIP_OF_MEMBERS)), GROUPS.getType());
|
||||
private static final String SKIP_EVALUATION = "kc.authz.fgap.skip";
|
||||
public static final AdminPermissionsSchema SCHEMA = new AdminPermissionsSchema();
|
||||
|
||||
|
|
@ -533,4 +534,4 @@ public class AdminPermissionsSchema extends AuthorizationSchema {
|
|||
|
||||
return Boolean.parseBoolean(session.getAttributeOrDefault(SKIP_EVALUATION, Boolean.FALSE.toString()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,6 +54,8 @@ import org.junit.jupiter.api.Test;
|
|||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.IMPERSONATE;
|
||||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.MANAGE;
|
||||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.MANAGE_GROUP_MEMBERSHIP;
|
||||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.MANAGE_MEMBERSHIP;
|
||||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.MANAGE_MEMBERSHIP_OF_MEMBERS;
|
||||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.MAP_ROLES;
|
||||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.RESET_PASSWORD;
|
||||
import static org.keycloak.authorization.fgap.AdminPermissionsSchema.VIEW;
|
||||
|
|
@ -309,50 +311,60 @@ public class UserResourceTypeEvaluationTest extends AbstractPermissionTest {
|
|||
|
||||
@Test
|
||||
public void testManageGroupMembership() {
|
||||
UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0);
|
||||
UserPolicyRepresentation allowMyAdminPermission = createUserPolicy(realm, client, "Only My Admin User Policy", myadmin.getId());
|
||||
GroupRepresentation target = createGroup("target");
|
||||
UserRepresentation userBob = createUser("bob");
|
||||
|
||||
// myadmin shouldn't be able to manage group membership of the user just yet
|
||||
try {
|
||||
realmAdminClient.realm(realm.getName()).users().get(userAlice.getId()).joinGroup("no-such");
|
||||
realmAdminClient.realm(realm.getName()).users().get(userBob.getId()).joinGroup(target.getId());
|
||||
fail("Expected Exception wasn't thrown.");
|
||||
} catch (Exception ex) {
|
||||
assertThat(ex, instanceOf(ForbiddenException.class));
|
||||
}
|
||||
|
||||
//create all-users permission for "myadmin" (so that myadmin can add users into a group)
|
||||
UserPolicyRepresentation policy = createUserPolicy(realm, client,"Only My Admin User Policy", realm.admin().users().search("myadmin").get(0).getId());
|
||||
ScopePermissionRepresentation allUsersPermission = createAllPermission(client, usersType, policy, Set.of(MANAGE_GROUP_MEMBERSHIP));
|
||||
|
||||
//check myadmin can manage membership using all-users permission
|
||||
// only the user-side permission is not enough
|
||||
createAllPermission(client, usersType, allowMyAdminPermission, Set.of(MANAGE_GROUP_MEMBERSHIP));
|
||||
try {
|
||||
realmAdminClient.realm(realm.getName()).users().get(userAlice.getId()).joinGroup("no-such");
|
||||
fail("Expected Exception wasn't thrown.");
|
||||
} catch (Exception ex) {
|
||||
// expecting here NotFoundException: https://github.com/keycloak/keycloak/blob/b5c95e9f1c58bc500316dd5c0f2d3bb5e197ca99/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java#L1060
|
||||
assertThat(ex, instanceOf(NotFoundException.class));
|
||||
}
|
||||
|
||||
// remove all-users permissions to test user-permission
|
||||
allUsersPermission = getScopePermissionsResource(client).findByName(allUsersPermission.getName());
|
||||
getScopePermissionsResource(client).findById(allUsersPermission.getId()).remove();
|
||||
|
||||
// now myadmin cannot manage membership
|
||||
try {
|
||||
realmAdminClient.realm(realm.getName()).users().get(userAlice.getId()).joinGroup("no-such");
|
||||
realmAdminClient.realm(realm.getName()).users().get(userBob.getId()).joinGroup(target.getId());
|
||||
fail("Expected Exception wasn't thrown.");
|
||||
} catch (Exception ex) {
|
||||
assertThat(ex, instanceOf(ForbiddenException.class));
|
||||
}
|
||||
|
||||
// create userPermission
|
||||
createPermission(client, userAlice.getId(), usersType, Set.of(MANAGE_GROUP_MEMBERSHIP), policy);
|
||||
// target group must allow membership changes
|
||||
createAllPermission(client, AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, allowMyAdminPermission, Set.of(MANAGE_MEMBERSHIP));
|
||||
realmAdminClient.realm(realm.getName()).users().get(userBob.getId()).joinGroup(target.getId());
|
||||
assertTrue(realm.admin().users().get(userBob.getId()).groups().stream().anyMatch(group -> target.getId().equals(group.getId())));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testManageGroupMembershipDeniedForProtectedGroupMembers() {
|
||||
UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0);
|
||||
UserPolicyRepresentation allowMyAdminPermission = createUserPolicy(realm, client, "Only My Admin User Policy", myadmin.getId());
|
||||
UserPolicyRepresentation denyMyAdminPermission = createUserPolicy(Logic.NEGATIVE, realm, client, "Not My Admin User Policy", myadmin.getId());
|
||||
GroupRepresentation vault = createGroup("vault");
|
||||
GroupRepresentation target = createGroup("target");
|
||||
UserRepresentation userBob = createUser("bob");
|
||||
|
||||
realm.admin().users().get(userAlice.getId()).joinGroup(vault.getId());
|
||||
|
||||
createAllPermission(client, usersType, allowMyAdminPermission, Set.of(MANAGE_GROUP_MEMBERSHIP));
|
||||
createAllPermission(client, AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, allowMyAdminPermission, Set.of(MANAGE_MEMBERSHIP));
|
||||
createAllPermission(client, AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, allowMyAdminPermission, Set.of(MANAGE_MEMBERSHIP_OF_MEMBERS));
|
||||
createPermission(client, vault.getId(), AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, Set.of(MANAGE_MEMBERSHIP_OF_MEMBERS), denyMyAdminPermission);
|
||||
|
||||
realmAdminClient.realm(realm.getName()).users().get(userBob.getId()).joinGroup(target.getId());
|
||||
assertTrue(realm.admin().users().get(userBob.getId()).groups().stream().anyMatch(group -> target.getId().equals(group.getId())));
|
||||
|
||||
//check myadmin can manage membership using individual permission
|
||||
try {
|
||||
realmAdminClient.realm(realm.getName()).users().get(userAlice.getId()).joinGroup("no-such");
|
||||
realmAdminClient.realm(realm.getName()).users().get(userAlice.getId()).joinGroup(target.getId());
|
||||
fail("Expected Exception wasn't thrown.");
|
||||
} catch (Exception ex) {
|
||||
// expecting here NotFoundException: https://github.com/keycloak/keycloak/blob/b5c95e9f1c58bc500316dd5c0f2d3bb5e197ca99/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java#L1060
|
||||
assertThat(ex, instanceOf(NotFoundException.class));
|
||||
} catch (ForbiddenException expected) {
|
||||
}
|
||||
|
||||
assertFalse(realm.admin().users().get(userAlice.getId()).groups().stream().anyMatch(group -> target.getId().equals(group.getId())));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
Loading…
Reference in a new issue