diff --git a/plugins/rbac-backend/migrations/20231212224526_migrations.js b/plugins/rbac-backend/migrations/20231212224526_migrations.js index 4f45c733d5..b4e4d82002 100644 --- a/plugins/rbac-backend/migrations/20231212224526_migrations.js +++ b/plugins/rbac-backend/migrations/20231212224526_migrations.js @@ -4,6 +4,7 @@ */ exports.up = async function up(knex) { const casbinDoesExist = await knex.schema.hasTable('casbin_rule'); + const policyMetadataDoesExist = await knex.schema.hasTable('policy-metadata'); let policies = []; let groupPolicies = []; @@ -34,26 +35,32 @@ exports.up = async function up(knex) { }); } - await knex.schema - .createTable('policy-metadata', table => { - table.increments('id').primary(); - table.string('policy').primary(); - table.string('source'); - }) - .then(async () => { - for (const policy of policies) { - await knex - .table('policy-metadata') - .insert({ source: 'legacy', policy: policy }); - } - }) - .then(async () => { - for (const groupPolicy of groupPolicies) { - await knex - .table('policy-metadata') - .insert({ source: 'legacy', policy: groupPolicy }); - } - }); + if (!policyMetadataDoesExist) { + await knex.schema + .createTable('policy-metadata', table => { + table.increments('id').primary(); + table.string('policy').primary(); + table.string('source'); + }) + .then(async () => { + const metadata = []; + for (const policy of policies) { + metadata.push({ source: 'legacy', policy: policy }); + } + if (metadata.length > 0) { + await knex.table('policy-metadata').insert(metadata); + } + }) + .then(async () => { + const metadata = []; + for (const groupPolicy of groupPolicies) { + metadata.push({ source: 'legacy', policy: groupPolicy }); + } + if (metadata.length > 0) { + await knex.table('policy-metadata').insert(metadata); + } + }); + } }; /** diff --git a/plugins/rbac-backend/migrations/20231221113214_migrations.js b/plugins/rbac-backend/migrations/20231221113214_migrations.js index 30b4c31125..dd54381289 100644 --- a/plugins/rbac-backend/migrations/20231221113214_migrations.js +++ b/plugins/rbac-backend/migrations/20231221113214_migrations.js @@ -4,44 +4,39 @@ */ exports.up = async function up(knex) { const casbinDoesExist = await knex.schema.hasTable('casbin_rule'); - let groupPolicies = []; + const roleMetadataDoesExist = await knex.schema.hasTable('role-metadata'); + const groupPolicies = new Set(); if (casbinDoesExist) { - groupPolicies = await knex + await knex .select('*') .from('casbin_rule') .where('ptype', 'g') .then(listGroupPolicies => { - const allGroupPolicies = []; - let rbacFlag = false; for (const groupPolicy of listGroupPolicies) { const { v1 } = groupPolicy; - if (v1 === 'role:default/rbac_admin') { - rbacFlag = true; - continue; - } - allGroupPolicies.push(v1); + groupPolicies.add(v1); } - if (rbacFlag) { - allGroupPolicies.push('role:default/rbac_admin'); - } - return allGroupPolicies; }); } - await knex.schema - .createTable('role-metadata', table => { - table.increments('id').primary(); - table.string('roleEntityRef').primary(); - table.string('source'); - }) - .then(async () => { - for (const groupPolicy of groupPolicies) { - await knex - .table('role-metadata') - .insert({ source: 'legacy', roleEntityRef: groupPolicy }); - } - }); + if (!roleMetadataDoesExist) { + await knex.schema + .createTable('role-metadata', table => { + table.increments('id').primary(); + table.string('roleEntityRef').primary(); + table.string('source'); + }) + .then(async () => { + const metadata = []; + for (const groupPolicy of groupPolicies) { + metadata.push({ source: 'legacy', roleEntityRef: groupPolicy }); + } + if (metadata.length > 0) { + await knex.table('role-metadata').insert(metadata); + } + }); + } }; /** diff --git a/plugins/rbac-backend/migrations/20240201144429_migrations.js b/plugins/rbac-backend/migrations/20240201144429_migrations.js new file mode 100644 index 0000000000..6a5162d31c --- /dev/null +++ b/plugins/rbac-backend/migrations/20240201144429_migrations.js @@ -0,0 +1,25 @@ +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.up = async function up(knex) { + const isRoleMetaDataExist = await knex.schema.hasTable('role-metadata'); + if (isRoleMetaDataExist) { + await knex.schema.alterTable('role-metadata', table => { + table.string('description'); + }); + } +}; + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.down = async function down(knex) { + const isRoleMetaDataExist = await knex.schema.hasTable('role-metadata'); + if (isRoleMetaDataExist) { + await knex.schema.alterTable('role-metadata', table => { + table.dropColumn('description'); + }); + } +}; diff --git a/plugins/rbac-backend/src/database/policy-metadata-storage.test.ts b/plugins/rbac-backend/src/database/policy-metadata-storage.test.ts index 2b66dd6d3b..bf851306a7 100644 --- a/plugins/rbac-backend/src/database/policy-metadata-storage.test.ts +++ b/plugins/rbac-backend/src/database/policy-metadata-storage.test.ts @@ -31,6 +31,17 @@ describe('policy-metadata-db-table', () => { }), migrations: { skip: false }, }; + await knex.schema.createTable('casbin_rule', table => { + table.increments('id').primary(); + table.string('ptype'); + table.string('v0'); + table.string('v1'); + table.string('v2'); + table.string('v3'); + table.string('v4'); + table.string('v5'); + table.string('v6'); + }); await migrate(databaseManagerMock); return { knex, diff --git a/plugins/rbac-backend/src/database/role-metadata.test.ts b/plugins/rbac-backend/src/database/role-metadata.test.ts index 494b42a512..29129ac0d7 100644 --- a/plugins/rbac-backend/src/database/role-metadata.test.ts +++ b/plugins/rbac-backend/src/database/role-metadata.test.ts @@ -26,6 +26,17 @@ describe('role-metadata-db-table', () => { }), migrations: { skip: false }, }; + await knex.schema.createTable('casbin_rule', table => { + table.increments('id').primary(); + table.string('ptype'); + table.string('v0'); + table.string('v1'); + table.string('v2'); + table.string('v3'); + table.string('v4'); + table.string('v5'); + table.string('v6'); + }); await migrate(databaseManagerMock); return { knex, @@ -69,7 +80,12 @@ describe('role-metadata-db-table', () => { trx, ); await trx.commit(); - expect(roleMetadata).toEqual({ source: 'rest' }); + expect(roleMetadata).toEqual({ + description: null, + id: 1, + roleEntityRef: 'role:default/some-super-important-role', + source: 'rest', + }); } catch (err) { await trx.rollback(); throw err; @@ -88,8 +104,10 @@ describe('role-metadata-db-table', () => { let id; try { id = await db.createRoleMetadata( - { source: 'configuration' }, - 'role:default/some-super-important-role', + { + source: 'configuration', + roleEntityRef: 'role:default/some-super-important-role', + }, trx, ); await trx.commit(); @@ -105,6 +123,7 @@ describe('role-metadata-db-table', () => { expect(metadata.length).toEqual(1); expect(metadata[0]).toEqual({ roleEntityRef: 'role:default/some-super-important-role', + description: null, id: 1, source: 'configuration', }); @@ -125,8 +144,11 @@ describe('role-metadata-db-table', () => { await expect(async () => { try { await db.createRoleMetadata( - { source: 'configuration' }, - 'role:default/some-super-important-role', + { + source: 'configuration', + roleEntityRef: 'role:default/some-super-important-role', + }, + trx, ); await trx.commit(); @@ -151,12 +173,14 @@ describe('role-metadata-db-table', () => { await expect( db.createRoleMetadata( - { source: 'configuration' }, - 'role:default/some-super-important-role', + { + source: 'configuration', + roleEntityRef: 'role:default/some-super-important-role', + }, trx, ), ).rejects.toThrow( - `Failed to create the role metadata: '{"roleEntityRef":"role:default/some-super-important-role","source":"configuration"}'.`, + `Failed to create the role metadata: '{"source":"configuration","roleEntityRef":"role:default/some-super-important-role"}'.`, ); }); @@ -172,8 +196,10 @@ describe('role-metadata-db-table', () => { const trx = await knex.transaction(); try { await db.createRoleMetadata( - { source: 'configuration' }, - 'role:default/some-super-important-role', + { + source: 'configuration', + roleEntityRef: 'role:default/some-super-important-role', + }, trx, ); await trx.commit(); @@ -182,7 +208,7 @@ describe('role-metadata-db-table', () => { throw err; } }).rejects.toThrow( - `Failed to create the role metadata: '{"roleEntityRef":"role:default/some-super-important-role","source":"configuration"}'.`, + `Failed to create the role metadata: '{"source":"configuration","roleEntityRef":"role:default/some-super-important-role"}'.`, ); }); @@ -200,8 +226,10 @@ describe('role-metadata-db-table', () => { const trx = await knex.transaction(); try { await db.createRoleMetadata( - { source: 'configuration' }, - 'role:default/some-super-important-role', + { + source: 'configuration', + roleEntityRef: 'role:default/some-super-important-role', + }, trx, ); await trx.commit(); @@ -246,6 +274,7 @@ describe('role-metadata-db-table', () => { ); expect(metadata.length).toEqual(1); expect(metadata[0]).toEqual({ + description: null, source: 'rest', roleEntityRef: 'role:default/some-super-important-role', id: 1, @@ -315,6 +344,7 @@ describe('role-metadata-db-table', () => { ); expect(metadata.length).toEqual(1); expect(metadata[0]).toEqual({ + description: null, source: 'configuration', roleEntityRef: 'role:default/important-role', id: 1, diff --git a/plugins/rbac-backend/src/database/role-metadata.ts b/plugins/rbac-backend/src/database/role-metadata.ts index d13bb46448..8311609e04 100644 --- a/plugins/rbac-backend/src/database/role-metadata.ts +++ b/plugins/rbac-backend/src/database/role-metadata.ts @@ -2,28 +2,30 @@ import { ConflictError, InputError, NotFoundError } from '@backstage/errors'; import { Knex } from 'knex'; -import { RoleMetadata } from '@janus-idp/backstage-plugin-rbac-common'; +import { RoleMetadata, Source } from '@janus-idp/backstage-plugin-rbac-common'; + +import { deepSortedEqual } from '../helper'; export const ROLE_METADATA_TABLE = 'role-metadata'; export interface RoleMetadataDao extends RoleMetadata { id?: number; roleEntityRef: string; + source: Source; } export interface RoleMetadataStorage { findRoleMetadata( roleEntityRef: string, trx?: Knex.Transaction, - ): Promise; + ): Promise; createRoleMetadata( - roleMetadata: RoleMetadata, - roleEntityRef: string, + roleMetadata: RoleMetadataDao, trx: Knex.Transaction, ): Promise; updateRoleMetadata( roleMetadata: RoleMetadataDao, - roleEntityRef: string, + oldRoleEntityRef: string, trx: Knex.Transaction, ): Promise; removeRoleMetadata( @@ -38,17 +40,6 @@ export class DataBaseRoleMetadataStorage implements RoleMetadataStorage { async findRoleMetadata( roleEntityRef: string, trx: Knex.Transaction, - ): Promise { - const roleMetadataDao = await this.findRoleMetadataDao(roleEntityRef, trx); - if (roleMetadataDao) { - return this.daoToMetadata(roleMetadataDao); - } - return undefined; - } - - private async findRoleMetadataDao( - roleEntityRef: string, - trx: Knex.Transaction, ): Promise { const db = trx || this.knex; return await db @@ -59,62 +50,64 @@ export class DataBaseRoleMetadataStorage implements RoleMetadataStorage { } async createRoleMetadata( - roleMetadata: RoleMetadata, - roleEntityRef: string, + metadata: RoleMetadataDao, trx: Knex.Transaction, ): Promise { - if (await this.findRoleMetadataDao(roleEntityRef, trx)) { + if (await this.findRoleMetadata(metadata.roleEntityRef, trx)) { throw new ConflictError( - `A metadata for role ${roleEntityRef} has already been stored`, + `A metadata for role ${metadata.roleEntityRef} has already been stored`, ); } - const metadataDao = this.metadataToDao(roleMetadata, roleEntityRef); const result = await trx(ROLE_METADATA_TABLE) - .insert(metadataDao) + .insert(metadata) .returning<[{ id: number }]>('id'); if (result && result?.length > 0) { return result[0].id; } throw new Error( - `Failed to create the role metadata: '${JSON.stringify(metadataDao)}'.`, + `Failed to create the role metadata: '${JSON.stringify(metadata)}'.`, ); } async updateRoleMetadata( - newRoleMetadataDao: RoleMetadataDao, - roleEntityRef: string, + newRoleMetadata: RoleMetadataDao, + oldRoleEntityRef: string, trx: Knex.Transaction, ): Promise { - const currentMetadataDao = await this.findRoleMetadataDao( - roleEntityRef, + const currentMetadataDao = await this.findRoleMetadata( + oldRoleEntityRef, trx, ); if (!currentMetadataDao) { throw new NotFoundError( - `A metadata for role '${roleEntityRef}' was not found`, + `A metadata for role '${oldRoleEntityRef}' was not found`, ); } if ( currentMetadataDao.source !== 'legacy' && - currentMetadataDao.source !== newRoleMetadataDao.source + currentMetadataDao.source !== newRoleMetadata.source ) { throw new InputError(`The RoleMetadata.source field is 'read-only'.`); } + if (deepSortedEqual(currentMetadataDao, newRoleMetadata)) { + return; + } + const result = await trx(ROLE_METADATA_TABLE) .where('id', currentMetadataDao.id) - .update(newRoleMetadataDao) + .update(newRoleMetadata) .returning('id'); if (!result || result.length === 0) { throw new Error( `Failed to update the role metadata '${JSON.stringify( currentMetadataDao, - )}' with new value: '${JSON.stringify(newRoleMetadataDao)}'.`, + )}' with new value: '${JSON.stringify(newRoleMetadata)}'.`, ); } } @@ -123,7 +116,7 @@ export class DataBaseRoleMetadataStorage implements RoleMetadataStorage { roleEntityRef: string, trx: Knex.Transaction, ): Promise { - const metadataDao = await this.findRoleMetadataDao(roleEntityRef, trx); + const metadataDao = await this.findRoleMetadata(roleEntityRef, trx); if (!metadataDao) { throw new NotFoundError( `A metadata for role '${roleEntityRef}' was not found`, @@ -134,20 +127,22 @@ export class DataBaseRoleMetadataStorage implements RoleMetadataStorage { .delete() .whereIn('id', [metadataDao.id!]); } +} - private daoToMetadata(dao: RoleMetadataDao): RoleMetadata { - return { - source: dao.source, - }; - } +export function daoToMetadata(dao: RoleMetadataDao): RoleMetadata { + return { + source: dao.source, + description: dao.description, + }; +} - private metadataToDao( - roleMetadata: RoleMetadata, - roleEntityRef: string, - ): RoleMetadataDao { - return { - roleEntityRef, - source: roleMetadata.source, - }; - } +export function metadataToDao( + roleMetadata: RoleMetadataDao, + roleEntityRef: string, +): RoleMetadataDao { + return { + roleEntityRef, + source: roleMetadata.source, + description: roleMetadata.description, + }; } diff --git a/plugins/rbac-backend/src/file-permissions/csv.ts b/plugins/rbac-backend/src/file-permissions/csv.ts index 94e3d10ff3..325e81ab5b 100644 --- a/plugins/rbac-backend/src/file-permissions/csv.ts +++ b/plugins/rbac-backend/src/file-permissions/csv.ts @@ -4,7 +4,7 @@ import { Logger } from 'winston'; import { PolicyMetadataStorage } from '../database/policy-metadata-storage'; import { RoleMetadataStorage } from '../database/role-metadata'; -import { metadataStringToPolicy, transformArraytoPolicy } from '../helper'; +import { metadataStringToPolicy, transformArrayToPolicy } from '../helper'; import { EnforcerDelegate } from '../service/enforcer-delegate'; import { MODEL } from '../service/permission-model'; import { @@ -121,7 +121,7 @@ export const loadFilteredPoliciesFromCSV = async ( ); for (const policy of policies) { - const err = validatePolicy(transformArraytoPolicy(policy)); + const err = validatePolicy(transformArrayToPolicy(policy)); if (err) { logger.warn( `Failed to validate policy from file ${policyFile}. Cause: ${err.message}`, diff --git a/plugins/rbac-backend/src/helper.test.ts b/plugins/rbac-backend/src/helper.test.ts new file mode 100644 index 0000000000..ce28651b60 --- /dev/null +++ b/plugins/rbac-backend/src/helper.test.ts @@ -0,0 +1,250 @@ +import { RoleMetadataDao } from './database/role-metadata'; +import { + deepSortedEqual, + metadataStringToPolicy, + policiesToString, + policyToString, + removeTheDifference, + transformArrayToPolicy, +} from './helper'; +// Import the function to test +import { EnforcerDelegate } from './service/enforcer-delegate'; + +describe('helper.ts', () => { + describe('policyToString', () => { + it('should convert permission policy to string', () => { + const policy = [ + 'user:default/some-user', + 'catalog-entity', + 'read', + 'allow', + ]; + const expectedString = + '[user:default/some-user, catalog-entity, read, allow]'; + expect(policyToString(policy)).toEqual(expectedString); + }); + }); + describe('policiesToString', () => { + it('should convert one permission policy to string', () => { + const policies = [ + ['user:default/some-user', 'catalog-entity', 'read', 'allow'], + ]; + const expectedString = + '[[user:default/some-user, catalog-entity, read, allow]]'; + expect(policiesToString(policies)).toEqual(expectedString); + }); + + it('should convert empty permission policy array to string', () => { + const policies = [[]]; + const expectedString = '[[]]'; + expect(policiesToString(policies)).toEqual(expectedString); + }); + }); + + describe('metadataStringToPolicy', () => { + it('parses a permission policy string', () => { + const policy = '[user:default/some-user, catalog-entity, read, allow]'; + const expectedPolicy = [ + 'user:default/some-user', + 'catalog-entity', + 'read', + 'allow', + ]; + expect(metadataStringToPolicy(policy)).toEqual(expectedPolicy); + }); + + it('parses a grouping policy', () => { + const policy = '[user:default/some-user, role:default/dev]'; + const expectedPolicy = ['user:default/some-user', 'role:default/dev']; + expect(metadataStringToPolicy(policy)).toEqual(expectedPolicy); + }); + }); + + describe('removeTheDifference', () => { + const mockEnforcerDelegate: Partial = { + removeGroupingPolicy: jest.fn().mockImplementation(), + }; + + beforeEach(() => { + (mockEnforcerDelegate.removeGroupingPolicy as jest.Mock).mockClear(); + }); + + it('removes the difference between originalGroup and addedGroup', async () => { + const originalGroup = [ + 'user:default/some-user', + 'user:default/dev', + 'user:default/admin', + ]; + const addedGroup = ['user:default/some-user', 'user:default/dev']; + const source = 'rest'; + const roleName = 'role:default/admin'; + + await removeTheDifference( + originalGroup, + addedGroup, + source, + roleName, + mockEnforcerDelegate as EnforcerDelegate, + ); + + expect(mockEnforcerDelegate.removeGroupingPolicy).toHaveBeenCalledWith( + ['user:default/admin', roleName], + source, + true, + ); + }); + + it('does nothing when originalGroup and addedGroup are the same', async () => { + const originalGroup = ['user:default/some-user', 'user:default/dev']; + const addedGroup = ['user:default/some-user', 'user:default/dev']; + const source = 'rest'; + const roleName = 'role:default/admin'; + + await removeTheDifference( + originalGroup, + addedGroup, + source, + roleName, + mockEnforcerDelegate as EnforcerDelegate, + ); + + expect(mockEnforcerDelegate.removeGroupingPolicy).not.toHaveBeenCalled(); + }); + + it('does nothing when originalGroup is empty', async () => { + const originalGroup: string[] = []; + const addedGroup = ['user:default/some-user', 'role:default/dev']; + const source = 'rest'; + const roleName = 'admin'; + + await removeTheDifference( + originalGroup, + addedGroup, + source, + roleName, + mockEnforcerDelegate as EnforcerDelegate, + ); + + expect(mockEnforcerDelegate.removeGroupingPolicy).not.toHaveBeenCalled(); + }); + }); + + describe('transformArrayToPolicy', () => { + it('transforms array to RoleBasedPolicy object', () => { + const policyArray = [ + 'role:default/dev', + 'catalog-entity', + 'read', + 'allow', + ]; + const expectedPolicy = { + entityReference: 'role:default/dev', + permission: 'catalog-entity', + policy: 'read', + effect: 'allow', + }; + + const result = transformArrayToPolicy(policyArray); + + expect(result).toEqual(expectedPolicy); + }); + }); + + describe('deepSortedEqual', () => { + it('should return true for identical objects with nested properties in different order', () => { + const obj1: RoleMetadataDao = { + description: 'qa team', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + const obj2: RoleMetadataDao = { + roleEntityRef: 'role:default/qa', + description: 'qa team', + id: 1, + source: 'rest', + }; + expect(deepSortedEqual(obj1, obj2)).toBe(true); + }); + + it('should return true for identical objects with different ordering of top-level properties', () => { + const obj1: RoleMetadataDao = { + description: 'qa team', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + const obj2: RoleMetadataDao = { + id: 1, + description: 'qa team', + source: 'rest', + roleEntityRef: 'role:default/qa', + }; + expect(deepSortedEqual(obj1, obj2)).toBe(true); + }); + + it('should return false for objects with different values', () => { + const obj1: RoleMetadataDao = { + description: 'qa', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + const obj2: RoleMetadataDao = { + description: 'great qa', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + expect(deepSortedEqual(obj1, obj2)).toBe(false); + }); + + it('should return false for objects with different source', () => { + const obj1: RoleMetadataDao = { + description: 'qa teams', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + const obj2: RoleMetadataDao = { + description: 'qa teams', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'configuration', + }; + expect(deepSortedEqual(obj1, obj2)).toBe(false); + }); + + it('should return false for objects with different id', () => { + const obj1: RoleMetadataDao = { + description: 'qa teams', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + const obj2: RoleMetadataDao = { + description: 'qa teams', + id: 2, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + expect(deepSortedEqual(obj1, obj2)).toBe(false); + }); + + it('should return false for objects with different role entity reference', () => { + const obj1: RoleMetadataDao = { + description: 'qa teams', + id: 1, + roleEntityRef: 'role:default/qa', + source: 'rest', + }; + const obj2: RoleMetadataDao = { + description: 'qa teams', + id: 1, + roleEntityRef: 'role:default/dev', + source: 'rest', + }; + expect(deepSortedEqual(obj1, obj2)).toBe(false); + }); + }); +}); diff --git a/plugins/rbac-backend/src/helper.ts b/plugins/rbac-backend/src/helper.ts index 738d45a96e..2c4ff3a681 100644 --- a/plugins/rbac-backend/src/helper.ts +++ b/plugins/rbac-backend/src/helper.ts @@ -1,4 +1,4 @@ -import { difference } from 'lodash'; +import { difference, isEqual, sortBy, toPairs } from 'lodash'; import { RoleBasedPolicy, @@ -39,7 +39,17 @@ export async function removeTheDifference( } } -export function transformArraytoPolicy(policyArray: string[]): RoleBasedPolicy { +export function transformArrayToPolicy(policyArray: string[]): RoleBasedPolicy { const [entityReference, permission, policy, effect] = policyArray; return { entityReference, permission, policy, effect }; } + +export function deepSortedEqual( + obj1: Record, + obj2: Record, +): boolean { + const sortedObj1 = sortBy(toPairs(obj1), ([key]) => key); + const sortedObj2 = sortBy(toPairs(obj2), ([key]) => key); + + return isEqual(sortedObj1, sortedObj2); +} diff --git a/plugins/rbac-backend/src/service/enforcer-delegate.ts b/plugins/rbac-backend/src/service/enforcer-delegate.ts index 35a485473d..8a67854a77 100644 --- a/plugins/rbac-backend/src/service/enforcer-delegate.ts +++ b/plugins/rbac-backend/src/service/enforcer-delegate.ts @@ -12,7 +12,10 @@ import { PermissionPolicyMetadataDao, PolicyMetadataStorage, } from '../database/policy-metadata-storage'; -import { RoleMetadataStorage } from '../database/role-metadata'; +import { + RoleMetadataDao, + RoleMetadataStorage, +} from '../database/role-metadata'; import { policiesToString, policyToString } from '../helper'; export class EnforcerDelegate { @@ -84,7 +87,7 @@ export class EnforcerDelegate { async addPolicies( policies: string[][], source: Source, - externalTrx: Knex.Transaction, + externalTrx?: Knex.Transaction, ): Promise { const trx = externalTrx || (await this.knex.transaction()); try { @@ -122,14 +125,14 @@ export class EnforcerDelegate { const entityRef = policy[1]; let metadata; - if (entityRef.startsWith(`role:`)) { - metadata = await this.roleMetadataStorage.findRoleMetadata( - entityRef, - trx, - ); - } - try { + if (entityRef.startsWith(`role:`)) { + metadata = await this.roleMetadataStorage.findRoleMetadata( + entityRef, + trx, + ); + } + await this.policyMetadataStorage.createPolicyMetadata( source, policy, @@ -138,8 +141,7 @@ export class EnforcerDelegate { if (!metadata && !isUpdate) { await this.roleMetadataStorage.createRoleMetadata( - { source }, - entityRef, + { source, roleEntityRef: entityRef }, trx, ); } @@ -161,38 +163,25 @@ export class EnforcerDelegate { async addGroupingPolicies( policies: string[][], - source: Source, + roleMetadata: RoleMetadataDao, externalTrx?: Knex.Transaction, - isUpdate?: boolean, ): Promise { const trx = externalTrx ?? (await this.knex.transaction()); try { for (const policy of policies) { - const entityRef = policy[1]; - let metadata; - - if (entityRef.startsWith(`role:`)) { - metadata = await this.roleMetadataStorage.findRoleMetadata( - entityRef, - trx, - ); - } - await this.policyMetadataStorage.createPolicyMetadata( - source, + roleMetadata.source, policy, trx, ); + } - if (!metadata && !isUpdate) { - await this.roleMetadataStorage.createRoleMetadata( - { source }, - entityRef, - trx, - ); - } + const entityRef = roleMetadata.roleEntityRef; + if (!(await this.roleMetadataStorage.findRoleMetadata(entityRef, trx))) { + await this.roleMetadataStorage.createRoleMetadata(roleMetadata, trx); } + const ok = await this.enforcer.addGroupingPolicies(policies); if (!ok) { throw new Error( @@ -214,27 +203,27 @@ export class EnforcerDelegate { async updateGroupingPolicies( oldRole: string[][], newRole: string[][], - source: Source, + roleMetadata: RoleMetadataDao, allowToDeleteCSVFilePolicy?: boolean, externalTrx?: Knex.Transaction, ): Promise { const trx = externalTrx ?? (await this.knex.transaction()); - const newRoleName = newRole.at(0)?.at(1)!; const oldRoleName = oldRole.at(0)?.at(1)!; try { + // todo handle legacy... await this.roleMetadataStorage.updateRoleMetadata( - { source: source, roleEntityRef: newRoleName }, + roleMetadata, oldRoleName, trx, ); await this.removeGroupingPolicies( oldRole, - source, + roleMetadata.source, allowToDeleteCSVFilePolicy, true, trx, ); - await this.addGroupingPolicies(newRole, source, trx, true); + await this.addGroupingPolicies(newRole, roleMetadata, trx); if (!externalTrx) { await trx.commit(); } @@ -395,7 +384,11 @@ export class EnforcerDelegate { allowToDeleteCSVFilePolicy, ); if (!isUpdate) { - await this.roleMetadataStorage.removeRoleMetadata(roleEntity, trx); + if ( + await this.roleMetadataStorage.findRoleMetadata(roleEntity, trx) + ) { + await this.roleMetadataStorage.removeRoleMetadata(roleEntity, trx); + } } await this.policyMetadataStorage.removePolicyMetadata(policy, trx); } diff --git a/plugins/rbac-backend/src/service/enforcer.delegate.test.ts b/plugins/rbac-backend/src/service/enforcer.delegate.test.ts new file mode 100644 index 0000000000..de838099f0 --- /dev/null +++ b/plugins/rbac-backend/src/service/enforcer.delegate.test.ts @@ -0,0 +1,756 @@ +import { getVoidLogger } from '@backstage/backend-common'; +import { DatabaseService } from '@backstage/backend-plugin-api'; +import { ConfigReader } from '@backstage/config'; + +import { newEnforcer, newModelFromString } from 'casbin'; +import * as Knex from 'knex'; +import { MockClient } from 'knex-mock-client'; + +import { + PermissionPolicyMetadata, + Source, +} from '@janus-idp/backstage-plugin-rbac-common'; + +import { CasbinDBAdapterFactory } from '../database/casbin-adapter-factory'; +import { + PermissionPolicyMetadataDao, + PolicyMetadataStorage, +} from '../database/policy-metadata-storage'; +import { + RoleMetadataDao, + RoleMetadataStorage, +} from '../database/role-metadata'; +import { BackstageRoleManager } from '../role-manager/role-manager'; +import { EnforcerDelegate } from './enforcer-delegate'; +import { MODEL } from './permission-model'; + +const catalogApi = { + getEntityAncestors: jest.fn().mockImplementation(), + getLocationById: jest.fn().mockImplementation(), + getEntities: jest.fn().mockImplementation(), + getEntitiesByRefs: jest.fn().mockImplementation(), + queryEntities: jest.fn().mockImplementation(), + getEntityByRef: jest.fn().mockImplementation(), + refreshEntity: jest.fn().mockImplementation(), + getEntityFacets: jest.fn().mockImplementation(), + addLocation: jest.fn().mockImplementation(), + getLocationByRef: jest.fn().mockImplementation(), + removeLocationById: jest.fn().mockImplementation(), + removeEntityByUid: jest.fn().mockImplementation(), + validateEntity: jest.fn().mockImplementation(), +}; + +const roleMetadataStorageMock: RoleMetadataStorage = { + findRoleMetadata: jest.fn().mockImplementation(), + createRoleMetadata: jest.fn().mockImplementation(), + updateRoleMetadata: jest.fn().mockImplementation(), + removeRoleMetadata: jest.fn().mockImplementation(), +}; + +const policyMetadataStorageMock: PolicyMetadataStorage = { + findPolicyMetadataBySource: jest + .fn() + .mockImplementation( + async (_source: Source): Promise => { + return []; + }, + ), + findPolicyMetadata: jest.fn().mockImplementation(), + createPolicyMetadata: jest.fn().mockImplementation(), + removePolicyMetadata: jest.fn().mockImplementation(), +}; + +const tokenManagerMock = { + getToken: jest.fn().mockImplementation(async () => { + return Promise.resolve({ token: 'some-token' }); + }), + authenticate: jest.fn().mockImplementation(), +}; + +const dbManagerMock: DatabaseService = { + getClient: jest.fn().mockImplementation(), +}; + +const config = new ConfigReader({ + backend: { + database: { + client: 'better-sqlite3', + connection: ':memory:', + }, + }, + permission: { + rbac: {}, + }, +}); +const policy = ['user:default/tom', 'policy-entity', 'read', 'allow']; +const secondPolicy = ['user:default/tim', 'catalog-entity', 'write', 'allow']; + +const groupingPolicy = ['user:default/tom', 'role:default/dev-team']; +const secondGroupingPolicy = ['user:default/tim', 'role:default/qa-team']; + +describe('EnforcerDelegate', () => { + beforeEach(() => { + (policyMetadataStorageMock.createPolicyMetadata as jest.Mock).mockReset(); + (roleMetadataStorageMock.createRoleMetadata as jest.Mock).mockReset(); + (roleMetadataStorageMock.findRoleMetadata as jest.Mock).mockReset(); + }); + + async function createEnfDelegate( + policies?: string[][], + groupingPolicies?: string[][], + ): Promise { + const theModel = newModelFromString(MODEL); + const logger = getVoidLogger(); + + const sqliteInMemoryAdapter = await new CasbinDBAdapterFactory( + config, + dbManagerMock, + ).createAdapter(); + + const enf = await newEnforcer(theModel, sqliteInMemoryAdapter); + + const rm = new BackstageRoleManager(catalogApi, logger, tokenManagerMock); + enf.setRoleManager(rm); + enf.enableAutoBuildRoleLinks(false); + await enf.buildRoleLinks(); + + if (policies && policies.length > 0) { + await enf.addPolicies(policies); + } + if (groupingPolicies && groupingPolicies.length > 0) { + await enf.addGroupingPolicies(groupingPolicies); + } + + const knex = Knex.knex({ client: MockClient }); + + return new EnforcerDelegate( + enf, + policyMetadataStorageMock, + roleMetadataStorageMock, + knex, + ); + } + + describe('hasPolicy', () => { + it('has policy should return false', async () => { + const enfDelegate = await createEnfDelegate(); + const result = await enfDelegate.hasPolicy(...policy); + + expect(result).toBeFalsy(); + }); + + it('has policy should return true', async () => { + const enfDelegate = await createEnfDelegate([policy]); + + const result = await enfDelegate.hasPolicy(...policy); + + expect(result).toBeTruthy(); + }); + }); + + describe('hasGroupingPolicy', () => { + it('has policy should return false', async () => { + const enfDelegate = await createEnfDelegate([policy]); + const result = await enfDelegate.hasGroupingPolicy(...groupingPolicy); + + expect(result).toBeFalsy(); + }); + + it('has policy should return true', async () => { + const enfDelegate = await createEnfDelegate([], [groupingPolicy]); + + const result = await enfDelegate.hasGroupingPolicy(...groupingPolicy); + + expect(result).toBeTruthy(); + }); + }); + + describe('getPolicy', () => { + it('should return empty array', async () => { + const enfDelegate = await createEnfDelegate(); + const policies = await enfDelegate.getPolicy(); + + expect(policies.length).toEqual(0); + }); + + it('should return policy', async () => { + const enfDelegate = await createEnfDelegate([policy]); + + const policies = await enfDelegate.getPolicy(); + + expect(policies.length).toEqual(1); + expect(policies[0]).toEqual(policy); + }); + }); + + describe('getGroupingPolicy', () => { + it('should return empty array', async () => { + const enfDelegate = await createEnfDelegate(); + const groupingPolicies = await enfDelegate.getGroupingPolicy(); + + expect(groupingPolicies.length).toEqual(0); + }); + + it('should return grouping policy', async () => { + const enfDelegate = await createEnfDelegate([], [groupingPolicy]); + + const policies = await enfDelegate.getGroupingPolicy(); + + expect(policies.length).toEqual(1); + expect(policies[0]).toEqual(groupingPolicy); + }); + }); + + describe('getFilteredPolicy', () => { + it('should return empty array', async () => { + const enfDelegate = await createEnfDelegate(); + // filter by policy assignment person + const policies = await enfDelegate.getFilteredPolicy(0, policy[0]); + + expect(policies.length).toEqual(0); + }); + + it('should return filteredPolicy', async () => { + const enfDelegate = await createEnfDelegate([policy, secondPolicy]); + + // filter by policy assignment person + const policies = await enfDelegate.getFilteredPolicy( + 0, + 'user:default/tim', + ); + + expect(policies.length).toEqual(1); + expect(policies[0]).toEqual(secondPolicy); + }); + }); + + describe('getFilteredGroupingPolicy', () => { + it('should return empty array', async () => { + const enfDelegate = await createEnfDelegate(); + // filter by policy assignment person + const policies = await enfDelegate.getFilteredGroupingPolicy( + 0, + 'user:default/tim', + ); + + expect(policies.length).toEqual(0); + }); + + it('should return filteredPolicy', async () => { + const enfDelegate = await createEnfDelegate( + [], + [groupingPolicy, secondGroupingPolicy], + ); + + // filter by policy assignment person + const policies = await enfDelegate.getFilteredGroupingPolicy( + 0, + 'user:default/tim', + ); + + expect(policies.length).toEqual(1); + expect(policies[0]).toEqual(secondGroupingPolicy); + }); + }); + + describe('addPolicy', () => { + it('should add policy', async () => { + const enfDelegate = await createEnfDelegate(); + + await enfDelegate.addPolicy(policy, 'rest'); + + const storePolicies = await enfDelegate.getPolicy(); + + expect(storePolicies).toEqual([policy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', policy, expect.anything()); + }); + + it('should fail to add policy, caused policy metadata storage error', async () => { + const enfDelegate = await createEnfDelegate(); + + policyMetadataStorageMock.createPolicyMetadata = jest + .fn() + .mockImplementation(() => { + throw new Error('some unexpected error'); + }); + + await expect(enfDelegate.addPolicy(policy, 'rest')).rejects.toThrow( + 'some unexpected error', + ); + }); + }); + + describe('addPolicies', () => { + it('should be added single policy', async () => { + const enfDelegate = await createEnfDelegate(); + + await enfDelegate.addPolicies([policy], 'rest'); + + const storePolicies = await enfDelegate.getPolicy(); + + expect(storePolicies).toEqual([policy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenNthCalledWith(1, 'rest', policy, expect.anything()); + }); + + it('should be added few policies', async () => { + const enfDelegate = await createEnfDelegate(); + + await enfDelegate.addPolicies([policy, secondPolicy], 'rest'); + + const storePolicies = await enfDelegate.getPolicy(); + + expect(storePolicies.length).toEqual(2); + expect(storePolicies).toEqual( + expect.arrayContaining([ + expect.objectContaining(policy), + expect.objectContaining(secondPolicy), + ]), + ); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledTimes(2); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', policy, expect.anything()); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', secondPolicy, expect.anything()); + }); + + it('should fail to add policy, because policy metadata storage fails', async () => { + const enfDelegate = await createEnfDelegate(); + + policyMetadataStorageMock.createPolicyMetadata = jest + .fn() + .mockImplementation(() => { + throw new Error('some unexpected error'); + }); + + await expect( + enfDelegate.addPolicies([policy, secondPolicy], 'rest'), + ).rejects.toThrow('some unexpected error'); + }); + + it('should not fail, when argument is empty array', async () => { + const enfDelegate = await createEnfDelegate(); + + enfDelegate.addPolicies([], 'rest'); + + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).not.toHaveBeenCalled(); + expect((await enfDelegate.getPolicy()).length).toEqual(0); + }); + }); + + describe('addGroupingPolicy', () => { + it('should add grouping policy and create role metadata', async () => { + (roleMetadataStorageMock.findRoleMetadata as jest.Mock).mockReturnValue( + Promise.resolve(undefined), + ); + + const enfDelegate = await createEnfDelegate(); + + await enfDelegate.addGroupingPolicy(groupingPolicy, 'rest'); + + const storePolicies = await enfDelegate.getGroupingPolicy(); + + expect(storePolicies).toEqual([groupingPolicy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + expect(roleMetadataStorageMock.createRoleMetadata).toHaveBeenCalled(); + }); + + it('should add grouping policy, but do not create role metadata', async () => { + roleMetadataStorageMock.findRoleMetadata = jest + .fn() + .mockImplementation( + async ( + _roleEntityRef: string, + _trx: Knex.Knex.Transaction, + ): Promise => { + return { source: 'csv-file', roleEntityRef: 'user:default/tom' }; + }, + ); + + const enfDelegate = await createEnfDelegate(); + + await enfDelegate.addGroupingPolicy(groupingPolicy, 'rest'); + + const storePolicies = await enfDelegate.getGroupingPolicy(); + + expect(storePolicies).toEqual([groupingPolicy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + expect(roleMetadataStorageMock.createRoleMetadata).not.toHaveBeenCalled(); + }); + + it('should fail to add policy, caused policy metadata storage error', async () => { + const enfDelegate = await createEnfDelegate(); + + policyMetadataStorageMock.createPolicyMetadata = jest + .fn() + .mockImplementation(() => { + throw new Error('some unexpected error'); + }); + + await expect( + enfDelegate.addGroupingPolicy(groupingPolicy, 'rest'), + ).rejects.toThrow('some unexpected error'); + }); + + it('should fail to add policy, caused role metadata storage error', async () => { + const enfDelegate = await createEnfDelegate(); + + roleMetadataStorageMock.createRoleMetadata = jest + .fn() + .mockImplementation(() => { + throw new Error('some unexpected error'); + }); + + await expect( + enfDelegate.addGroupingPolicy(groupingPolicy, 'rest'), + ).rejects.toThrow('some unexpected error'); + }); + + it('should not create role metadata if isUpdate is true', async () => { + const enfDelegate = await createEnfDelegate(); + + await enfDelegate.addGroupingPolicy( + groupingPolicy, + 'rest', + undefined, + true, + ); + const storedPolicies = await enfDelegate.getGroupingPolicy(); + + expect(storedPolicies).toEqual([groupingPolicy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + + expect(roleMetadataStorageMock.createRoleMetadata).not.toHaveBeenCalled(); + }); + + it('should not create role metadata, because metadata has been created', async () => { + roleMetadataStorageMock.findRoleMetadata = jest + .fn() + .mockImplementation( + async ( + _roleEntityRef: string, + _trx: Knex.Knex.Transaction, + ): Promise => { + return { source: 'csv-file', roleEntityRef: 'user:default/tom' }; + }, + ); + + const enfDelegate = await createEnfDelegate(); + + await enfDelegate.addGroupingPolicy(groupingPolicy, 'rest', undefined); + const storedPolicies = await enfDelegate.getGroupingPolicy(); + + expect(storedPolicies).toEqual([groupingPolicy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + + expect(roleMetadataStorageMock.createRoleMetadata).not.toHaveBeenCalled(); + }); + }); + + describe('addGroupingPolicies', () => { + it('should add grouping policies and create role metadata', async () => { + const enfDelegate = await createEnfDelegate(); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: 'role:default/security', + source: 'rest', + }; + await enfDelegate.addGroupingPolicies( + [groupingPolicy, secondGroupingPolicy], + roleMetadataDao, + ); + + const storedPolicies = await enfDelegate.getGroupingPolicy(); + expect(storedPolicies).toEqual([groupingPolicy, secondGroupingPolicy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', secondGroupingPolicy, expect.anything()); + + expect(roleMetadataStorageMock.createRoleMetadata).toHaveBeenCalledWith( + roleMetadataDao, + expect.anything(), + ); + }); + + it('should add grouping policies and create role metadata with description', async () => { + const enfDelegate = await createEnfDelegate(); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: 'role:default/security', + source: 'rest', + description: 'Role for security engineers', + }; + await enfDelegate.addGroupingPolicies( + [groupingPolicy, secondGroupingPolicy], + roleMetadataDao, + ); + + const storedPolicies = await enfDelegate.getGroupingPolicy(); + expect(storedPolicies).toEqual([groupingPolicy, secondGroupingPolicy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', secondGroupingPolicy, expect.anything()); + + expect(roleMetadataStorageMock.createRoleMetadata).toHaveBeenCalledWith( + roleMetadataDao, + expect.anything(), + ); + }); + + it('should fail to add grouping policy, because fail to create role metadata', async () => { + roleMetadataStorageMock.createRoleMetadata = jest + .fn() + .mockImplementation(() => { + throw new Error('some unexpected error'); + }); + + const enfDelegate = await createEnfDelegate(); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: 'role:default/security', + source: 'rest', + }; + await expect( + enfDelegate.addGroupingPolicies( + [groupingPolicy, secondGroupingPolicy], + roleMetadataDao, + ), + ).rejects.toThrow('some unexpected error'); + + // shouldn't store group policies + const storedPolicies = await enfDelegate.getGroupingPolicy(); + expect(storedPolicies).toEqual([]); + }); + + it('should not create role metadata, because metadata has been created', async () => { + roleMetadataStorageMock.findRoleMetadata = jest + .fn() + .mockImplementation( + async ( + _roleEntityRef: string, + _trx: Knex.Knex.Transaction, + ): Promise => { + return { source: 'csv-file', roleEntityRef: 'user:default/tom' }; + }, + ); + + const enfDelegate = await createEnfDelegate(); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: 'role:default/security', + source: 'rest', + }; + await enfDelegate.addGroupingPolicies( + [groupingPolicy, secondGroupingPolicy], + roleMetadataDao, + ); + const storedPolicies = await enfDelegate.getGroupingPolicy(); + + expect(storedPolicies).toEqual([groupingPolicy, secondGroupingPolicy]); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', secondGroupingPolicy, expect.anything()); + + expect(roleMetadataStorageMock.createRoleMetadata).not.toHaveBeenCalled(); + }); + }); + + describe('updateGroupingPolicies', () => { + it('should update grouping policies: add one more policy', async () => { + roleMetadataStorageMock.findRoleMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'csv-file', roleEntityRef: 'user:default/tom' }; + }); + policyMetadataStorageMock.findPolicyMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'rest' }; + }); + + const enfDelegate = await createEnfDelegate([], [groupingPolicy]); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: 'role:default/dev-team', + source: 'rest', + }; + await enfDelegate.updateGroupingPolicies( + [groupingPolicy], + [groupingPolicy, secondGroupingPolicy], + roleMetadataDao, + ); + + const storedPolicies = await enfDelegate.getGroupingPolicy(); + expect(storedPolicies.length).toEqual(2); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', secondGroupingPolicy, expect.anything()); + expect(roleMetadataStorageMock.updateRoleMetadata).toHaveBeenCalledWith( + roleMetadataDao, + roleMetadataDao.roleEntityRef, + expect.anything(), + ); + }); + + it('should update grouping policies: one policy should be removed', async () => { + roleMetadataStorageMock.findRoleMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'csv-file', roleEntityRef: 'user:default/tom' }; + }); + policyMetadataStorageMock.findPolicyMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'rest' }; + }); + + const enfDelegate = await createEnfDelegate( + [], + [groupingPolicy, secondGroupingPolicy], + ); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: 'role:default/dev-team', + source: 'rest', + }; + await enfDelegate.updateGroupingPolicies( + [groupingPolicy, secondGroupingPolicy], + [groupingPolicy], + roleMetadataDao, + ); + + const storedPolicies = await enfDelegate.getGroupingPolicy(); + expect(storedPolicies.length).toEqual(1); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith('rest', groupingPolicy, expect.anything()); + expect(roleMetadataStorageMock.updateRoleMetadata).toHaveBeenCalledWith( + roleMetadataDao, + roleMetadataDao.roleEntityRef, + expect.anything(), + ); + }); + + it('should update grouping policies: role should be renamed', async () => { + roleMetadataStorageMock.findRoleMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'csv-file', roleEntityRef: 'user:default/tom' }; + }); + policyMetadataStorageMock.findPolicyMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'rest' }; + }); + + const oldRoleName = 'role:default/dev-team'; + const newRoleName = 'role:default/new-team-name'; + const groupingPolicyWithRenamedRole = [groupingPolicy[0], newRoleName]; + const secondGroupingPolicyWithRenamedRole = [ + secondGroupingPolicy[1], + newRoleName, + ]; + + const enfDelegate = await createEnfDelegate( + [], + [groupingPolicy, secondGroupingPolicy], + ); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: newRoleName, + source: 'rest', + }; + await enfDelegate.updateGroupingPolicies( + [groupingPolicy, secondGroupingPolicy], + [groupingPolicyWithRenamedRole, secondGroupingPolicyWithRenamedRole], + roleMetadataDao, + ); + + const storedPolicies = await enfDelegate.getGroupingPolicy(); + expect(storedPolicies.length).toEqual(2); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith( + 'rest', + groupingPolicyWithRenamedRole, + expect.anything(), + ); + expect( + policyMetadataStorageMock.createPolicyMetadata, + ).toHaveBeenCalledWith( + 'rest', + secondGroupingPolicyWithRenamedRole, + expect.anything(), + ); + expect(roleMetadataStorageMock.updateRoleMetadata).toHaveBeenCalledWith( + roleMetadataDao, + oldRoleName, + expect.anything(), + ); + }); + + it('should update grouping policies: should be update role description', async () => { + roleMetadataStorageMock.findRoleMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'csv-file', roleEntityRef: 'user:default/tom' }; + }); + policyMetadataStorageMock.findPolicyMetadata = jest + .fn() + .mockImplementation(async (): Promise => { + return { source: 'rest' }; + }); + + const enfDelegate = await createEnfDelegate([], [groupingPolicy]); + + const roleMetadataDao: RoleMetadataDao = { + roleEntityRef: 'role:default/dev-team', + source: 'rest', + description: 'some-description', + }; + await enfDelegate.updateGroupingPolicies( + [groupingPolicy], + [groupingPolicy], + roleMetadataDao, + ); + + const storedPolicies = await enfDelegate.getGroupingPolicy(); + expect(storedPolicies.length).toEqual(1); + expect(storedPolicies).toEqual([groupingPolicy]); + expect(roleMetadataStorageMock.updateRoleMetadata).toHaveBeenCalledWith( + roleMetadataDao, + roleMetadataDao.roleEntityRef, + expect.anything(), + ); + }); + }); +}); diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index 642345c248..785b5aa515 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -51,15 +51,13 @@ const useAdminsFromConfig = async ( try { if (!adminRoleMeta) { await roleMetadataStorage.createRoleMetadata( - { source: 'configuration' }, - adminRoleName, + { source: 'configuration', roleEntityRef: adminRoleName }, trx, ); } else if (adminRoleMeta.source === 'legacy') { await roleMetadataStorage.removeRoleMetadata(adminRoleName, trx); await roleMetadataStorage.createRoleMetadata( - { source: 'configuration' }, - adminRoleName, + { source: 'configuration', roleEntityRef: adminRoleName }, trx, ); } 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 c21718b0e6..b153a9e3dc 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.test.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.test.ts @@ -19,12 +19,14 @@ import { policyEntityReadPermission, policyEntityUpdatePermission, Role, - RoleMetadata, Source, } from '@janus-idp/backstage-plugin-rbac-common'; import { PolicyMetadataStorage } from '../database/policy-metadata-storage'; -import { RoleMetadataStorage } from '../database/role-metadata'; +import { + RoleMetadataDao, + RoleMetadataStorage, +} from '../database/role-metadata'; import { EnforcerDelegate } from './enforcer-delegate'; import { RBACPermissionPolicy } from './permission-policy'; import { @@ -124,8 +126,6 @@ const mockEnforcer: Partial = { updatePolicies: jest.fn().mockImplementation(), - addOrUpdateGroupingPolicy: jest.fn().mockImplementation(), - updateGroupingPolicies: jest.fn().mockImplementation(), }; @@ -133,8 +133,8 @@ const roleMetadataStorageMock: RoleMetadataStorage = { findRoleMetadata: jest .fn() .mockImplementation( - async (_roleEntityRef: string): Promise => { - return { source: 'rest' }; + async (roleEntityRef: string): Promise => { + return { source: 'rest', roleEntityRef: roleEntityRef }; }, ), createRoleMetadata: jest.fn().mockImplementation(), @@ -207,8 +207,8 @@ describe('REST policies api', () => { roleMetadataStorageMock.findRoleMetadata = jest .fn() .mockImplementation( - async (_roleEntityRef: string): Promise => { - return { source: 'rest' }; + async (roleEntityRef: string): Promise => { + return { source: 'rest', roleEntityRef: roleEntityRef }; }, ); @@ -456,7 +456,7 @@ describe('REST policies api', () => { }); it('should not be created permission policy caused some unexpected error', async () => { - mockEnforcer.addOrUpdatePolicy = jest + mockEnforcer.addPolicies = jest .fn() .mockImplementation(async (): Promise => { throw new Error(`Failed to add policies`); @@ -1644,6 +1644,36 @@ describe('REST policies api', () => { }); expect(result.statusCode).toBe(201); + expect(mockEnforcer.addGroupingPolicies).toHaveBeenCalledWith( + [['user:default/permission_admin', 'role:default/rbac_admin']], + { + roleEntityRef: 'role:default/rbac_admin', + source: 'rest', + description: '', + }, + ); + }); + + it('should be created role with description', async () => { + const result = await request(app) + .post('/roles') + .send({ + memberReferences: ['user:default/permission_admin'], + name: 'role:default/rbac_admin', + metadata: { + description: 'some test description', + }, + }); + + expect(result.statusCode).toBe(201); + expect(mockEnforcer.addGroupingPolicies).toHaveBeenCalledWith( + [['user:default/permission_admin', 'role:default/rbac_admin']], + { + roleEntityRef: 'role:default/rbac_admin', + source: 'rest', + description: 'some test description', + }, + ); }); it('should not be created role, because it is has been already present', async () => { @@ -1664,7 +1694,7 @@ describe('REST policies api', () => { }); it('should not be created role caused some unexpected error', async () => { - mockEnforcer.addOrUpdateGroupingPolicy = jest + mockEnforcer.addGroupingPolicies = jest .fn() .mockImplementation(async (): Promise => { throw new Error('Fail to create new policy'); @@ -1846,6 +1876,135 @@ describe('REST policies api', () => { expect(result.statusCode).toEqual(204); }); + it('should nothing to update, because role and metadata are the same', 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'], + metadata: { + source: 'rest', + }, + }, + newRole: { + memberReferences: ['user:default/permission_admin'], + name: 'role:default/rbac_admin', + metadata: { + source: 'rest', + }, + }, + }); + + expect(result.statusCode).toEqual(204); + }); + + it('should nothing to update, because role and metadata are the same, but old role metadata was not send', 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', + metadata: { + source: 'rest', + }, + }, + }); + + expect(result.statusCode).toEqual(204); + }); + + it('should update description', 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', + metadata: { + source: 'rest', + description: 'some admin role.', + }, + }, + }); + + expect(result.statusCode).toEqual(200); + expect(mockEnforcer.updateGroupingPolicies).toHaveBeenCalledWith( + [['user:default/permission_admin', 'role:default/rbac_admin']], + [['user:default/permission_admin', 'role:default/rbac_admin']], + { + description: 'some admin role.', + roleEntityRef: 'role:default/rbac_admin', + source: 'rest', + }, + false, + ); + }); + + it('should update role and role description', async () => { + mockEnforcer.hasGroupingPolicy = jest + .fn() + .mockImplementation(async (...param: string[]): Promise => { + if (param[0] === 'user:default/permission_admin') { + return true; + } + 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', 'user:default/dev'], + name: 'role:default/rbac_admin', + metadata: { + source: 'rest', + description: 'some admin role.', + }, + }, + }); + + expect(result.statusCode).toEqual(200); + + expect(mockEnforcer.updateGroupingPolicies).toHaveBeenCalledWith( + [['user:default/permission_admin', 'role:default/rbac_admin']], + [ + ['user:default/test', 'role:default/rbac_admin'], + ['user:default/dev', 'role:default/rbac_admin'], + ], + { + description: 'some admin role.', + roleEntityRef: 'role:default/rbac_admin', + source: 'rest', + }, + false, + ); + }); + it('should fail to update policy - role metadata could not be found', async () => { mockEnforcer.hasGroupingPolicy = jest .fn() @@ -2307,6 +2466,7 @@ describe('REST policies api', () => { memberReferences: ['group:default/test', 'user:default/test'], name: 'role:default/test', metadata: { + description: undefined, source: 'rest', }, }, diff --git a/plugins/rbac-backend/src/service/policies-rest-api.ts b/plugins/rbac-backend/src/service/policies-rest-api.ts index e53f3d44c5..aac8d2e25f 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.ts @@ -42,8 +42,12 @@ import { import { PluginIdProvider } from '@janus-idp/backstage-plugin-rbac-node'; import { ConditionalStorage } from '../database/conditional-storage'; -import { RoleMetadataStorage } from '../database/role-metadata'; -import { policyToString } from '../helper'; +import { + daoToMetadata, + RoleMetadataDao, + RoleMetadataStorage, +} from '../database/role-metadata'; +import { deepSortedEqual, policyToString } from '../helper'; import { EnforcerDelegate } from './enforcer-delegate'; import { PluginPermissionMetadataCollector } from './plugin-endpoints'; import { @@ -208,9 +212,7 @@ export class PolicesServer { const processedPolicies = await this.processPolicies(policyRaw); - for (const policy of processedPolicies) { - await this.enforcer.addOrUpdatePolicy(policy, 'rest', false); - } + await this.enforcer.addPolicies(processedPolicies, 'rest'); response.status(201).end(); }); @@ -364,9 +366,12 @@ export class PolicesServer { } } - for (const role of roles) { - await this.enforcer.addOrUpdateGroupingPolicy(role, 'rest', false); - } + const metadata: RoleMetadataDao = { + roleEntityRef: roleRaw.name, + source: 'rest', + description: roleRaw.metadata?.description ?? '', + }; + await this.enforcer.addGroupingPolicies(roles, metadata); response.status(201).end(); }); @@ -410,6 +415,37 @@ export class PolicesServer { const newRole = this.transformRoleToArray(newRoleRaw); // todo shell we allow newRole with an empty array?... + const newMetadata: RoleMetadataDao = { + ...newRoleRaw.metadata, + source: newRoleRaw.metadata?.source ?? 'rest', + roleEntityRef: newRoleRaw.name, + }; + + const oldMetadata = + await this.roleMetadata.findRoleMetadata(roleEntityRef); + if (!oldMetadata) { + throw new NotFoundError(`Unable to find metadata for ${roleEntityRef}`); + } + if (oldMetadata.source === 'csv-file') { + throw new Error( + `Role ${roleEntityRef} can be modified only using csv policy file.`, + ); + } + if (oldMetadata.source === 'configuration') { + throw new Error( + `Role ${roleEntityRef} can be modified only using application config`, + ); + } + + if ( + isEqual(oldRole, newRole) && + deepSortedEqual(oldMetadata, newMetadata) + ) { + // no content: old role and new role are equal and their metadata too + response.status(204).end(); + return; + } + for (const role of newRole) { const hasRole = oldRole.some(element => { return isEqual(element, role); @@ -417,10 +453,6 @@ export class PolicesServer { // 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 } @@ -458,25 +490,10 @@ export class PolicesServer { } } - const metadata = await this.roleMetadata.findRoleMetadata(roleEntityRef); - if (!metadata) { - throw new NotFoundError(`Unable to find metadata for ${roleEntityRef}`); - } - if (metadata.source === 'csv-file') { - throw new Error( - `Role ${roleEntityRef} can be modified only using csv policy file.`, - ); - } - if (metadata.source === 'configuration') { - throw new Error( - `Role ${roleEntityRef} can be modified only using application config`, - ); - } - await this.enforcer.updateGroupingPolicies( oldRole, newRole, - 'rest', + newMetadata, false, ); @@ -704,7 +721,8 @@ export class PolicesServer { const result: Role[] = await Promise.all( Object.entries(combinedRoles).map(async ([role, value]) => { - const metadata = await this.roleMetadata.findRoleMetadata(role); + const metadataDao = await this.roleMetadata.findRoleMetadata(role); + const metadata = metadataDao ? daoToMetadata(metadataDao) : undefined; return Promise.resolve({ memberReferences: value, name: role, diff --git a/plugins/rbac-common/src/types.ts b/plugins/rbac-common/src/types.ts index 6565623865..b58d51fdd6 100644 --- a/plugins/rbac-common/src/types.ts +++ b/plugins/rbac-common/src/types.ts @@ -9,7 +9,11 @@ export type PermissionPolicyMetadata = { }; export type RoleMetadata = { - source: Source; + description?: string; + source?: Source; + // not implemented yet + modifiedBy?: string; + lastModified?: string; }; export type Policy = {