From 2612a3d41c56b39bcc2af24b5024d99e66876907 Mon Sep 17 00:00:00 2001 From: Patrick Knight Date: Tue, 9 Jan 2024 13:30:06 -0500 Subject: [PATCH] fix(rbac): handle preexisting policies by adding a legacy source --- .../src/service/enforcer-delegate.ts | 54 +++++++++++++------ .../src/service/permission-policy.ts | 23 +++++++- .../src/service/policy-builder.ts | 2 + plugins/rbac-common/src/types.ts | 3 +- 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/plugins/rbac-backend/src/service/enforcer-delegate.ts b/plugins/rbac-backend/src/service/enforcer-delegate.ts index a17c7f6376..e415f179b5 100644 --- a/plugins/rbac-backend/src/service/enforcer-delegate.ts +++ b/plugins/rbac-backend/src/service/enforcer-delegate.ts @@ -64,9 +64,9 @@ export class EnforcerDelegate { if (!ok) { throw new Error(`failed to create policy ${policyToString(policy)}`); } - addMetadataTrx.commit(); + await addMetadataTrx.commit(); } catch (err) { - addMetadataTrx.rollback(); + await addMetadataTrx.rollback(); throw err; } } @@ -87,9 +87,9 @@ export class EnforcerDelegate { `Failed to store policies ${policiesToString(policies)}`, ); } - addMetadataTrx.commit(); + await addMetadataTrx.commit(); } catch (err) { - addMetadataTrx.rollback(); + await addMetadataTrx.rollback(); throw err; } } @@ -107,9 +107,9 @@ export class EnforcerDelegate { if (!ok) { throw new Error(`failed to create policy ${policyToString(policy)}`); } - addMetadataTrx.commit(); + await addMetadataTrx.commit(); } catch (err) { - addMetadataTrx.rollback(); + await addMetadataTrx.rollback(); throw err; } } @@ -134,9 +134,9 @@ export class EnforcerDelegate { `Failed to store policies ${policiesToString(policies)}`, ); } - addMetadataTrx.commit(); + await addMetadataTrx.commit(); } catch (err) { - addMetadataTrx.rollback(); + await addMetadataTrx.rollback(); throw err; } } @@ -152,9 +152,9 @@ export class EnforcerDelegate { if (!ok) { throw new Error(`fail to delete policy ${policy}`); } - rmMetadataTrx.commit(); + await rmMetadataTrx.commit(); } catch (err) { - rmMetadataTrx.rollback(err); + await rmMetadataTrx.rollback(err); throw err; } } @@ -176,9 +176,9 @@ export class EnforcerDelegate { `Failed to delete policies ${policiesToString(policies)}`, ); } - rmMetadataTrx.commit(); + await rmMetadataTrx.commit(); } catch (err) { - rmMetadataTrx.rollback(err); + await rmMetadataTrx.rollback(err); throw err; } } @@ -196,9 +196,9 @@ export class EnforcerDelegate { if (!ok) { throw new Error(`Failed to delete policy ${policyToString(policy)}`); } - rmMetadataTrx.commit(); + await rmMetadataTrx.commit(); } catch (err) { - rmMetadataTrx.rollback(err); + await rmMetadataTrx.rollback(err); throw err; } } @@ -219,9 +219,9 @@ export class EnforcerDelegate { `Failed to delete grouping policies: ${policiesToString(policies)}`, ); } - rmMetadataTrx.commit(); + await rmMetadataTrx.commit(); } catch (err) { - rmMetadataTrx.rollback(err); + await rmMetadataTrx.rollback(err); throw err; } } @@ -265,4 +265,26 @@ export class EnforcerDelegate { ): Promise { return await this.metadataStorage.findPolicyMetadataBySource(source); } + + async getPoliciesWithoutSource(): Promise { + const policiesWithoutSource: string[][] = []; + const allPolicies = await this.enforcer.getPolicy(); + for (const policy of allPolicies) { + const sourcePolicy = + await this.metadataStorage.findPolicyMetadata(policy); + if (!sourcePolicy) { + policiesWithoutSource.push(policy); + } + } + return policiesWithoutSource; + } + + async migratePreexistingPolicies(enforcer: Enforcer): Promise { + const policies = await this.getPoliciesWithoutSource(); + + for (const policy of policies) { + await enforcer.removePolicy(...policy); + await this.addPolicy(policy, 'legacy'); + } + } } diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index 785094b76d..52fcf96436 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -42,6 +42,15 @@ async function addRoleMetadata( } } +const legacyPolicies = async (enf: EnforcerDelegate, policy: string[]) => { + if ( + (await enf.hasPolicy(...policy)) && + (await enf.getMetadata(policy)).source === 'legacy' + ) { + await enf.removePolicy(policy); + } +}; + const useAdmins = async ( admins: Config[], enf: EnforcerDelegate, @@ -55,7 +64,7 @@ const useAdmins = async ( if (!adminRoleMeta) { const trx = await knex.transaction(); await roleMetadataStorage.createRoleMetadata( - { source: 'default' }, + { source: 'configuration' }, adminRoleName, trx, ); @@ -71,15 +80,20 @@ const useAdmins = async ( }); const adminReadPermission = [adminRoleName, 'policy-entity', 'read', 'allow']; + + await legacyPolicies(enf, adminReadPermission); if (!(await enf.hasPolicy(...adminReadPermission))) { await enf.addPolicy(adminReadPermission, 'configuration'); } + const adminCreatePermission = [ adminRoleName, 'policy-entity', 'create', 'allow', ]; + + await legacyPolicies(enf, adminCreatePermission); if (!(await enf.hasPolicy(...adminCreatePermission))) { await enf.addPolicy(adminCreatePermission, 'configuration'); } @@ -90,6 +104,8 @@ const useAdmins = async ( 'delete', 'allow', ]; + + await legacyPolicies(enf, adminDeletePermission); if (!(await enf.hasPolicy(...adminDeletePermission))) { await enf.addPolicy(adminDeletePermission, 'configuration'); } @@ -100,6 +116,8 @@ const useAdmins = async ( 'update', 'allow', ]; + + await legacyPolicies(enf, adminUpdatePermission); if (!(await enf.hasPolicy(...adminUpdatePermission))) { await enf.addPolicy(adminUpdatePermission, 'configuration'); } @@ -166,6 +184,9 @@ const addPredefinedPoliciesAndGroupPolicies = async ( `Failed to validate policy from file ${preDefinedPoliciesFile}. Cause: ${err.message}`, ); } + + await legacyPolicies(enf, policy); + if (!(await enf.hasPolicy(...policy))) { await enf.addPolicy(policy, 'csv-file'); } diff --git a/plugins/rbac-backend/src/service/policy-builder.ts b/plugins/rbac-backend/src/service/policy-builder.ts index 9023dba66c..3874a7384b 100644 --- a/plugins/rbac-backend/src/service/policy-builder.ts +++ b/plugins/rbac-backend/src/service/policy-builder.ts @@ -92,6 +92,8 @@ export class PolicyBuilder { knex, ); + await enforcerDelegate.migratePreexistingPolicies(enf); + const options: RouterOptions = { config: env.config, logger: env.logger, diff --git a/plugins/rbac-common/src/types.ts b/plugins/rbac-common/src/types.ts index 07d92ae420..d8c5c73c11 100644 --- a/plugins/rbac-common/src/types.ts +++ b/plugins/rbac-common/src/types.ts @@ -1,7 +1,8 @@ export type Source = | 'rest' // created via REST API | 'csv-file' // created via policies-csv-file with defined path in the application configuration - | 'configuration'; // created from application configuration + | 'configuration' // created from application configuration + | 'legacy'; // preexisting policies export type RoleSource = Source | 'default'; // hard coded in the plugin code