fix(rbac): handle preexisting policies and grouping policies

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,
   ) {}
 
@@ -64,9 +66,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 +89,9 @@ export class EnforcerDelegate {
           `Failed to store policies ${policiesToString(policies)}`,
         );
       }
-      addMetadataTrx.commit();
+      await addMetadataTrx.commit();
     } catch (err) {
-      addMetadataTrx.rollback();
+      await addMetadataTrx.rollback();
       throw err;
     }
   }
@@ -98,18 +100,18 @@ 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);
       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;
     }
   }
@@ -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,
         );
       }
@@ -134,9 +136,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 +154,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 +178,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;
     }
   }
@@ -191,14 +193,17 @@ 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)}`);
       }
-      rmMetadataTrx.commit();
+      await rmMetadataTrx.commit();
     } catch (err) {
-      rmMetadataTrx.rollback(err);
+      await rmMetadataTrx.rollback(err);
       throw err;
     }
   }
@@ -211,17 +216,20 @@ 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) {
         throw new Error(
           `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 +273,46 @@ export class EnforcerDelegate {
   ): Promise<PermissionPolicyMetadataDao[]> {
     return await this.metadataStorage.findPolicyMetadataBySource(source);
   }
+
+  async getPoliciesWithoutSource(): Promise<string[][]> {
+    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 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');
+    }
+  }
 }

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,20 +27,14 @@ 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)) &&
+    (await enf.getMetadata(policy)).source === 'legacy'
+  ) {
+    await enf.removePolicy(policy);
   }
-}
+};
 
 const useAdmins = async (
   admins: Config[],
@@ -55,7 +49,7 @@ const useAdmins = async (
   if (!adminRoleMeta) {
     const trx = await knex.transaction();
     await roleMetadataStorage.createRoleMetadata(
-      { source: 'default' },
+      { source: 'configuration' },
       adminRoleName,
       trx,
     );
@@ -71,15 +65,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 +89,8 @@ const useAdmins = async (
     'delete',
     'allow',
   ];
+
+  await legacyPolicies(enf, adminDeletePermission);
   if (!(await enf.hasPolicy(...adminDeletePermission))) {
     await enf.addPolicy(adminDeletePermission, 'configuration');
   }
@@ -100,6 +101,8 @@ const useAdmins = async (
     'update',
     'allow',
   ];
+
+  await legacyPolicies(enf, adminUpdatePermission);
   if (!(await enf.hasPolicy(...adminUpdatePermission))) {
     await enf.addPolicy(adminUpdatePermission, 'configuration');
   }
@@ -147,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) {
@@ -166,6 +163,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');
     }
@@ -179,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');
     }
   }
 };

plugins/rbac-backend/src/service/policy-builder.ts

@@ -89,9 +89,12 @@ export class PolicyBuilder {
     const enforcerDelegate = new EnforcerDelegate(
       enf,
       policyMetadataStorage,
+      roleMetadataStorage,
       knex,
     );
 
+    await enforcerDelegate.migratePreexistingPolicies(enf);
+
     const options: RouterOptions = {
       config: env.config,
       logger: env.logger,

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