Skip to content

Commit da170a1

Browse files
SancretorAlexis CHATILLONPatAKnight
authored
feat(rbac): support direct group membership as superUsers (#7813)
* feat(rbac): support direct group membership as superUsers Signed-off-by: Alexis CHATILLON <[email protected]> * chore(rbac): update changeset to minor Co-authored-by: Patrick Knight <[email protected]> Signed-off-by: Alexis Chatillon <[email protected]> --------- Signed-off-by: Alexis CHATILLON <[email protected]> Signed-off-by: Alexis Chatillon <[email protected]> Co-authored-by: Alexis CHATILLON <[email protected]> Co-authored-by: Patrick Knight <[email protected]>
1 parent 2a57830 commit da170a1

File tree

4 files changed

+80
-1
lines changed

4 files changed

+80
-1
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@backstage-community/plugin-rbac-backend': minor
3+
---
4+
5+
Add support for group reference in superUsers list, using direct membership only

workspaces/rbac/plugins/rbac-backend/README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,11 @@ permission:
8181
superUsers:
8282
- name: user:default/alice
8383
- name: user:default/mike
84+
- name: group:default/admins
8485
```
8586
87+
> **Note:** **Transient memberships are not supported for `superUsers`.** Meaning, when a group is specified as a super user, only direct group memberships are taken into account. Users who belong to a sub-group of a configured super user group will not be granted super user access.
88+
8689
For more information on the available API endpoints accessible to the policy administrators, refer to the [API documentation](./docs/apis.md).
8790

8891
### Configure plugins with permission

workspaces/rbac/plugins/rbac-backend/src/policies/permission-policy.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,72 @@ describe('RBACPermissionPolicy Tests', () => {
10361036
);
10371037
});
10381038

1039+
it('should allow access to a user who is a member of a group configured as super user', async () => {
1040+
const superUsersConfig = new Array<{ name: string }>();
1041+
superUsersConfig.push({ name: 'group:default/super_users_group' });
1042+
1043+
const config = newConfig(csvPermFile, admins, superUsersConfig);
1044+
const adapter = await newAdapter(config);
1045+
const enfDelegateForTest = await newEnforcerDelegate(adapter, config);
1046+
const policyForTest = await newPermissionPolicy(
1047+
config,
1048+
enfDelegateForTest,
1049+
roleMetadataStorageTest,
1050+
);
1051+
1052+
const decision = await policyForTest.handle(
1053+
newPolicyQueryWithResourcePermission(
1054+
'catalog.entity.delete',
1055+
'catalog-entity',
1056+
'delete',
1057+
),
1058+
newPolicyQueryUser('user:default/some_user', [
1059+
'group:default/super_users_group',
1060+
]),
1061+
);
1062+
expect(decision.result).toBe(AuthorizeResult.ALLOW);
1063+
expectAuditorLogForPermission(
1064+
'user:default/some_user',
1065+
'catalog.entity.delete',
1066+
'catalog-entity',
1067+
'delete',
1068+
AuthorizeResult.ALLOW,
1069+
);
1070+
});
1071+
1072+
it('should deny access to a user who is not a member of a group configured as super user', async () => {
1073+
const superUsersConfig = new Array<{ name: string }>();
1074+
superUsersConfig.push({ name: 'group:default/super_users_group' });
1075+
1076+
const config = newConfig(csvPermFile, admins, superUsersConfig);
1077+
const adapter = await newAdapter(config);
1078+
const enfDelegateForTest = await newEnforcerDelegate(adapter, config);
1079+
const policyForTest = await newPermissionPolicy(
1080+
config,
1081+
enfDelegateForTest,
1082+
roleMetadataStorageTest,
1083+
);
1084+
1085+
const decision = await policyForTest.handle(
1086+
newPolicyQueryWithResourcePermission(
1087+
'catalog.entity.delete',
1088+
'catalog-entity',
1089+
'delete',
1090+
),
1091+
newPolicyQueryUser('user:default/some_user', [
1092+
'group:default/other_group',
1093+
]),
1094+
);
1095+
expect(decision.result).toBe(AuthorizeResult.DENY);
1096+
expectAuditorLogForPermission(
1097+
'user:default/some_user',
1098+
'catalog.entity.delete',
1099+
'catalog-entity',
1100+
'delete',
1101+
AuthorizeResult.DENY,
1102+
);
1103+
});
1104+
10391105
it('should remove users that are no longer in the config file', async () => {
10401106
const enfRole = await enfDelegate.getFilteredGroupingPolicy(1, adminRole);
10411107
const enfPermission = await enfDelegate.getFilteredPolicy(0, adminRole);

workspaces/rbac/plugins/rbac-backend/src/policies/permission-policy.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,12 @@ export class RBACPermissionPolicy implements PermissionPolicy {
199199
return { result: AuthorizeResult.DENY };
200200
}
201201

202-
if (this.superUserList!.includes(userEntityRef)) {
202+
if (
203+
this.superUserList!.includes(userEntityRef) ||
204+
user.info.ownershipEntityRefs.some(ref =>
205+
this.superUserList!.includes(ref),
206+
)
207+
) {
203208
await auditorEvent.success({
204209
meta: { result: AuthorizeResult.ALLOW },
205210
});

0 commit comments

Comments
 (0)