fix(rbac): reduce the number of permissions returned, add isResourced…

… flag (janus-idp#1474) * fix(rbac): reduce the number of permissions returned, add isResourced flag Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): update doc Signed-off-by: Oleksandr Andriienko <[email protected]> --------- Signed-off-by: Oleksandr Andriienko <[email protected]>
ciiay · Apr 17, 2024 · e5dda95 · e5dda95
1 parent 6f9fe17
commit e5dda95
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 51 deletions.
diff --git a/plugins/rbac-backend/docs/apis.md b/plugins/rbac-backend/docs/apis.md
@@ -415,37 +415,44 @@ Returns:
 [
   {
     "pluginId": "catalog",
-      "policies": [
-        {
-          "permission": "catalog-entity",
-          "policy": "read"
-        },
-        {
-          "permission": "catalog.entity.create",
-          "policy": "create"
-        },
-        {
-          "permission": "catalog-entity",
-          "policy": "delete"
-        },
-        {
-          "permission": "catalog-entity",
-          "policy": "update"
-        },
-        {
-          "permission": "catalog.location.read",
-          "policy": "read"
-        },
-        {
-          "permission": "catalog.location.create",
-          "policy": "create"
-        },
-        {
-          "permission": "catalog.location.delete",
-          "policy": "delete"
-        }
-      ]
-    },
+    "policies": [
+      {
+        "isResourced": true,
+        "permission": "catalog-entity",
+        "policy": "read"
+      },
+      {
+        "isResourced": false,
+        "permission": "catalog.entity.create",
+        "policy": "create"
+      },
+      {
+        "isResourced": true,
+        "permission": "catalog-entity",
+        "policy": "delete"
+      },
+      {
+        "isResourced": true,
+        "permission": "catalog-entity",
+        "policy": "update"
+      },
+      {
+        "isResourced": false,
+        "permission": "catalog.location.read",
+        "policy": "read"
+      },
+      {
+        "isResourced": false,
+        "permission": "catalog.location.create",
+        "policy": "create"
+      },
+      {
+        "isResourced": false,
+        "permission": "catalog.location.delete",
+        "policy": "delete"
+      }
+    ]
+  },
   ...
 ]
 ```

diff --git a/plugins/rbac-backend/src/service/plugin-endpoint.test.ts b/plugins/rbac-backend/src/service/plugin-endpoint.test.ts
@@ -69,7 +69,7 @@ describe('plugin-endpoint', () => {
       expect(policiesMetadata.length).toEqual(0);
     });
 
-    it('should return non empty plugin policies list', async () => {
+    it('should return non empty plugin policies list with resourced permission', async () => {
       backendPluginIDsProviderMock.getPluginIds.mockReturnValue(['permission']);
 
       mockUrlReaderService.readUrl.mockReturnValue(mockReadUrlResponse);
@@ -89,22 +89,19 @@ describe('plugin-endpoint', () => {
       expect(policiesMetadata[0].pluginId).toEqual('permission');
       expect(policiesMetadata[0].policies).toEqual([
         {
+          isResourced: true,
           permission: 'policy-entity',
           policy: 'read',
         },
-        {
-          permission: 'policy.entity.read',
-          policy: 'read',
-        },
       ]);
     });
 
-    it('should return plugin policies list without resource type permissions', async () => {
+    it('should return non empty plugin policies list with non resourced permission', async () => {
       backendPluginIDsProviderMock.getPluginIds.mockReturnValue(['permission']);
 
       mockUrlReaderService.readUrl.mockReturnValue(mockReadUrlResponse);
       bufferMock.toString.mockReturnValueOnce(
-        '{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
+        '{"permissions":[{"type":"basic","name":"catalog.entity.create","attributes":{"action":"create"}}]}',
       );
 
       const collector = new PluginPermissionMetadataCollector(
@@ -119,8 +116,9 @@ describe('plugin-endpoint', () => {
       expect(policiesMetadata[0].pluginId).toEqual('permission');
       expect(policiesMetadata[0].policies).toEqual([
         {
-          permission: 'policy.entity.read',
-          policy: 'read',
+          isResourced: false,
+          permission: 'catalog.entity.create',
+          policy: 'create',
         },
       ]);
     });
@@ -143,7 +141,7 @@ describe('plugin-endpoint', () => {
           throw new NotFoundError();
         });
       bufferMock.toString.mockReturnValueOnce(
-        '{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
+        '{"permissions":[{"type":"resource","resourceType":"policy-entity","name":"policy.entity.read","attributes":{"action":"read"}}]}',
       );
 
       const collector = new PluginPermissionMetadataCollector(
@@ -158,7 +156,8 @@ describe('plugin-endpoint', () => {
       expect(policiesMetadata[0].pluginId).toEqual('permission');
       expect(policiesMetadata[0].policies).toEqual([
         {
-          permission: 'policy.entity.read',
+          isResourced: true,
+          permission: 'policy-entity',
           policy: 'read',
         },
       ]);
@@ -182,7 +181,7 @@ describe('plugin-endpoint', () => {
           throw new Error('Unexpected error');
         });
       bufferMock.toString.mockReturnValueOnce(
-        '{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
+        '{"permissions":[{"type":"resource","resourceType":"policy-entity","name":"policy.entity.read","attributes":{"action":"read"}}]}',
       );
 
       const errorSpy = jest.spyOn(logger, 'error').mockClear();
@@ -199,7 +198,8 @@ describe('plugin-endpoint', () => {
       expect(policiesMetadata[0].pluginId).toEqual('permission');
       expect(policiesMetadata[0].policies).toEqual([
         {
-          permission: 'policy.entity.read',
+          isResourced: true,
+          permission: 'policy-entity',
           policy: 'read',
         },
       ]);
@@ -222,7 +222,7 @@ describe('plugin-endpoint', () => {
         });
       bufferMock.toString
         .mockReturnValueOnce(
-          '{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
+          '{"permissions":[{"type":"resource","resourceType":"policy-entity","name":"policy.entity.read","attributes":{"action":"read"}}]}',
         )
         .mockReturnValueOnce('non json data');
 
@@ -240,7 +240,8 @@ describe('plugin-endpoint', () => {
       expect(policiesMetadata[0].pluginId).toEqual('permission');
       expect(policiesMetadata[0].policies).toEqual([
         {
-          permission: 'policy.entity.read',
+          isResourced: true,
+          permission: 'policy-entity',
           policy: 'read',
         },
       ]);

diff --git a/plugins/rbac-backend/src/service/plugin-endpoints.ts b/plugins/rbac-backend/src/service/plugin-endpoints.ts
@@ -142,18 +142,22 @@ export class PluginPermissionMetadataCollector {
 }
 
 function permissionsToCasbinPolicies(permissions: Permission[]): Policy[] {
-  const policies = [];
+  const policies: Policy[] = [];
   for (const permission of permissions) {
     if (isResourcePermission(permission)) {
       policies.push({
         permission: permission.resourceType,
         policy: permission.attributes.action || 'use',
+        isResourced: true,
+      });
+    } else {
+      policies.push({
+        permission: permission.name,
+        policy: permission.attributes.action || 'use',
+        isResourced: false,
       });
     }
-    policies.push({
-      permission: permission.name,
-      policy: permission.attributes.action || 'use',
-    });
   }
+
   return policies;
 }
diff --git a/plugins/rbac-common/src/types.ts b/plugins/rbac-common/src/types.ts
@@ -21,6 +21,7 @@ export type RoleMetadata = {
 
 export type Policy = {
   permission?: string;
+  isResourced?: boolean;
   policy?: string;
   effect?: string;
   metadata?: PermissionPolicyMetadata;