feat(rbac): allow to disable group inheritance (#2143)

* feat(rbac): allow to disable group inheritance Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com> * feat(rbac): clean up Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com> * feat(rbac): fix unit tests Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com> * feat(rbac): fix eslint Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com> --------- Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
janus-idp · Sep 25, 2024 · c6c4ff5 · c6c4ff5
1 parent b750bbf
commit c6c4ff5
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 9 deletions.
diff --git a/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts b/plugins/rbac-backend/src/role-manager/ancestor-search-memo.ts
@@ -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);
 
@@ -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
@@ -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
@@ -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',
       );
     }
   }