fix(rbac): check source before throwing duplicate warning (#1278)

Author: PatAKnight

plugins/rbac-backend/src/file-permissions/csv.test.ts

@@ -798,6 +798,17 @@ describe('CSV file', () => {
     let enfDelegate: EnforcerDelegate;
 
     beforeEach(async () => {
+      policyMetadataStorageMock.findPolicyMetadata = jest
+        .fn()
+        .mockImplementation(
+          async (
+            _policy: string[],
+            _trx: Knex.Knex.Transaction,
+          ): Promise<PermissionPolicyMetadata> => {
+            return { source: 'csv-file' };
+          },
+        );
+
       const adapter = new StringAdapter(
         `
           p, user:default/known_user, test.resource.deny, use, allow
@@ -828,6 +839,7 @@ describe('CSV file', () => {
 
     afterEach(() => {
       (loggerMock.warn as jest.Mock).mockReset();
+      (policyMetadataStorageMock.findPolicyMetadata as jest.Mock).mockReset();
     });
 
     it('should add policies from the CSV file', async () => {
@@ -926,6 +938,23 @@ describe('CSV file', () => {
         'role:default/catalog-updater',
       ];
 
+      policyMetadataStorageMock.findPolicyMetadata = jest
+        .fn()
+        .mockImplementation(
+          async (
+            policy: string[],
+            _trx: Knex.Knex.Transaction,
+          ): Promise<PermissionPolicyMetadata> => {
+            if (
+              isEqual(policy, duplicatePolicyEnforcer) ||
+              isEqual(policy, duplicateRoleEnforcer)
+            ) {
+              return { source: 'rest' };
+            }
+            return { source: 'csv-file' };
+          },
+        );
+
       await enfDelegate.addPolicy(duplicatePolicyEnforcer, 'rest');
       await enfDelegate.addGroupingPolicy(duplicateRoleEnforcer, 'rest');
 
@@ -943,7 +972,7 @@ describe('CSV file', () => {
 
       expect(loggerMock.warn).toHaveBeenNthCalledWith(
         1,
-        `Duplicate policy: ${duplicatePolicyEnforcer} found in the file ${errorPolicyFile}`,
+        `Duplicate policy: ${duplicatePolicyEnforcer} found in the file ${errorPolicyFile}, originates from source: rest`,
       );
       expect(loggerMock.warn).toHaveBeenNthCalledWith(
         2,

plugins/rbac-backend/src/service/permission-policy.test.ts

@@ -100,7 +100,19 @@ const policyMetadataStorageMock: PolicyMetadataStorage = {
         return [];
       },
     ),
-  findPolicyMetadata: jest.fn().mockImplementation(),
+  findPolicyMetadata: jest
+    .fn()
+    .mockImplementation(
+      async (
+        _policy: string[],
+        _trx: Knex.Knex.Transaction,
+      ): Promise<PermissionPolicyMetadata> => {
+        const test: PermissionPolicyMetadata = {
+          source: 'csv-file',
+        };
+        return test;
+      },
+    ),
   createPolicyMetadata: jest.fn().mockImplementation(),
   removePolicyMetadata: jest.fn().mockImplementation(),
 };

plugins/rbac-backend/src/service/policies-validation.ts

@@ -154,7 +154,7 @@ async function validateGroupingPolicy(
   const metadata = await roleMetadataStorage.findRoleMetadata(parent);
   if (metadata && metadata.source !== source && metadata.source !== 'legacy') {
     return new Error(
-      `You could not add user or group to the role created with source ${metadata.source}`,
+      `Group policy is invalid: ${groupPolicy}. You could not add user or group to the role created with source ${metadata.source}`,
     );
   }
   return undefined;
@@ -176,9 +176,12 @@ export async function validateAllPredefinedPolicies(
     }
 
     if (await enforcer.hasPolicy(...policy)) {
-      return new Error(
-        `Duplicate policy: ${policy} found in the file ${preDefinedPoliciesFile}`,
-      );
+      const source = (await enforcer.getMetadata(policy)).source;
+      if (source !== 'csv-file') {
+        return new Error(
+          `Duplicate policy: ${policy} found in the file ${preDefinedPoliciesFile}, originates from source: ${source}`,
+        );
+      }
     }
   }
 
@@ -194,9 +197,12 @@ export async function validateAllPredefinedPolicies(
     }
 
     if (await enforcer.hasGroupingPolicy(...groupPolicy)) {
-      return new Error(
-        `Duplicate role: ${groupPolicy} found in the file ${preDefinedPoliciesFile}`,
-      );
+      const source = (await enforcer.getMetadata(groupPolicy)).source;
+      if (source !== 'csv-file') {
+        return new Error(
+          `Duplicate role: ${groupPolicy} found in the file ${preDefinedPoliciesFile}, originates from source: ${source}`,
+        );
+      }
     }
   }
   return undefined;