Skip to content

Commit

Permalink
chore(release): back-port rbac-backend bug fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
  • Loading branch information
AndrienkoAleksandr committed Oct 23, 2024
1 parent a47a45e commit 1b5bd9d
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 10 deletions.
8 changes: 8 additions & 0 deletions .changeset/fuzzy-dryers-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@janus-idp/backstage-plugin-rbac-backend": patch
---

Update RBAC dependencies to address the following issues:

- fix broken conditional alias $ownerRefs
- add the ability to disable RBAC group inheritance
4 changes: 3 additions & 1 deletion plugins/rbac-backend/docs/group-hierarchy.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,11 @@ permission:
maxDepth: 1
```
The `maxDepth` must be greater than 0 to ensure that the graphs are built correctly. Also the graph
The `maxDepth` must be greater than or equal to 0 to ensure that the graphs are built correctly. Also the graph
will be built with a hierarchy of 1 + maxDepth.

A value of 0 for maxDepth disables the group inheritance feature.

## Non-Existent Groups in the Hierarchy

For group hierarchy to function, groups don't need to be present in the catalog as long as the group
Expand Down
12 changes: 6 additions & 6 deletions plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,18 @@ export class AncestorSearchMemo {
allGroups: Entity[],
current_depth: number,
) {
const depth = current_depth + 1;
if (this.maxDepth && current_depth >= this.maxDepth) {
return;
}

const groupName = `group:${group.metadata.namespace?.toLocaleLowerCase(
'en-US',
)}/${group.metadata.name.toLocaleLowerCase('en-US')}`;
if (!memo.hasEntityRef(groupName)) {
memo.setNode(groupName);
}

if (this.maxDepth !== undefined && current_depth >= this.maxDepth) {
return;
}
const depth = current_depth + 1;

const parent = group.spec?.parent as string;
const parentGroup = allGroups.find(g => g.metadata.name === parent);

Expand All @@ -175,7 +175,7 @@ export class AncestorSearchMemo {
current_depth: number,
) {
// We add one to the maxDepth here because the user is considered the starting node
if (this.maxDepth && current_depth >= this.maxDepth + 1) {
if (this.maxDepth !== undefined && current_depth >= this.maxDepth + 1) {
return;
}
const depth = current_depth + 1;
Expand Down
47 changes: 46 additions & 1 deletion plugins/rbac-backend/src/role-manager/role-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ describe('BackstageRoleManager', () => {

expect(errorRoleManager).toBeUndefined();
expect(expectedError).toMatchObject({
message: 'Max Depth for RBAC group hierarchy must be greater than zero',
message:
'Max Depth for RBAC group hierarchy must be greater than or equal to zero',
});
});
});
Expand Down Expand Up @@ -328,6 +329,50 @@ describe('BackstageRoleManager', () => {
expect(result).toBeTruthy();
});

// user:default/mike should inherits from group:default/team-a
//
// Hierarchy:
//
// group:default/team-a
// |
// group:default/team-b
// |
// user:default/mike
//
it('should disable group inheritance when max-depth=0', async () => {
// max-depth=0
const config = newConfigReader(0);
const rm = new BackstageRoleManager(
catalogApiMock as CatalogApi,
loggerMock as LoggerService,
catalogDBClient,
rbacDBClient,
config,
mockAuthService,
);
const groupMock = createGroupEntity('team-b', 'team-a', [], ['mike']);
const groupParentMock = createGroupEntity('team-a', undefined, [
'team-b',
]);

catalogApiMock.getEntities.mockImplementation((arg: any) => {
const hasMember = arg.filter['relations.hasMember'];
if (hasMember && hasMember === 'user:default/mike') {
return { items: [groupMock] };
}
return { items: [groupMock, groupParentMock] };
});

let result = await rm.hasLink(
'user:default/mike',
'group:default/team-b',
);
expect(result).toBeTruthy();

result = await rm.hasLink('user:default/mike', 'group:default/team-a');
expect(result).toBeFalsy();
});

// user:default/mike should inherits role:default/team-a from group:default/team-a
//
// Hierarchy:
Expand Down
4 changes: 2 additions & 2 deletions plugins/rbac-backend/src/role-manager/role-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ export class BackstageRoleManager implements RoleManager {
this.allRoles = new Map<string, RoleMemberList>();
const rbacConfig = this.config.getOptionalConfig('permission.rbac');
this.maxDepth = rbacConfig?.getOptionalNumber('maxDepth');
if (this.maxDepth !== undefined && this.maxDepth! <= 0) {
if (this.maxDepth !== undefined && this.maxDepth! < 0) {
throw new Error(
'Max Depth for RBAC group hierarchy must be greater than zero',
'Max Depth for RBAC group hierarchy must be greater than or equal to zero',
);
}
}
Expand Down
1 change: 1 addition & 0 deletions plugins/rbac-backend/src/service/policy-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export class PolicyBuilder {
),
auth: auth,
httpAuth: httpAuth,
userInfo: env.userInfo,
};

const server = new PoliciesServer(
Expand Down

0 comments on commit 1b5bd9d

Please sign in to comment.