From 4d878a29babd86bd7896d69e6b2b63392b6e6cc8 Mon Sep 17 00:00:00 2001 From: Patrick Knight Date: Fri, 27 Oct 2023 13:23:16 -0400 Subject: [PATCH] feat(rbac): implement the concept of roles in rbac (#867) * feat(rbac): implement the concept of roles in rbac * feat(rbac): review suggestions * feat(rbac): more code review feedback * fix(rbac) ability to update role name --- plugins/rbac-backend/README.md | 10 +- plugins/rbac-backend/model/rbac-policy.csv | 7 +- .../src/service/permission-policy.ts | 54 +- .../src/service/policies-rest-api.test.ts | 723 ++++++++++++++++++ .../src/service/policies-rest-api.ts | 391 ++++++++-- .../src/service/policies-validation.test.ts | 52 +- .../src/service/policies-validation.ts | 34 +- .../src/service/role-manager.test.ts | 563 +++++++++++++- .../rbac-backend/src/service/role-manager.ts | 99 ++- plugins/rbac-common/src/types.ts | 7 +- 10 files changed, 1805 insertions(+), 135 deletions(-) diff --git a/plugins/rbac-backend/README.md b/plugins/rbac-backend/README.md index 6bad570573..97e317a529 100644 --- a/plugins/rbac-backend/README.md +++ b/plugins/rbac-backend/README.md @@ -1,6 +1,6 @@ # RBAC backend plugin for Backstage -This plugin seamlessly integrates with the Backstage permission framework](https://backstage.io/docs/permissions/overview/) to empower you with robust role-based access control capabilities within your Backstage environment. +This plugin seamlessly integrates with the [Backstage permission framework](https://backstage.io/docs/permissions/overview/) to empower you with robust role-based access control capabilities within your Backstage environment. The Backstage permission framework is a core component of the Backstage project, designed to provide meticulous control over resource and action access. Our RBAC plugin harnesses the power of this framework, allowing you to tailor access permissions without the need for coding. Instead, you can effortlessly manage your access policies through User interface embedded within Backstage or via the configuration files. @@ -59,8 +59,12 @@ The RBAC plugin also allows you to import policies from an external file. These Here's an example of an external permission policies configuration file named `rbac-policy.csv`: ```CSV -p, user:default/bob, catalog-entity, read, deny -p, user:default/alice, catalog.entity.create, use, deny +p, role:default/team_a, catalog-entity, read, deny +p, role:default/team_b, catalog.entity.create, use, deny + +g, user:default/bob, role:default/team_a + +g, group:default/team_b, role:default/team_b ``` You can specify the path to this configuration file in your application configuration: diff --git a/plugins/rbac-backend/model/rbac-policy.csv b/plugins/rbac-backend/model/rbac-policy.csv index 45bb354e64..9e810c3aad 100644 --- a/plugins/rbac-backend/model/rbac-policy.csv +++ b/plugins/rbac-backend/model/rbac-policy.csv @@ -1,4 +1,7 @@ -p, user:default/guest, catalog-entity, read, deny -p, user:default/guest, catalog.entity.create, use, deny +p, role:default/guests, catalog-entity, read, deny +p, role:default/guests, catalog.entity.create, use, deny + +g, user:default/guest, role:default/guests + diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index 5e7388c616..261f8ead52 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -19,28 +19,48 @@ import { Logger } from 'winston'; import { MODEL } from './permission-model'; -const useAdmins = (admins: Config[], enf: Enforcer) => { +const useAdmins = async (admins: Config[], enf: Enforcer) => { + const adminRoleName = 'role:default/rbac_admin'; admins.flatMap(async localConfig => { const name = localConfig.getString('name'); - const adminReadPermission = [name, 'policy-entity', 'read', 'allow']; - if (!(await enf.hasPolicy(...adminReadPermission))) { - await enf.addPolicy(...adminReadPermission); - } - const adminCreatePermission = [name, 'policy-entity', 'create', 'allow']; - if (!(await enf.hasPolicy(...adminCreatePermission))) { - await enf.addPolicy(...adminCreatePermission); + const adminRole = [name, adminRoleName]; + if (!(await enf.hasGroupingPolicy(...adminRole))) { + await enf.addGroupingPolicy(...adminRole); } + }); + const adminReadPermission = [adminRoleName, 'policy-entity', 'read', 'allow']; + if (!(await enf.hasPolicy(...adminReadPermission))) { + await enf.addPolicy(...adminReadPermission); + } + const adminCreatePermission = [ + adminRoleName, + 'policy-entity', + 'create', + 'allow', + ]; + if (!(await enf.hasPolicy(...adminCreatePermission))) { + await enf.addPolicy(...adminCreatePermission); + } - const adminDeletePermission = [name, 'policy-entity', 'delete', 'allow']; - if (!(await enf.hasPolicy(...adminDeletePermission))) { - await enf.addPolicy(...adminDeletePermission); - } + const adminDeletePermission = [ + adminRoleName, + 'policy-entity', + 'delete', + 'allow', + ]; + if (!(await enf.hasPolicy(...adminDeletePermission))) { + await enf.addPolicy(...adminDeletePermission); + } - const adminUpdatePermission = [name, 'policy-entity', 'update', 'allow']; - if (!(await enf.hasPolicy(...adminUpdatePermission))) { - await enf.addPolicy(...adminUpdatePermission); - } - }); + const adminUpdatePermission = [ + adminRoleName, + 'policy-entity', + 'update', + 'allow', + ]; + if (!(await enf.hasPolicy(...adminUpdatePermission))) { + await enf.addPolicy(...adminUpdatePermission); + } }; const addPredefinedPolicies = async ( diff --git a/plugins/rbac-backend/src/service/policies-rest-api.test.ts b/plugins/rbac-backend/src/service/policies-rest-api.test.ts index f46ee83e01..bf32d3de81 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.test.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.test.ts @@ -28,11 +28,21 @@ const mockEnforcer: Partial = { .mockImplementation(async (..._param: string[]): Promise => { return true; }), + addGroupingPolicies: jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }), hasPolicy: jest .fn() .mockImplementation(async (..._param: string[]): Promise => { return false; }), + hasGroupingPolicy: jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return false; + }), loadPolicy: jest.fn().mockImplementation(async () => {}), enableAutoSave: jest.fn().mockImplementation((_enable: boolean) => {}), getFilteredPolicy: jest @@ -44,6 +54,13 @@ const mockEnforcer: Partial = { ]; }, ), + getFilteredGroupingPolicy: jest + .fn() + .mockImplementation( + async (_fieldIndex: number, ..._fieldValues: string[]) => { + return [['user:default/permission_admin', 'role:default/rbac_admin']]; + }, + ), }; jest.mock('casbin', () => { @@ -96,6 +113,11 @@ describe('REST policies api', () => { .mockImplementation(async (..._param: string[]): Promise => { return false; }); + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return false; + }); const config = new ConfigReader({ backend: { @@ -892,6 +914,707 @@ describe('REST policies api', () => { }); }); + describe('GET /roles', () => { + it('should return a status of Unauthorized', async () => { + mockedAuthorizeConditional.mockImplementationOnce(async () => [ + { result: AuthorizeResult.DENY }, + ]); + const result = await request(app).get('/roles').send(); + + expect(mockedAuthorizeConditional).toHaveBeenCalledWith( + [{ permission: policyEntityReadPermission }], + { token: 'token' }, + ); + expect(result.statusCode).toBe(403); + expect(result.body.error).toEqual({ + name: 'NotAllowedError', + message: '', + }); + }); + + it('should be returned list all roles', async () => { + mockEnforcer.getGroupingPolicy = jest + .fn() + .mockImplementation(async () => { + return [ + ['group:default/test', 'role:default/test'], + ['group:default/team_a', 'role:default/team_a'], + ]; + }); + const result = await request(app).get('/roles').send(); + expect(result.statusCode).toBe(200); + expect(result.body).toEqual([ + { + memberReferences: ['group:default/test'], + name: 'role:default/test', + }, + { + memberReferences: ['group:default/team_a'], + name: 'role:default/team_a', + }, + ]); + }); + }); + + describe('GET /roles/:kind/:namespace/:name', () => { + it('should return a status of Unauthorized', async () => { + mockedAuthorizeConditional.mockImplementationOnce(async () => [ + { result: AuthorizeResult.DENY }, + ]); + const result = await request(app) + .get('/roles/role/default/rbac_admin') + .send(); + + expect(mockedAuthorizeConditional).toHaveBeenCalledWith( + [{ permission: policyEntityReadPermission }], + { token: 'token' }, + ); + expect(result.statusCode).toBe(403); + expect(result.body.error).toEqual({ + name: 'NotAllowedError', + message: '', + }); + }); + + it('should return an input error when kind is wrong', async () => { + const result = await request(app) + .get('/roles/test/default/rbac_admin') + .send(); + expect(result.statusCode).toBe(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Unsupported kind test. List supported values ["user", "group", "role"]`, + }); + }); + + it('should be returned roles by role reference', async () => { + const result = await request(app) + .get('/roles/role/default/rbac_admin') + .send(); + expect(result.statusCode).toBe(200); + expect(result.body).toEqual([ + { + memberReferences: ['user:default/permission_admin'], + name: 'role:default/rbac_admin', + }, + ]); + }); + + it('should be returned roles by role reference not found', async () => { + mockEnforcer.getFilteredGroupingPolicy = jest + .fn() + .mockImplementation( + async (_fieldIndex: number, ..._fieldValues: string[]) => { + return []; + }, + ); + + const result = await request(app) + .get('/roles/role/default/rbac_admin') + .send(); + expect(result.statusCode).toBe(404); + expect(result.body).toEqual({ + error: { message: '', name: 'NotFoundError' }, + request: { + method: 'GET', + url: '/roles/role/default/rbac_admin', + }, + response: { statusCode: 404 }, + }); + }); + }); + + describe('POST /roles', () => { + it('should return a status of Unauthorized', async () => { + mockedAuthorizeConditional.mockImplementationOnce(async () => [ + { result: AuthorizeResult.DENY }, + ]); + const result = await request(app).post('/roles').send(); + + expect(mockedAuthorizeConditional).toHaveBeenCalledWith( + [{ permission: policyEntityCreatePermission }], + { token: 'token' }, + ); + expect(result.statusCode).toBe(403); + expect(result.body.error).toEqual({ + name: 'NotAllowedError', + message: '', + }); + }); + + it('should not be created role - req body is an empty', async () => { + const result = await request(app).post('/roles').send(); + + expect(result.statusCode).toBe(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Invalid role definition. Cause: 'name' field must not be empty`, + }); + }); + + it('should not be created role - memberReferences is missing', async () => { + const result = await request(app).post('/roles').send({ + name: 'role:default/test', + }); + + expect(result.statusCode).toBe(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Invalid role definition. Cause: 'memberReferences' field must not be empty`, + }); + }); + + it('should not be created role - memberReferences is empty', async () => { + const result = await request(app).post('/roles').send({ + memberReferences: [], + name: 'role:default/test', + }); + + expect(result.statusCode).toBe(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Invalid role definition. Cause: 'memberReferences' field must not be empty`, + }); + }); + + it('should not be created role - memberReferences is invalid', async () => { + const result = await request(app) + .post('/roles') + .send({ + memberReferences: ['user'], + name: 'role:default/test', + }); + + expect(result.statusCode).toBe(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Invalid role definition. Cause: Entity reference "user" had missing or empty kind (e.g. did not start with "component:" or similar)`, + }); + }); + + it('should not be created role - name is empty', async () => { + const result = await request(app) + .post('/roles') + .send({ + memberReferences: ['user:default/permission_admin'], + }); + + expect(result.statusCode).toBe(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Invalid role definition. Cause: 'name' field must not be empty`, + }); + }); + + it('should be created role', async () => { + const result = await request(app) + .post('/roles') + .send({ + memberReferences: ['user:default/permission_admin'], + name: 'role:default/rbac_admin', + }); + + expect(result.statusCode).toBe(201); + }); + + it('should not be created role, because it is has been already present', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + + const result = await request(app) + .post('/roles') + .send({ + memberReferences: ['user:default/permission_admin'], + name: 'role:default/rbac_admin', + }); + + expect(result.statusCode).toBe(409); + }); + + it('should not be created role caused some unexpected error', async () => { + mockEnforcer.addGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return false; + }); + + const result = await request(app) + .post('/roles') + .send({ + memberReferences: ['user:default/permission_admin'], + name: 'role:default/rbac_admin', + }); + + expect(result.statusCode).toBe(500); + }); + }); + + describe('PUT /roles/:kind/:namespace/:name', () => { + it('should return a status of Unauthorized', async () => { + mockedAuthorizeConditional.mockImplementationOnce(async () => [ + { result: AuthorizeResult.DENY }, + ]); + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send(); + + expect(mockedAuthorizeConditional).toHaveBeenCalledWith( + [{ permission: policyEntityUpdatePermission }], + { token: 'token' }, + ); + expect(result.statusCode).toBe(403); + expect(result.body.error).toEqual({ + name: 'NotAllowedError', + message: '', + }); + }); + + it('should fail to update role - old role is absent', async () => { + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send(); + + expect(result.statusCode).toEqual(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `'oldRole' object must be present`, + }); + }); + + it('should fail to update role - new role is absent', async () => { + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ oldRole: {} }); + + expect(result.statusCode).toEqual(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `'newRole' object must be present`, + }); + }); + + it('should fail to update role - oldRole entity is absent', async () => { + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ oldRole: {}, newRole: {} }); + + expect(result.statusCode).toEqual(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Invalid old role object. Cause: 'memberReferences' field must not be empty`, + }); + }); + + it('should fail to update role - newRole entity is absent', async () => { + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { memberReferences: ['user:default/permission_admin'] }, + newRole: {}, + }); + + expect(result.statusCode).toEqual(400); + expect(result.body.error).toEqual({ + name: 'InputError', + message: `Invalid new role object. Cause: 'name' field must not be empty`, + }); + }); + + it('should fail to update role - old role not found', async () => { + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/test'], + name: 'role/default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(404); + expect(result.body.error).toEqual({ + name: 'NotFoundError', + message: '', + }); + }); + + it('should fail to update role - newRole is already present', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/test'], + name: 'role:default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(409); + expect(result.body.error).toEqual({ + name: 'ConflictError', + message: '', + }); + }); + + it('should nothing to update', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/permission_admin'], + name: 'role:default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(204); + }); + + it('should fail to update role - unable to remove oldRole', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (...param: string[]): Promise => { + if (param[0] === 'user:default/test') { + return false; + } + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return false; + }); + + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/test'], + name: 'role:default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(500); + expect(result.body.error).toEqual({ + name: 'Error', + message: 'Unexpected error', + }); + }); + + it('should fail to update role - unable to add newRole', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (...param: string[]): Promise => { + if (param[0] === 'user:default/test') { + return false; + } + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.addGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return false; + }); + + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/test'], + name: 'role:default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(500); + expect(result.body.error).toEqual({ + name: 'Error', + message: 'Unexpected error', + }); + }); + + it('should update role', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (...param: string[]): Promise => { + if (param[0] === 'user:default/test') { + return false; + } + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.addGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/test'], + name: 'role:default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(200); + }); + + it('should update role where newRole has multiple roles', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (...param: string[]): Promise => { + if ( + param[0] === 'user:default/test' || + param[0] === 'user:default/test2' + ) { + return false; + } + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.addGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/test', 'user:default/test2'], + name: 'role:default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(200); + }); + + it('should update role where newRole has multiple roles with one being from oldRole', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (...param: string[]): Promise => { + if (param[0] === 'user:default/test') { + return false; + } + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.addGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: [ + 'user:default/permission_admin', + 'user:default/test', + ], + name: 'role:default/rbac_admin', + }, + }); + + expect(result.statusCode).toEqual(200); + }); + + it('should update role name', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (...param: string[]): Promise => { + if (param[0] === 'user:default/test') { + return false; + } + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.addGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + + const result = await request(app) + .put('/roles/role/default/rbac_admin') + .send({ + oldRole: { + memberReferences: ['user:default/permission_admin'], + }, + newRole: { + memberReferences: ['user:default/test'], + name: 'role:default/test', + }, + }); + + expect(result.statusCode).toEqual(200); + }); + }); + + describe('DELETE /roles/:kind/:namespace/:name', () => { + it('should return a status of Unauthorized', async () => { + mockedAuthorizeConditional.mockImplementationOnce(async () => [ + { result: AuthorizeResult.DENY }, + ]); + const result = await request(app) + .delete('/roles/role/default/rbac_admin') + .send(); + + expect(mockedAuthorizeConditional).toHaveBeenCalledWith( + [{ permission: policyEntityDeletePermission }], + { token: 'token' }, + ); + expect(result.statusCode).toBe(403); + expect(result.body.error).toEqual({ + name: 'NotAllowedError', + message: '', + }); + }); + + it('should fail to delete, because unexpected error', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return false; + }); + + const result = await request(app) + .delete( + '/roles/role/default/rbac_admin?memberReferences=group:default/test', + ) + .send(); + + expect(result.statusCode).toEqual(500); + expect(result.body.error).toEqual({ + name: 'Error', + message: 'Unexpected error', + }); + }); + + it('should fail to delete, because not found error', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return false; + }); + + const result = await request(app) + .delete( + '/roles/role/default/rbac_admin?memberReferences=group:default/test', + ) + .send(); + + expect(result.statusCode).toEqual(404); + expect(result.body.error).toEqual({ + name: 'NotFoundError', + message: '', + }); + }); + + it('should delete a user / group from a role', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + + const result = await request(app) + .delete( + '/roles/role/default/rbac_admin?memberReferences=group:default/test', + ) + .send(); + + expect(result.statusCode).toEqual(204); + }); + + it('should delete a role', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + mockEnforcer.removeGroupingPolicies = jest + .fn() + .mockImplementation(async (..._param: string[]): Promise => { + return true; + }); + + const result = await request(app) + .delete('/roles/role/default/rbac_admin') + .send(); + + expect(result.statusCode).toEqual(204); + }); + }); + describe('GetFirstQuery', () => { describe('getFirstQuery', () => { it('should return an empty string for undefined query value', () => { diff --git a/plugins/rbac-backend/src/service/policies-rest-api.ts b/plugins/rbac-backend/src/service/policies-rest-api.ts index a93a81f04d..5ab66003d6 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.ts @@ -26,19 +26,21 @@ import { isEqual } from 'lodash'; import { ParsedQs } from 'qs'; import { - EntityReferencedPolicy, policyEntityCreatePermission, policyEntityDeletePermission, policyEntityPermissions, policyEntityReadPermission, policyEntityUpdatePermission, RESOURCE_TYPE_POLICY_ENTITY, + Role, + RoleBasedPolicy, } from '@janus-idp/backstage-plugin-rbac-common'; import { validateEntityReference, validatePolicy, - validatePolicyQueries, + validateQueries, + validateRole, } from './policies-validation'; export class PolicesServer { @@ -95,10 +97,12 @@ export class PolicesServer { response.send({ status: 'Authorized' }); }); - router.get('/policies', async (req, response) => { + // Policy CRUD + + router.get('/policies', async (request, response) => { const decision = await this.authorize( this.identity, - req, + request, this.permissions, { permission: policyEntityReadPermission, @@ -110,11 +114,11 @@ export class PolicesServer { } let policies: string[][]; - if (this.isPolicyFilterEnabled(req)) { - const entityRef = this.getFirstQuery(req.query.entityRef); - const permission = this.getFirstQuery(req.query.permission); - const policy = this.getFirstQuery(req.query.policy); - const effect = this.getFirstQuery(req.query.effect); + if (this.isPolicyFilterEnabled(request)) { + const entityRef = this.getFirstQuery(request.query.entityRef); + const permission = this.getFirstQuery(request.query.permission); + const policy = this.getFirstQuery(request.query.policy); + const effect = this.getFirstQuery(request.query.effect); const filter: string[] = [entityRef, permission, policy, effect]; policies = await this.enforcer.getFilteredPolicy(0, ...filter); @@ -125,29 +129,32 @@ export class PolicesServer { response.json(this.transformPolicyArray(...policies)); }); - router.get('/policies/:kind/:namespace/:name', async (req, response) => { - const decision = await this.authorize( - this.identity, - req, - this.permissions, - { - permission: policyEntityReadPermission, - }, - ); + router.get( + '/policies/:kind/:namespace/:name', + async (request, response) => { + const decision = await this.authorize( + this.identity, + request, + this.permissions, + { + permission: policyEntityReadPermission, + }, + ); - if (decision.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); // 403 - } + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); // 403 + } - const entityRef = this.getEntityReference(req); + const entityRef = this.getEntityReference(request); - const policy = await this.enforcer.getFilteredPolicy(0, entityRef); - if (policy.length !== 0) { - response.json(this.transformPolicyArray(...policy)); - } else { - throw new NotFoundError(); // 404 - } - }); + const policy = await this.enforcer.getFilteredPolicy(0, entityRef); + if (policy.length !== 0) { + response.json(this.transformPolicyArray(...policy)); + } else { + throw new NotFoundError(); // 404 + } + }, + ); router.delete( '/policies/:kind/:namespace/:name', @@ -167,7 +174,7 @@ export class PolicesServer { const entityRef = this.getEntityReference(request); - const err = validatePolicyQueries(request); + const err = validateQueries(request); if (err) { throw new InputError( // 400 `Invalid policy definition. Cause: ${err.message}`, @@ -206,7 +213,7 @@ export class PolicesServer { throw new NotAllowedError(); // 403 } - const policyRaw: EntityReferencedPolicy = request.body; + const policyRaw: RoleBasedPolicy = request.body; const err = validatePolicy(policyRaw); if (err) { throw new InputError( // 400 @@ -227,10 +234,169 @@ export class PolicesServer { response.status(201).end(); }); - router.put('/policies/:kind/:namespace/:name', async (req, resp) => { + router.put( + '/policies/:kind/:namespace/:name', + async (request, response) => { + const decision = await this.authorize( + this.identity, + request, + this.permissions, + { + permission: policyEntityUpdatePermission, + }, + ); + + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); // 403 + } + + const entityRef = this.getEntityReference(request); + + const oldPolicyRaw = request.body.oldPolicy; + if (!oldPolicyRaw) { + throw new InputError(`'oldPolicy' object must be present`); // 400 + } + const newPolicyRaw = request.body.newPolicy; + if (!newPolicyRaw) { + throw new InputError(`'newPolicy' object must be present`); // 400 + } + + oldPolicyRaw.entityReference = entityRef; + let err = validatePolicy(oldPolicyRaw); + if (err) { + throw new InputError( // 400 + `Invalid old policy object. Cause: ${err.message}`, + ); + } + newPolicyRaw.entityReference = entityRef; + err = validatePolicy(newPolicyRaw); + if (err) { + throw new InputError( // 400 + `Invalid new policy object. Cause: ${err.message}`, + ); + } + + const oldPolicy = this.transformPolicyToArray(oldPolicyRaw); + const newPolicy = this.transformPolicyToArray(newPolicyRaw); + + if (await this.enforcer.hasPolicy(...newPolicy)) { + if (isEqual(oldPolicy, newPolicy)) { + response.status(204).end(); + return; + } + throw new ConflictError(); // 409 + } + + if (!(await this.enforcer.hasPolicy(...oldPolicy))) { + throw new NotFoundError(); // 404 + } + + // enforcer.updatePolicy(oldPolicyPermission, newPolicyPermission) was not implemented + // for ORMTypeAdapter. + // So, let's compensate this combination delete + create. + const isRemoved = await this.enforcer.removePolicy(...oldPolicy); + if (!isRemoved) { + throw new Error('Unexpected error'); // 500 + } + + const isAdded = await this.enforcer.addPolicy(...newPolicy); + if (!isAdded) { + throw new Error('Unexpected error'); + } + + response.status(200).end(); + }, + ); + + // Role CRUD + + router.get('/roles', async (request, response) => { const decision = await this.authorize( this.identity, - req, + request, + this.permissions, + { + permission: policyEntityReadPermission, + }, + ); + + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); // 403 + } + + const roles = await this.enforcer.getGroupingPolicy(); + + response.json(this.transformRoleArray(...roles)); + }); + + router.get('/roles/:kind/:namespace/:name', async (request, response) => { + const decision = await this.authorize( + this.identity, + request, + this.permissions, + { + permission: policyEntityReadPermission, + }, + ); + + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); // 403 + } + const roleEntityRef = this.getEntityReference(request); + + const role = await this.enforcer.getFilteredGroupingPolicy( + 1, + roleEntityRef, + ); + + if (role.length !== 0) { + response.json(this.transformRoleArray(...role)); + } else { + throw new NotFoundError(); // 404 + } + }); + + router.post('/roles', async (request, response) => { + const decision = await this.authorize( + this.identity, + request, + this.permissions, + { + permission: policyEntityCreatePermission, + }, + ); + + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); // 403 + } + const roleRaw: Role = request.body; + const err = validateRole(roleRaw); + if (err) { + throw new InputError( // 400 + `Invalid role definition. Cause: ${err.message}`, + ); + } + + const roles = this.transformRoleToArray(roleRaw); + + for (const role in roles) { + if (await this.enforcer.hasGroupingPolicy(...role)) { + throw new ConflictError(); // 409 + } + } + + const isAdded = await this.enforcer.addGroupingPolicies(roles); + + if (!isAdded) { + throw new Error('Unexpected error'); // 500 + } + response.status(201).end(); + }); + + router.put('/roles/:kind/:namespace/:name', async (request, response) => { + const decision = await this.authorize( + this.identity, + request, this.permissions, { permission: policyEntityUpdatePermission, @@ -240,71 +406,127 @@ export class PolicesServer { if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError(); // 403 } + const roleEntityRef = this.getEntityReference(request); - const entityRef = this.getEntityReference(req); + const oldRoleRaw = request.body.oldRole; - const oldPolicyRaw = req.body.oldPolicy; - if (!oldPolicyRaw) { - throw new InputError(`'oldPolicy' object must be present`); // 400 + if (!oldRoleRaw) { + throw new InputError(`'oldRole' object must be present`); // 400 } - const newPolicyRaw = req.body.newPolicy; - if (!newPolicyRaw) { - throw new InputError(`'newPolicy' object must be present`); // 400 + const newRoleRaw = request.body.newRole; + if (!newRoleRaw) { + throw new InputError(`'newRole' object must be present`); // 400 } - oldPolicyRaw.entityReference = entityRef; - let err = validatePolicy(oldPolicyRaw); + oldRoleRaw.name = roleEntityRef; + let err = validateRole(oldRoleRaw); if (err) { throw new InputError( // 400 - `Invalid old policy object. Cause: ${err.message}`, + `Invalid old role object. Cause: ${err.message}`, ); } - newPolicyRaw.entityReference = entityRef; - err = validatePolicy(newPolicyRaw); + err = validateRole(newRoleRaw); if (err) { throw new InputError( // 400 - `Invalid new policy object. Cause: ${err.message}`, + `Invalid new role object. Cause: ${err.message}`, ); } - const oldPolicy = this.transformPolicyToArray(oldPolicyRaw); - const newPolicy = this.transformPolicyToArray(newPolicyRaw); - - if (await this.enforcer.hasPolicy(...newPolicy)) { - if (isEqual(oldPolicy, newPolicy)) { - resp.status(204).end(); - return; + const oldRole = this.transformRoleToArray(oldRoleRaw); + const newRole = this.transformRoleToArray(newRoleRaw); + + for (const role of newRole) { + const hasRole = oldRole.some(element => { + return isEqual(element, role); + }); + // if the role is already part of old role and is a grouping policy we want to skip returning a conflict error + // to allow for other roles to be checked and added + if (await this.enforcer.hasGroupingPolicy(...role)) { + if (isEqual(oldRole, newRole)) { + response.status(204).end(); + return; + } + if (!hasRole) { + throw new ConflictError(); // 409 + } } - throw new ConflictError(); // 409 } - if (!(await this.enforcer.hasPolicy(...oldPolicy))) { - throw new NotFoundError(); // 404 + for (const role of oldRole) { + if (!(await this.enforcer.hasGroupingPolicy(...role))) { + throw new NotFoundError(); // 404 + } } - // enforcer.updatePolicy(oldPolicyPermission, newPolicyPermission) was not implemented + // enforcer.updateGroupingPolicy(oldRole, newRole) was not implemented // for ORMTypeAdapter. // So, let's compensate this combination delete + create. - const isRemoved = await this.enforcer.removePolicy(...oldPolicy); + const isRemoved = await this.enforcer.removeGroupingPolicies(oldRole); if (!isRemoved) { throw new Error('Unexpected error'); // 500 } - const isAdded = await this.enforcer.addPolicy(...newPolicy); + const isAdded = await this.enforcer.addGroupingPolicies(newRole); if (!isAdded) { throw new Error('Unexpected error'); } - resp.status(200).end(); + response.status(200).end(); }); + router.delete( + '/roles/:kind/:namespace/:name', + async (request, response) => { + let roles = []; + const decision = await this.authorize( + this.identity, + request, + this.permissions, + { + permission: policyEntityDeletePermission, + }, + ); + + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); // 403 + } + + const roleEntityRef = this.getEntityReference(request); + + if (request.query.memberReferences) { + const memberReferences = this.getFirstQuery( + request.query.memberReferences!, + ); + + roles.push([memberReferences, roleEntityRef]); + } else { + roles = await this.enforcer.getFilteredGroupingPolicy( + 1, + roleEntityRef, + ); + } + + for (const role of roles) { + if (!(await this.enforcer.hasGroupingPolicy(...role))) { + throw new NotFoundError(); // 404 + } + } + + const isRemoved = await this.enforcer.removeGroupingPolicies(roles); + if (!isRemoved) { + throw new Error('Unexpected error'); // 500 + } + response.status(204).end(); + }, + ); + return router; } - getEntityReference(req: Request): string { - const kind = req.params.kind; - const namespace = req.params.namespace; - const name = req.params.name; + getEntityReference(request: Request): string { + const kind = request.params.kind; + const namespace = request.params.namespace; + const name = request.params.name; const entityRef = `${kind}:${namespace}/${name}`; const err = validateEntityReference(entityRef); @@ -315,14 +537,33 @@ export class PolicesServer { return entityRef; } - transformPolicyArray(...policies: string[][]): EntityReferencedPolicy[] { + transformPolicyArray(...policies: string[][]): RoleBasedPolicy[] { return policies.map((p: string[]) => { const [entityReference, permission, policy, effect] = p; return { entityReference, permission, policy, effect }; }); } - transformPolicyToArray(policy: EntityReferencedPolicy): string[] { + transformRoleArray(...roles: string[][]): Role[] { + const combinedRoles: { [key: string]: string[] } = {}; + + roles.forEach(([value, role]) => { + if (combinedRoles.hasOwnProperty(role)) { + combinedRoles[role].push(value); + } else { + combinedRoles[role] = [value]; + } + }); + + const result: Role[] = Object.entries(combinedRoles).map( + ([role, value]) => { + return { memberReferences: value, name: role }; + }, + ); + return result; + } + + transformPolicyToArray(policy: RoleBasedPolicy): string[] { return [ policy.entityReference!, policy.permission!, @@ -331,6 +572,14 @@ export class PolicesServer { ]; } + transformRoleToArray(role: Role): string[][] { + const roles: string[][] = []; + for (const entity of role.memberReferences) { + roles.push([entity, role.name]); + } + return roles; + } + getFirstQuery( queryValue: string | string[] | ParsedQs | ParsedQs[] | undefined, ): string { @@ -350,12 +599,12 @@ export class PolicesServer { throw new InputError(`This api doesn't support nested query`); } - isPolicyFilterEnabled(req: Request): boolean { + isPolicyFilterEnabled(request: Request): boolean { return ( - !!req.query.entityRef || - !!req.query.permission || - !!req.query.policy || - !!req.query.effect + !!request.query.entityRef || + !!request.query.permission || + !!request.query.policy || + !!request.query.effect ); } } diff --git a/plugins/rbac-backend/src/service/policies-validation.test.ts b/plugins/rbac-backend/src/service/policies-validation.test.ts index ce40a17426..bbf10a3ccf 100644 --- a/plugins/rbac-backend/src/service/policies-validation.test.ts +++ b/plugins/rbac-backend/src/service/policies-validation.test.ts @@ -1,22 +1,23 @@ -import { EntityReferencedPolicy } from '@janus-idp/backstage-plugin-rbac-common'; +import { RoleBasedPolicy } from '@janus-idp/backstage-plugin-rbac-common'; import { validateEntityReference, validatePolicy, - validatePolicyQueries, + validateQueries, + validateRole, } from './policies-validation'; describe('rest data validation', () => { describe('validate entity referenced policy', () => { it('should return an error when entity reference is empty', () => { - const policy: EntityReferencedPolicy = {}; + const policy: RoleBasedPolicy = {}; const err = validatePolicy(policy); expect(err).toBeTruthy(); expect(err?.message).toEqual(`'entityReference' must not be empty`); }); it('should return an error when permission is empty', () => { - const policy: EntityReferencedPolicy = { + const policy: RoleBasedPolicy = { entityReference: 'user:default/guest', }; const err = validatePolicy(policy); @@ -25,7 +26,7 @@ describe('rest data validation', () => { }); it('should return an error when policy is empty', () => { - const policy: EntityReferencedPolicy = { + const policy: RoleBasedPolicy = { entityReference: 'user:default/guest', permission: 'catalog-entity', }; @@ -35,7 +36,7 @@ describe('rest data validation', () => { }); it('should return an error when effect is empty', () => { - const policy: EntityReferencedPolicy = { + const policy: RoleBasedPolicy = { entityReference: 'user:default/guest', permission: 'catalog-entity', policy: 'read', @@ -46,7 +47,7 @@ describe('rest data validation', () => { }); it('should return an error when effect has an invalid value', () => { - const policy: EntityReferencedPolicy = { + const policy: RoleBasedPolicy = { entityReference: 'user:default/guest', permission: 'catalog-entity', policy: 'read', @@ -60,7 +61,7 @@ describe('rest data validation', () => { }); it(`pass validation when all fields are valid. Effect 'allow' should be valid`, () => { - const policy: EntityReferencedPolicy = { + const policy: RoleBasedPolicy = { entityReference: 'user:default/guest', permission: 'catalog-entity', policy: 'read', @@ -71,7 +72,7 @@ describe('rest data validation', () => { }); it(`pass validation when all fields are valid. Effect 'deny' should be valid`, () => { - const policy: EntityReferencedPolicy = { + const policy: RoleBasedPolicy = { entityReference: 'user:default/guest', permission: 'catalog-entity', policy: 'read', @@ -151,6 +152,10 @@ describe('rest data validation', () => { ref: 'user:', expectedError: `Entity reference "user:" was not on the form [:][/]`, }, + { + ref: 'admin:default/test', + expectedError: `Unsupported kind admin. List supported values ["user", "group", "role"]`, + }, ]; for (const entityRef of invalidOrUnsupportedEntityRefs) { const err = validateEntityReference(entityRef.ref); @@ -164,6 +169,7 @@ describe('rest data validation', () => { 'user:default/guest', 'user:default/John Doe', 'user:default/John Doe/developer', + 'role:default/team-a', ]; for (const entityRef of invalidOrUnsupportedEntityRefs) { const err = validateEntityReference(entityRef); @@ -175,7 +181,7 @@ describe('rest data validation', () => { describe('validatePolicyQueries', () => { it('should return an error when "permission" query param is missing', () => { const request = { query: { policy: 'read', effect: 'allow' } } as any; - const err = validatePolicyQueries(request); + const err = validateQueries(request); expect(err).toBeTruthy(); expect(err?.message).toEqual('specify "permission" query param.'); }); @@ -184,7 +190,7 @@ describe('rest data validation', () => { const request = { query: { permission: 'user:default/guest', effect: 'allow' }, } as any; - const err = validatePolicyQueries(request); + const err = validateQueries(request); expect(err).toBeTruthy(); expect(err?.message).toEqual('specify "policy" query param.'); }); @@ -193,7 +199,7 @@ describe('rest data validation', () => { const request = { query: { permission: 'user:default/guest', policy: 'read' }, } as any; - const err = validatePolicyQueries(request); + const err = validateQueries(request); expect(err).toBeTruthy(); expect(err?.message).toEqual('specify "effect" query param.'); }); @@ -206,7 +212,27 @@ describe('rest data validation', () => { effect: 'allow', }, } as any; - const err = validatePolicyQueries(request); + const err = validateQueries(request); + expect(err).toBeUndefined(); + }); + }); + + describe('validateRole', () => { + it('should return an error when "memberReferences" query param is missing', () => { + const request = { name: 'role:default/user' } as any; + const err = validateRole(request); + expect(err).toBeTruthy(); + expect(err?.message).toEqual( + `'memberReferences' field must not be empty`, + ); + }); + + it('should pass validation when all required query params are present', () => { + const request = { + memberReferences: ['user:default/guest'], + name: 'role:default/user', + } as any; + const err = validateRole(request); expect(err).toBeUndefined(); }); }); diff --git a/plugins/rbac-backend/src/service/policies-validation.ts b/plugins/rbac-backend/src/service/policies-validation.ts index 516a0b93b6..8f5f7bdcaa 100644 --- a/plugins/rbac-backend/src/service/policies-validation.ts +++ b/plugins/rbac-backend/src/service/policies-validation.ts @@ -3,11 +3,9 @@ import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { Request } from 'express-serve-static-core'; -import { EntityReferencedPolicy } from '@janus-idp/backstage-plugin-rbac-common'; +import { Role, RoleBasedPolicy } from '@janus-idp/backstage-plugin-rbac-common'; -export function validatePolicy( - policy: EntityReferencedPolicy, -): Error | undefined { +export function validatePolicy(policy: RoleBasedPolicy): Error | undefined { const err = validateEntityReference(policy.entityReference); if (err) { return err; @@ -34,6 +32,24 @@ export function validatePolicy( return undefined; } +export function validateRole(role: Role): Error | undefined { + if (!role.name) { + return new Error(`'name' field must not be empty`); + } + + if (!role.memberReferences || role.memberReferences.length === 0) { + return new Error(`'memberReferences' field must not be empty`); + } + + for (const member of role.memberReferences) { + const err = validateEntityReference(member); + if (err) { + return err; + } + } + return undefined; +} + function isValidEffectValue(effect: string): boolean { return ( effect === AuthorizeResult.ALLOW.toLocaleLowerCase() || @@ -61,16 +77,20 @@ export function validateEntityReference(entityRef?: string): Error | undefined { ); } - if (entityRefCompound.kind !== 'user' && entityRefCompound.kind !== 'group') { + if ( + entityRefCompound.kind !== 'user' && + entityRefCompound.kind !== 'group' && + entityRefCompound.kind !== 'role' + ) { return new Error( - `Unsupported kind ${entityRefCompound.kind}. List supported values ["user", "group"]`, + `Unsupported kind ${entityRefCompound.kind}. List supported values ["user", "group", "role"]`, ); } return undefined; } -export function validatePolicyQueries(request: Request): Error | undefined { +export function validateQueries(request: Request): Error | undefined { if (!request.query.permission) { return new Error('specify "permission" query param.'); } diff --git a/plugins/rbac-backend/src/service/role-manager.test.ts b/plugins/rbac-backend/src/service/role-manager.test.ts index eb357f77c5..8a16d77b94 100644 --- a/plugins/rbac-backend/src/service/role-manager.test.ts +++ b/plugins/rbac-backend/src/service/role-manager.test.ts @@ -35,18 +35,6 @@ describe('BackstageRoleManager', () => { }); describe('unimplemented methods', () => { - it('should throw an error for addLink', async () => { - await expect( - roleManager.addLink('user:default/role1', 'user:default/role2'), - ).rejects.toThrow('Method "addLink" not implemented.'); - }); - - it('should throw an error for deleteLink', async () => { - await expect( - roleManager.deleteLink('user:default/role1', 'user:default/role2'), - ).rejects.toThrow('Method "deleteLink" not implemented.'); - }); - it('should throw an error for syncedHasLink', () => { expect(() => roleManager.syncedHasLink!('user:default/role1', 'user:default/role2'), @@ -66,6 +54,28 @@ describe('BackstageRoleManager', () => { }); }); + describe('addLink test', () => { + it('should create a link between two entities', async () => { + roleManager.addLink('user:default/test', 'role:default/rbac_admin'); + const result = await roleManager.hasLink( + 'user:default/test', + 'role:default/rbac_admin', + ); + expect(result).toBe(true); + }); + }); + + describe('deleteLink test', () => { + it('should delete a link', async () => { + roleManager.addLink('test', 'role:test', ''); + roleManager.addLink('test', 'role:test2', ''); + + roleManager.deleteLink('test', 'role:test'); + const result = await roleManager.hasLink('test', 'role:test'); + expect(result).toBe(false); + }); + }); + describe('hasLink tests', () => { it('should throw an error for unsupported domain', async () => { await expect( @@ -123,6 +133,52 @@ describe('BackstageRoleManager', () => { expect(result).toBeFalsy(); }); + // user:default/mike should not inherits from role:default/role + // + // Hierarchy: + // + // user:default/mike -> user without role + // + it('should return false for hasLink when user without role', async () => { + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/somerole', + ); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + { + filter: { + kind: 'Group', + 'relations.hasMember': ['user:default/mike'], + }, + fields: [ + 'metadata.name', + 'kind', + 'metadata.namespace', + 'spec.parent', + 'spec.children', + ], + }, + { token: 'some-token' }, + ); + expect(result).toBeFalsy(); + }); + + // user:default/mike should inherits from role:default/somerole + // + // Hierarchy: + // + // user:default/mike -> role:default/somerole + // + it('should return true for hasLink when user:default/mike inherits from role:default/somerole', async () => { + roleManager.addLink('user:default/mike', 'role:default/somerole'); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/somerole', + ); + expect(result).toBeTruthy(); + }); + // user:default/mike should inherits from group:default/somegroup // // Hierarchy: @@ -142,6 +198,26 @@ describe('BackstageRoleManager', () => { expect(result).toBeTruthy(); }); + // user:default/mike should inherits from with role:default/somerole from group:default/somegroup + // + // Hierarchy: + // + // group:default/somegroup -> role:default/somerole + // | + // user:default/mike + // + it('should return true for hasLink when user:default/mike inherits role:default/somerole from group:default/somegroup', async () => { + const entityMock = createGroupEntity('somegroup', undefined, []); + roleManager.addLink('group:default/somegroup', 'role:default/somerole'); + catalogApiMock.getEntities.mockReturnValue({ items: [entityMock] }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/somerole', + ); + expect(result).toBeTruthy(); + }); + // user:default/mike should not inherits from group:default/somegroup // // Hierarchy: @@ -161,6 +237,26 @@ describe('BackstageRoleManager', () => { expect(result).toBeFalsy(); }); + // user:default/mike should not inherits from role:default/somerole + // + // Hierarchy: + // + // group:default/not-matched-group role:default/somerole + // | | + // user:default/mike group:default/somegroup + // + it('should return false for hasLink when user:default/mike does not inherits role:default/somerole', async () => { + const entityMock = createGroupEntity('not-matched-group', undefined, []); + roleManager.addLink('group:default/somegroup', 'role:default/somerole'); + catalogApiMock.getEntities.mockReturnValue({ items: [entityMock] }); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/somerole', + ); + expect(result).toBeFalsy(); + }); + // user:default/mike should inherits from group:default/team-a // // Hierarchy: @@ -196,6 +292,43 @@ describe('BackstageRoleManager', () => { expect(result).toBeTruthy(); }); + // user:default/mike should inherits role:default/team-a from group:default/team-a + // + // Hierarchy: + // + // group:default/team-a -> role:default/team-a + // | + // group:default/team-b + // | + // user:default/mike + // + it('should return true for hasLink, when user:default/mike inherits role:default/team-a from group:default/team-a', async () => { + const groupMock = createGroupEntity('team-b', 'team-a', []); + const groupParentMock = createGroupEntity('team-a', undefined, [ + 'team-b', + ]); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupParentMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-a', 'role:default/team-a'); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-a', + ); + expect(result).toBeTruthy(); + }); + // user:default/mike should inherits from group:default/team-b. // // Hierarchy: @@ -248,6 +381,60 @@ describe('BackstageRoleManager', () => { expect(result).toBeTruthy(); }); + // user:default/mike should inherits role:default/team-a from group:default/team-a. + // + // Hierarchy: + // + // |---------group:default/team-b------------| -> role:default/team-b + // | | | + // user:default/team-c group:default/team-a group:default/team-d + // | | | + // user:default/tom user:default/mike user:default:john + // + it('should return true for hasLink, when user:default/mike inherits role:default/team-b from group:default/team-b', async () => { + const groupAMock = createGroupEntity('team-a', undefined, [ + 'team-b', + 'team-c', + 'team-d', + ]); + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupCMock = createGroupEntity('team-c', 'team-a', []); + const groupDMock = createGroupEntity('team-d', 'team-a', []); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupBMock] }; + } + if (hasMember && hasMember[0] === 'user:default/tom') { + return { items: [groupCMock] }; + } + if (hasMember && hasMember[0] === 'user:default/john') { + return { items: [groupDMock] }; + } + + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + if (hasParent && hasParent[0] === 'group:default/team-c') { + return { items: [groupAMock] }; + } + if (hasParent && hasParent[0] === 'group:default/team-d') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-b', 'role:default/team-b'); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-b', + ); + expect(result).toBeTruthy(); + }); + // user:default/mike should not inherits from group:default/team-c // // Hierarchy: @@ -281,6 +468,41 @@ describe('BackstageRoleManager', () => { expect(result).toBeFalsy(); }); + // user:default/mike should not inherits role:default/team-c from group:default/team-c + // + // Hierarchy: + // + // group:default/team-a group:default/team-c -> role:default/team-c + // | + // group:default/team-b + // | + // user:default/mike + // + it('should return false for hasLink, when user:default/mike does not inherits role:default/team-c from group:default/team-c', async () => { + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupAMock = createGroupEntity('team-a', undefined, ['team-b']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupBMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-c', 'role:default/team-c'); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-c', + ); + expect(result).toBeFalsy(); + }); + // user:default/mike should inherits from group:default/team-a // // Hierarchy: @@ -320,6 +542,47 @@ describe('BackstageRoleManager', () => { expect(result).toBeTruthy(); }); + // user:default/mike should inherits role:default/team-a from group:default/team-a + // + // Hierarchy: + // + // group:default/team-a -> role:default/team-a group:default/team-b + // | | + // group:default/team-c group:default/team-d + // | | + // |--------user:default/mike -------------------| + // + it('should return true for hasLink, when user:default/mike inherits role:default/team-a group tree with group:default/team-a', async () => { + const groupCMock = createGroupEntity('team-c', 'team-a', []); + const groupDMock = createGroupEntity('team-d', 'team-b', []); + const groupAMock = createGroupEntity('team-a', undefined, ['team-c']); + const groupBMock = createGroupEntity('team-b', undefined, ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock, groupDMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if ( + hasParent && + hasParent[0] === 'group:default/team-c' && + hasParent[1] === 'group:default/team-d' + ) { + return { items: [groupAMock, groupBMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-a', 'role:default/team-a'); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-a', + ); + expect(result).toBeTruthy(); + }); + // user:default/mike should not inherits from group:default/team-e // // Hierarchy: @@ -359,6 +622,47 @@ describe('BackstageRoleManager', () => { expect(result).toBeFalsy(); }); + // user:default/mike should not inherits role:default/team-e from group:default/team-e + // + // Hierarchy: + // + // group:default/team-a group:default/team-b group:default/team-e -> role:default/team-e + // | | + // group:default/team-c group:default/team-d + // | | + // user:default/mike + // + it('should return false for hasLink, when user:default/mike inherits role:default/team-e from group:default/team-e', async () => { + const groupCMock = createGroupEntity('team-c', 'team-a', []); + const groupDMock = createGroupEntity('team-d', 'team-b', []); + const groupAMock = createGroupEntity('team-a', undefined, ['team-c']); + const groupBMock = createGroupEntity('team-b', undefined, ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock, groupDMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if ( + hasParent && + hasParent[0] === 'group:default/team-c' && + hasParent[1] === 'group:default/team-d' + ) { + return { items: [groupAMock, groupBMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-e', 'role:default/team-e'); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-e', + ); + expect(result).toBeFalsy(); + }); + // user:default/mike should inherits from group:default/team-b and group:default/team-a, but we have cycle dependency. // So return false on call hasLink. // @@ -405,6 +709,55 @@ describe('BackstageRoleManager', () => { ); }); + // user:default/mike should inherits role:default/team-b and role:default/team-a from group:default/team-b and group:default/team-a, but we have cycle dependency. + // So return false on call hasLink. + // + // Hierarchy: + // + // group:default/team-a -> role:default/team-a + // ↓ ↑ + // group:default/team-b -> role:default/team-b + // ↓ + // user:default/mike + // + it('should return false for hasLink, when user:default/mike inherits role:default/team-b and role:default/team-a from group:default/team-a and group:default/team-b, but we have cycle dependency', async () => { + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupAMock = createGroupEntity('team-a', 'team-b', ['team-b']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupBMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-b', 'role:default/team-b'); + roleManager.addLink('group:default/team-a', 'role:default/team-a'); + + let result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-b', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-b', + ); + + result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + }); + // user:default/mike should inherits from group:default/team-a, group:default/team-b, group:default/team-c, but we have cycle dependency. // So return false on call hasLink. // @@ -466,6 +819,71 @@ describe('BackstageRoleManager', () => { ); }); + // user:default/mike should inherits the roles from group:default/team-a, group:default/team-b, group:default/team-c, but we have cycle dependency. + // So return false on call hasLink. + // + // Hierarchy: + // + // group:default/team-a -> role:default/team-a + // ↓ ↑ + // group:default/team-b -> role:default/team-b + // ↓ + // group:default/team-c -> role:default/team-c + // ↓ + // user:default/mike + // + it('should return false for hasLink, when user:default/mike inherits the roles from group:default/team-a, group:default/team-b, group:default/team-c, but we have cycle dependency', async () => { + const groupAMock = createGroupEntity('team-a', 'team-b', ['team-b']); + const groupBMock = createGroupEntity('team-b', 'team-a', []); + const groupCMock = createGroupEntity('team-c', 'team-b', []); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + if (hasParent && hasParent[0] === 'group:default/team-c') { + return { items: [groupBMock] }; + } + if (hasParent && hasParent[0] === 'group:default/team-b') { + return { items: [groupAMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-a', 'role:default/team-a'); + roleManager.addLink('group:default/team-b', 'role:default/team-b'); + roleManager.addLink('group:default/team-c', 'role:default/team-c'); + + let result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-c', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-c', + ); + + result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-b', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-b', + ); + + result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-b"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + }); + // user:default/mike should inherits from group:default/team-a, but we have cycle dependency: team-a -> team-c. // So return false on call hasLink. // @@ -511,6 +929,53 @@ describe('BackstageRoleManager', () => { ); }); + // user:default/mike should inherits role from group:default/team-a, but we have cycle dependency: team-a -> team-c. + // So return false on call hasLink. + // + // Hierarchy: + // + // group:default/team-a -> role:default/team-a group:default/team-b + // ↓ ↑ ↓ + // group:default/team-c -> role:default/team-c group:default/team-d + // ↓ ↓ + // user:default/mike -------------------| + // + it('should return false for hasLink, when user:default/mike inherits role from group tree with group:default/team-a, but we cycle dependency', async () => { + const groupCMock = createGroupEntity('team-c', 'team-a', ['mike']); + const groupDMock = createGroupEntity('team-d', 'team-b', ['mike']); + const groupAMock = createGroupEntity('team-a', 'team-c', ['team-c']); + const groupBMock = createGroupEntity('team-b', undefined, ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + // first iteration + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock, groupDMock] }; + } + const hasParent = arg.filter['relations.parentOf']; + // second iteration + if ( + hasParent && + hasParent[0] === 'group:default/team-c' && + hasParent[1] === 'group:default/team-d' + ) { + return { items: [groupAMock, groupBMock] }; + } + return { items: [] }; + }); + roleManager.addLink('group:default/team-a', 'role:default/team-a'); + roleManager.addLink('group:default/team-c', 'role:default/team-c'); + + const result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-c"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + }); + // user:default/mike should inherits from group:default/team-a, but we have cycle dependency: team-a -> team-c. // So return false on call hasLink. // @@ -581,6 +1046,80 @@ describe('BackstageRoleManager', () => { ); expect(result).toBeTruthy(); }); + + // user:default/mike should inherits role:default/team-a from group:default/team-a, but we have cycle dependency: team-a -> team-c. + // So return false on call hasLink. + // + // user:default/tom should inherits role:default/team-b from group:default/team-b. Cycle dependency in the neighbor subgraph, should + // not affect evaluation user:default/tom inheritance. + // + // Hierarchy: + // + // group:default/root + // ↓ ↓ + // group:default/team-a -> role:default/team-a group:default/team-b -> role:default/team-b + // ↓ ↑ ↓ + // group:default/team-c group:default/team-d + // ↓ ↓ + // user:default/mike user:default/tom + // + it('should return false for hasLink for user:default/mike and role:default/team-a(cycle dependency), but should be true for user:default/tom and role:default/team-b', async () => { + const groupRootMock = createGroupEntity('root', undefined, [ + 'team-a', + 'team-b', + ]); + const groupCMock = createGroupEntity('team-c', 'team-a', ['team-a']); + const groupDMock = createGroupEntity('team-d', 'team-b', []); + const groupAMock = createGroupEntity('team-a', 'root', ['team-c']); + const groupBMock = createGroupEntity('team-b', 'root', ['team-d']); + + catalogApiMock.getEntities.mockImplementation((arg: any) => { + const hasMember = arg.filter['relations.hasMember']; + + if (hasMember && hasMember[0] === 'user:default/mike') { + return { items: [groupCMock] }; + } + if (hasMember && hasMember[0] === 'user:default/tom') { + return { items: [groupDMock] }; + } + + const hasParent = arg.filter['relations.parentOf']; + + if (hasParent && hasParent[0] === 'group:default/team-c') { + return { items: [groupAMock] }; + } + + if (hasParent && hasParent[0] === 'group:default/team-d') { + return { items: [groupBMock] }; + } + + if ( + (hasParent && hasParent[0] === 'group:default/team-b') || + hasParent[0] === 'group:default/team-a' + ) { + return { items: [groupRootMock] }; + } + return { items: [] }; + }); + + roleManager.addLink('group:default/team-a', 'role:default/team-a'); + roleManager.addLink('group:default/team-b', 'role:default/team-b'); + + let result = await roleManager.hasLink( + 'user:default/mike', + 'role:default/team-a', + ); + expect(result).toBeFalsy(); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Detected cycle dependencies in the Group graph: [["group:default/team-a","group:default/team-c"]]. Admin/(catalog owner) have to fix it to make RBAC permission evaluation correct for group: group:default/team-a', + ); + + result = await roleManager.hasLink( + 'user:default/tom', + 'role:default/team-b', + ); + expect(result).toBeTruthy(); + }); }); function createGroupEntity( name: string, diff --git a/plugins/rbac-backend/src/service/role-manager.ts b/plugins/rbac-backend/src/service/role-manager.ts index cad710cfe5..84620415f3 100644 --- a/plugins/rbac-backend/src/service/role-manager.ts +++ b/plugins/rbac-backend/src/service/role-manager.ts @@ -8,6 +8,44 @@ import { Logger } from 'winston'; type FilterRelations = 'relations.hasMember' | 'relations.parentOf'; +class Role { + public name: string; + + private roles: Role[]; + + public constructor(name: string) { + this.name = name; + this.roles = []; + } + + public addRole(role: Role): void { + if (this.roles.some(n => n.name === role.name)) { + return; + } + this.roles.push(role); + } + + public deleteRole(role: Role): void { + this.roles = this.roles.filter(n => n.name !== role.name); + } + + public hasRole(name: string, hierarchyLevel: number): boolean { + if (this.name === name) { + return true; + } + if (hierarchyLevel <= 0) { + return false; + } + for (const role of this.roles) { + if (role.hasRole(name, hierarchyLevel - 1)) { + return true; + } + } + + return false; + } +} + // AncestorSearchMemo - should be used to build group hierarchy graph for User entity reference. // It supports search group entity reference link in the graph. // Also AncestorSearchMemo supports detection cycle dependencies between groups in the graph. @@ -73,11 +111,14 @@ class AncestorSearchMemo { } export class BackstageRoleManager implements RoleManager { + private allRoles: Map; constructor( private readonly catalogApi: CatalogApi, private readonly log: Logger, private readonly tokenManager: TokenManager, - ) {} + ) { + this.allRoles = new Map(); + } /** * clear clears all stored data and resets the role manager to the initial state. @@ -92,11 +133,14 @@ export class BackstageRoleManager implements RoleManager { * domain is a prefix to the roles. */ async addLink( - _name1: string, - _name2: string, + name1: string, + name2: string, ..._domain: string[] ): Promise { - throw new Error('Method "addLink" not implemented.'); + const role1 = this.getOrCreateRole(name1); + const role2 = this.getOrCreateRole(name2); + + role1.addRole(role2); } /** @@ -105,16 +149,21 @@ export class BackstageRoleManager implements RoleManager { * domain is a prefix to the roles. */ async deleteLink( - _name1: string, - _name2: string, + name1: string, + name2: string, ..._domain: string[] ): Promise { - throw new Error('Method "deleteLink" not implemented.'); + const role1 = this.getOrCreateRole(name1); + const role2 = this.getOrCreateRole(name2); + role1.deleteRole(role2); } /** * hasLink determines whether role: name1 inherits role: name2. * domain is a prefix to the roles. + * + * name1 will always be the user that we are authorizing + * name2 will be the roles that the role manager is aware of */ async hasLink( name1: string, @@ -129,6 +178,13 @@ export class BackstageRoleManager implements RoleManager { return true; } + const tempRole = this.getOrCreateRole(name1); + + // Immediately check if the our temporary role has a link with the role that we are comparing it to + if (this.parseEntityKind(name2) === 'role' && tempRole.hasRole(name2, 1)) { + return true; + } + // name1 is always user in our case. // name2 is user or group. // user(name1) couldn't inherit user(name2). @@ -144,8 +200,6 @@ export class BackstageRoleManager implements RoleManager { if (!memo.isAcyclic()) { const cycles = memo.findCycles(); - memo.hasEntityRef(name2); - this.log.warn( `Detected cycle ${ cycles.length > 0 ? 'dependencies' : 'dependency' @@ -156,6 +210,16 @@ export class BackstageRoleManager implements RoleManager { return false; } + + // iterate through the known roles to check if the second name is apart of the known roles + // and if the group that is attached to the second name is apart of the graph + if (!memo.hasEntityRef(name2) && this.parseEntityKind(name2) === 'role') { + for (const [key, value] of this.allRoles.entries()) { + if (value.hasRole(name2, 1) && memo.hasEntityRef(key)) { + return true; + } + } + } return memo.hasEntityRef(name2); } @@ -244,4 +308,21 @@ export class BackstageRoleManager implements RoleManager { await this.findAncestorGroups(memo); } } + + private getOrCreateRole(name: string): Role { + const role = this.allRoles.get(name); + if (role) { + return role; + } + const newRole = new Role(name); + this.allRoles.set(name, newRole); + + return newRole; + } + + // parse the entity to find out if it is a user / group / or role + private parseEntityKind(name: string): string { + const parsed = name.split(':'); + return parsed[0]; + } } diff --git a/plugins/rbac-common/src/types.ts b/plugins/rbac-common/src/types.ts index 1271aeb61f..b406715920 100644 --- a/plugins/rbac-common/src/types.ts +++ b/plugins/rbac-common/src/types.ts @@ -4,10 +4,15 @@ export type Policy = { effect?: string; }; -export type EntityReferencedPolicy = Policy & { +export type RoleBasedPolicy = Policy & { entityReference?: string; }; +export type Role = { + memberReferences: string[]; + name: string; +}; + export type UpdatePolicy = { oldPolicy: Policy; newPolicy: Policy;