From 574261138dd9bd32dae55a8dc2615d4d4eebfb54 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Mon, 8 Apr 2024 16:21:13 +0300 Subject: [PATCH 1/2] fix(rbac): reduce the number of permissions returned, add isResourced flag Signed-off-by: Oleksandr Andriienko --- .../src/service/plugin-endpoint.test.ts | 31 ++++++++++--------- .../src/service/plugin-endpoints.ts | 14 ++++++--- plugins/rbac-common/src/types.ts | 1 + 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/plugins/rbac-backend/src/service/plugin-endpoint.test.ts b/plugins/rbac-backend/src/service/plugin-endpoint.test.ts index 815b05272b..8e1a00396d 100644 --- 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 index 8cedad91d5..ac21e5272b 100644 --- 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 index b3ea85686b..6cf9670102 100644 --- 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; From 7b85cf7aaf6c83bf0a46cac8e5f93f211b632626 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Tue, 9 Apr 2024 11:58:40 +0300 Subject: [PATCH 2/2] fix(rbac): update doc Signed-off-by: Oleksandr Andriienko --- plugins/rbac-backend/docs/apis.md | 69 +++++++++++++++++-------------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/plugins/rbac-backend/docs/apis.md b/plugins/rbac-backend/docs/apis.md index a249130892..03ae51a33b 100644 --- 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" + } + ] + }, ... ] ```