diff --git a/plugins/rbac-backend/src/service/enforcer-delegate.ts b/plugins/rbac-backend/src/service/enforcer-delegate.ts index e415f179b5..c495391c00 100644 --- a/plugins/rbac-backend/src/service/enforcer-delegate.ts +++ b/plugins/rbac-backend/src/service/enforcer-delegate.ts @@ -12,12 +12,14 @@ import { PermissionPolicyMetadataDao, PolicyMetadataStorage, } from '../database/policy-metadata-storage'; +import { RoleMetadataStorage } from '../database/role-metadata'; import { policiesToString, policyToString } from '../helper'; export class EnforcerDelegate { constructor( private readonly enforcer: Enforcer, private readonly metadataStorage: PolicyMetadataStorage, + private readonly roleMetadataStorage: RoleMetadataStorage, private readonly knex: Knex, ) {} @@ -98,9 +100,9 @@ export class EnforcerDelegate { const addMetadataTrx = await this.knex.transaction(); try { - await this.metadataStorage.createPolicyMetadata( - source, - policy, + await this.roleMetadataStorage.createRoleMetadata( + { source: source }, + policy.at(1)!, addMetadataTrx, ); const ok = await this.enforcer.addGroupingPolicy(...policy); @@ -122,9 +124,9 @@ export class EnforcerDelegate { try { for (const policy of policies) { - await this.metadataStorage.createPolicyMetadata( - source, - policy, + await this.roleMetadataStorage.createRoleMetadata( + { source: source }, + policy.at(1)!, addMetadataTrx, ); } @@ -191,7 +193,10 @@ export class EnforcerDelegate { try { await this.checkIfPolicyModifiable(policy, allowToDeleCSVFilePolicy); - await this.metadataStorage.removePolicyMetadata(policy, rmMetadataTrx); + await this.roleMetadataStorage.removeRoleMetadata( + policy.at(1)!, + rmMetadataTrx, + ); const ok = await this.enforcer.removeGroupingPolicy(...policy); if (!ok) { throw new Error(`Failed to delete policy ${policyToString(policy)}`); @@ -211,7 +216,10 @@ export class EnforcerDelegate { try { for (const policy of policies) { await this.checkIfPolicyModifiable(policy, allowToDeleCSVFilePolicy); - await this.metadataStorage.removePolicyMetadata(policy, rmMetadataTrx); + await this.roleMetadataStorage.removeRoleMetadata( + policy.at(1)!, + rmMetadataTrx, + ); } const ok = await this.enforcer.removeGroupingPolicies(policies); if (!ok) { @@ -279,12 +287,32 @@ export class EnforcerDelegate { return policiesWithoutSource; } + async getGroupPoliciesWithoutSource(): Promise { + const policiesWithoutSource: string[][] = []; + const allPolicies = await this.enforcer.getGroupingPolicy(); + for (const policy of allPolicies) { + const sourcePolicy = await this.roleMetadataStorage.findRoleMetadata( + policy.at(1)!, + ); + if (!sourcePolicy) { + policiesWithoutSource.push(policy); + } + } + return policiesWithoutSource; + } + async migratePreexistingPolicies(enforcer: Enforcer): Promise { const policies = await this.getPoliciesWithoutSource(); + const groupPolicies = await this.getGroupPoliciesWithoutSource(); for (const policy of policies) { await enforcer.removePolicy(...policy); await this.addPolicy(policy, 'legacy'); } + + for (const policy of groupPolicies) { + await enforcer.removeGroupingPolicy(...policy); + await this.addGroupingPolicy(policy, 'legacy'); + } } } diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index 52fcf96436..ca0fa1ecdf 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -18,7 +18,7 @@ import { FileAdapter, newEnforcer, newModelFromString } from 'casbin'; import { Knex } from 'knex'; import { Logger } from 'winston'; -import { RoleSource, Source } from '@janus-idp/backstage-plugin-rbac-common'; +import { Source } from '@janus-idp/backstage-plugin-rbac-common'; import { ConditionalStorage } from '../database/conditional-storage'; import { RoleMetadataStorage } from '../database/role-metadata'; @@ -27,21 +27,6 @@ import { EnforcerDelegate } from './enforcer-delegate'; import { MODEL } from './permission-model'; import { validateEntityReference } from './policies-validation'; -async function addRoleMetadata( - groupPolicy: string[], - source: RoleSource, - roleMetadataStorage: RoleMetadataStorage, - trx: Knex.Transaction, -) { - const entityRef = groupPolicy[1]; - if (entityRef.startsWith(`role:`)) { - const metadata = await roleMetadataStorage.findRoleMetadata(entityRef); - if (!metadata) { - await roleMetadataStorage.createRoleMetadata({ source }, entityRef, trx); - } - } -} - const legacyPolicies = async (enf: EnforcerDelegate, policy: string[]) => { if ( (await enf.hasPolicy(...policy)) && @@ -165,16 +150,10 @@ const addPredefinedPoliciesAndGroupPolicies = async ( } const delRoleMetaTrx = await knex.transaction(); - try { - for (const roleMeta of rolesToDelete) { - await roleMetadataStorage.removeRoleMetadata(roleMeta, delRoleMetaTrx); - } - await enf.removeGroupingPolicies(groupPoliciesToDelete, true); - delRoleMetaTrx.commit(); - } catch (err) { - delRoleMetaTrx.rollback(); - throw err; + for (const roleMeta of rolesToDelete) { + await roleMetadataStorage.removeRoleMetadata(roleMeta, delRoleMetaTrx); } + await enf.removeGroupingPolicies(groupPoliciesToDelete, true); await enf.removePolicies(policiesToDelete, true); for (const policy of policies) { @@ -200,20 +179,7 @@ const addPredefinedPoliciesAndGroupPolicies = async ( roleMetadataStorage, `csv-file`, ); - const trx = await knex.transaction(); - try { - await addRoleMetadata( - groupPolicy, - 'csv-file', - roleMetadataStorage, - trx, - ); - await enf.addGroupingPolicy(groupPolicy, 'csv-file'); - trx.commit(); - } catch (err) { - trx.rollback(); - throw err; - } + enf.addGroupingPolicy(groupPolicy, 'csv-file'); } } }; diff --git a/plugins/rbac-backend/src/service/policy-builder.ts b/plugins/rbac-backend/src/service/policy-builder.ts index 3874a7384b..21bcc6fcde 100644 --- a/plugins/rbac-backend/src/service/policy-builder.ts +++ b/plugins/rbac-backend/src/service/policy-builder.ts @@ -89,6 +89,7 @@ export class PolicyBuilder { const enforcerDelegate = new EnforcerDelegate( enf, policyMetadataStorage, + roleMetadataStorage, knex, );