From a67e4205b64d2ad9d6570b0b1523c82712ea3738 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Wed, 23 Oct 2024 13:55:35 +0300 Subject: [PATCH] chore(release): back-port rbac-backend bug fixes Signed-off-by: Oleksandr Andriienko --- .changeset/fuzzy-dryers-lie.md | 8 ++++ plugins/rbac-backend/docs/group-hierarchy.md | 4 +- .../src/role-manager/ancestor-search-memo.ts | 12 ++--- .../src/role-manager/role-manager.test.ts | 47 ++++++++++++++++++- .../src/role-manager/role-manager.ts | 4 +- .../src/service/policy-builder.ts | 1 + 6 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 .changeset/fuzzy-dryers-lie.md diff --git a/.changeset/fuzzy-dryers-lie.md b/.changeset/fuzzy-dryers-lie.md new file mode 100644 index 0000000000..2c6eba864b --- /dev/null +++ b/.changeset/fuzzy-dryers-lie.md @@ -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 diff --git a/plugins/rbac-backend/docs/group-hierarchy.md b/plugins/rbac-backend/docs/group-hierarchy.md index e2bab1998a..13f4a3bb28 100644 --- a/plugins/rbac-backend/docs/group-hierarchy.md +++ b/plugins/rbac-backend/docs/group-hierarchy.md @@ -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 diff --git a/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts b/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts index 71c8aa2697..990ecde44d 100644 --- a/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts +++ b/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts @@ -141,11 +141,6 @@ 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')}`; @@ -153,6 +148,11 @@ export class AncestorSearchMemo { 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); @@ -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; diff --git a/plugins/rbac-backend/src/role-manager/role-manager.test.ts b/plugins/rbac-backend/src/role-manager/role-manager.test.ts index dd4b2840f8..05fba0c48c 100644 --- a/plugins/rbac-backend/src/role-manager/role-manager.test.ts +++ b/plugins/rbac-backend/src/role-manager/role-manager.test.ts @@ -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', }); }); }); @@ -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: diff --git a/plugins/rbac-backend/src/role-manager/role-manager.ts b/plugins/rbac-backend/src/role-manager/role-manager.ts index 1f7a15ca15..a5bb239a65 100644 --- a/plugins/rbac-backend/src/role-manager/role-manager.ts +++ b/plugins/rbac-backend/src/role-manager/role-manager.ts @@ -23,9 +23,9 @@ export class BackstageRoleManager implements RoleManager { this.allRoles = new Map(); 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', ); } } diff --git a/plugins/rbac-backend/src/service/policy-builder.ts b/plugins/rbac-backend/src/service/policy-builder.ts index 0ddfe9efb7..7acf5806af 100644 --- a/plugins/rbac-backend/src/service/policy-builder.ts +++ b/plugins/rbac-backend/src/service/policy-builder.ts @@ -159,6 +159,7 @@ export class PolicyBuilder { ), auth: auth, httpAuth: httpAuth, + userInfo: env.userInfo, }; const server = new PoliciesServer(