fix(rbac): handle preexisting grouping policies

cloud-eda · Jan 9, 2024 · aa82ecb · aa82ecb
1 parent 2612a3d
commit aa82ecb
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 47 deletions.
diff --git 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<string[][]> {
+    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<void> {
     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
@@ -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
@@ -89,6 +89,7 @@ export class PolicyBuilder {
     const enforcerDelegate = new EnforcerDelegate(
       enf,
       policyMetadataStorage,
+      roleMetadataStorage,
       knex,
     );
-Original file line number
+Diff line change
@@ Expand Up / @@ -89,6 +89,7 @@ export class PolicyBuilder { @@
         const enforcerDelegate = new EnforcerDelegate(
           enf,
           policyMetadataStorage,
+          roleMetadataStorage,
           knex,
         );
@@ Expand Down @@