Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(release): update RBAC to delivery bug fixes for RHDH 1.3.1 #2375

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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