fix(rbac): review suggestions

janus-idp · Feb 19, 2024 · 70b898a · 70b898a
1 parent 440b992
commit 70b898a
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 28 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
@@ -15,7 +15,7 @@ export class AncestorSearchMemo {
   private tokenManager: TokenManager;
   private catalogApi: CatalogApi;
 
-  private userName: string;
+  private userEntityRef: string;
 
   private allGroups: Entity[];
 
@@ -25,7 +25,7 @@ export class AncestorSearchMemo {
     catalogApi: CatalogApi,
   ) {
     this.graph = new Graph({ directed: true });
-    this.userName = userEntityRef.split('/')[1];
+    this.userEntityRef = userEntityRef;
     this.tokenManager = tokenManager;
     this.catalogApi = catalogApi;
     this.allGroups = [];
@@ -69,18 +69,25 @@ export class AncestorSearchMemo {
     const { items } = await this.catalogApi.getEntities(
       {
         filter: { kind: 'Group' },
-        fields: [
-          'metadata.name',
-          'metadata.namespace',
-          'spec.parent',
-          'spec.members',
-        ],
+        fields: ['metadata.name', 'metadata.namespace', 'spec.parent'],
       },
       { token },
     );
     this.allGroups = items;
   }
 
+  async getUserGroups(): Promise<Entity[]> {
+    const { token } = await this.tokenManager.getToken();
+    const { items } = await this.catalogApi.getEntities(
+      {
+        filter: { kind: 'Group', 'relations.hasMember': this.userEntityRef },
+        fields: ['metadata.name', 'metadata.namespace', 'spec.parent'],
+      },
+      { token },
+    );
+    return items;
+  }
+
   traverseGroups(memo: AncestorSearchMemo, group: Entity) {
     const groupsRefs = new Set<string>();
     const groupName = `group:${group.metadata.namespace?.toLocaleLowerCase(
@@ -107,10 +114,7 @@ export class AncestorSearchMemo {
   }
 
   async buildUserGraph(memo: AncestorSearchMemo) {
-    const userGroups = this.allGroups.filter(group => {
-      const members = group.spec?.members as string[];
-      return members?.includes(this.userName);
-    });
+    const userGroups = await this.getUserGroups();
     userGroups.forEach(group => this.traverseGroups(memo, group));
   }
 }
diff --git a/plugins/rbac-backend/src/role-manager/role-manager.test.ts b/plugins/rbac-backend/src/role-manager/role-manager.test.ts
@@ -113,12 +113,7 @@ describe('BackstageRoleManager', () => {
           filter: {
             kind: 'Group',
           },
-          fields: [
-            'metadata.name',
-            'metadata.namespace',
-            'spec.parent',
-            'spec.members',
-          ],
+          fields: ['metadata.name', 'metadata.namespace', 'spec.parent'],
         },
         { token: 'some-token' },
       );
@@ -141,12 +136,7 @@ describe('BackstageRoleManager', () => {
           filter: {
             kind: 'Group',
           },
-          fields: [
-            'metadata.name',
-            'metadata.namespace',
-            'spec.parent',
-            'spec.members',
-          ],
+          fields: ['metadata.name', 'metadata.namespace', 'spec.parent'],
         },
         { token: 'some-token' },
       );
@@ -642,7 +632,11 @@ describe('BackstageRoleManager', () => {
       const groupBMock = createGroupEntity('team-b', 'team-a', ['team-c']);
       const groupCMock = createGroupEntity('team-c', 'team-b', [], ['mike']);
 
-      catalogApiMock.getEntities.mockImplementation((_arg: any) => {
+      catalogApiMock.getEntities.mockImplementation((arg: any) => {
+        const hasMember = arg.filter['relations.hasMember'];
+        if (hasMember && hasMember === 'user:default/mike') {
+          return { items: [groupCMock] };
+        }
         return { items: [groupAMock, groupBMock, groupCMock] };
       });
 
@@ -692,7 +686,11 @@ describe('BackstageRoleManager', () => {
       const groupBMock = createGroupEntity('team-b', 'team-a', ['team-c']);
       const groupCMock = createGroupEntity('team-c', 'team-b', ['mike']);
 
-      catalogApiMock.getEntities.mockImplementation((_arg: any) => {
+      catalogApiMock.getEntities.mockImplementation((arg: any) => {
+        const hasMember = arg.filter['relations.hasMember'];
+        if (hasMember && hasMember === 'user:default/mike') {
+          return { items: [groupCMock] };
+        }
         return { items: [groupAMock, groupBMock, groupCMock] };
       });
 
@@ -825,7 +823,13 @@ describe('BackstageRoleManager', () => {
       const groupAMock = createGroupEntity('team-a', 'team-c', ['team-c']);
       const groupBMock = createGroupEntity('team-b', 'root', ['team-d']);
 
-      catalogApiMock.getEntities.mockImplementation((_arg: any) => {
+      catalogApiMock.getEntities.mockImplementation((arg: any) => {
+        const hasMember = arg.filter['relations.hasMember'];
+        if (hasMember && hasMember === 'user:default/mike') {
+          return { items: [groupCMock] };
+        } else if (hasMember && hasMember === 'user:default/tom') {
+          return { items: [groupDMock] };
+        }
         return {
           items: [
             groupRootMock,
@@ -884,7 +888,13 @@ describe('BackstageRoleManager', () => {
       const groupAMock = createGroupEntity('team-a', 'team-c', ['team-c']);
       const groupBMock = createGroupEntity('team-b', 'root', ['team-d']);
 
-      catalogApiMock.getEntities.mockImplementation((_arg: any) => {
+      catalogApiMock.getEntities.mockImplementation((arg: any) => {
+        const hasMember = arg.filter['relations.hasMember'];
+        if (hasMember && hasMember === 'user:default/mike') {
+          return { items: [groupCMock] };
+        } else if (hasMember && hasMember === 'user:default/tom') {
+          return { items: [groupDMock] };
+        }
         return {
           items: [
             groupRootMock,