feat(rbac): save role modification information to the metadata (#1280)

* feat(rbac): save role modification information to the metadata

Signed-off-by: Oleksandr Andriienko <[email protected]>

Author: AndrienkoAleksandr

plugins/rbac-backend/migrations/20240404111242_migrations.js

@@ -0,0 +1,41 @@
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.up = async function up(knex) {
+  const isRoleMetaDataExist = await knex.schema.hasTable('role-metadata');
+  if (isRoleMetaDataExist) {
+    await knex.schema.alterTable('role-metadata', table => {
+      table.string('author');
+      table.string('modifiedBy');
+      table.dateTime('createdAt');
+      table.dateTime('lastModified');
+    });
+
+    await knex('role-metadata')
+      .update({
+        description:
+          'The default permission policy for the admin role allows for the creation, deletion, updating, and reading of roles and permission policies.',
+        author: 'application configuration',
+        modifiedBy: 'application configuration',
+        lastModified: new Date().toUTCString(),
+      })
+      .where('roleEntityRef', 'role:default/rbac_admin');
+  }
+};
+
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.down = async function down(knex) {
+  const isRoleMetaDataExist = await knex.schema.hasTable('role-metadata');
+  if (isRoleMetaDataExist) {
+    await knex.schema.alterTable('role-metadata', table => {
+      table.dropColumn('author');
+      table.dropColumn('modifiedBy');
+      table.dropColumn('createdAt');
+      table.dropColumn('lastModified');
+    });
+  }
+};

plugins/rbac-backend/src/database/policy-metadata-storage.test.ts

@@ -31,17 +31,6 @@ describe('policy-metadata-db-table', () => {
       }),
       migrations: { skip: false },
     };
-    await knex.schema.createTable('casbin_rule', table => {
-      table.increments('id').primary();
-      table.string('ptype');
-      table.string('v0');
-      table.string('v1');
-      table.string('v2');
-      table.string('v3');
-      table.string('v4');
-      table.string('v5');
-      table.string('v6');
-    });
     await migrate(databaseManagerMock);
     return {
       knex,

plugins/rbac-backend/src/database/role-metadata.test.ts

@@ -26,17 +26,6 @@ describe('role-metadata-db-table', () => {
       }),
       migrations: { skip: false },
     };
-    await knex.schema.createTable('casbin_rule', table => {
-      table.increments('id').primary();
-      table.string('ptype');
-      table.string('v0');
-      table.string('v1');
-      table.string('v2');
-      table.string('v3');
-      table.string('v4');
-      table.string('v5');
-      table.string('v6');
-    });
     await migrate(databaseManagerMock);
     return {
       knex,
@@ -81,8 +70,12 @@ describe('role-metadata-db-table', () => {
           );
           await trx.commit();
           expect(roleMetadata).toEqual({
+            author: null,
+            createdAt: null,
             description: null,
             id: 1,
+            lastModified: null,
+            modifiedBy: null,
             roleEntityRef: 'role:default/some-super-important-role',
             source: 'rest',
           });
@@ -122,9 +115,13 @@ describe('role-metadata-db-table', () => {
         );
         expect(metadata.length).toEqual(1);
         expect(metadata[0]).toEqual({
+          author: null,
+          createdAt: null,
           roleEntityRef: 'role:default/some-super-important-role',
           description: null,
           id: 1,
+          lastModified: null,
+          modifiedBy: null,
           source: 'configuration',
         });
       },
@@ -274,10 +271,14 @@ describe('role-metadata-db-table', () => {
         );
         expect(metadata.length).toEqual(1);
         expect(metadata[0]).toEqual({
+          author: null,
+          createdAt: null,
           description: null,
           source: 'rest',
           roleEntityRef: 'role:default/some-super-important-role',
           id: 1,
+          lastModified: null,
+          modifiedBy: null,
         });
       },
     );
@@ -344,10 +345,14 @@ describe('role-metadata-db-table', () => {
         );
         expect(metadata.length).toEqual(1);
         expect(metadata[0]).toEqual({
+          author: null,
+          createdAt: null,
           description: null,
           source: 'configuration',
           roleEntityRef: 'role:default/important-role',
           id: 1,
+          lastModified: null,
+          modifiedBy: null,
         });
       },
     );

plugins/rbac-backend/src/database/role-metadata.ts

@@ -133,6 +133,10 @@ export function daoToMetadata(dao: RoleMetadataDao): RoleMetadata {
   return {
     source: dao.source,
     description: dao.description,
+    author: dao.author,
+    modifiedBy: dao.modifiedBy,
+    createdAt: dao.createdAt,
+    lastModified: dao.lastModified,
   };
 }
 

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

@@ -768,7 +768,10 @@ describe('CSV file', () => {
           },
         );
 
-      await enfDelegate.addGroupingPolicy(duplicateRest, 'rest');
+      await enfDelegate.addGroupingPolicy(duplicateRest, {
+        source: 'rest',
+        roleEntityRef: duplicateRest[1],
+      });
 
       const errorPolicyFile = resolve(
         __dirname,
@@ -962,7 +965,10 @@ describe('CSV file', () => {
         );
 
       await enfDelegate.addPolicy(duplicatePolicyEnforcer, 'rest');
-      await enfDelegate.addGroupingPolicy(duplicateRoleEnforcer, 'rest');
+      await enfDelegate.addGroupingPolicy(duplicateRoleEnforcer, {
+        source: 'rest',
+        roleEntityRef: duplicateRoleEnforcer[1],
+      });
 
       const errorPolicyFile = resolve(
         __dirname,

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

@@ -3,7 +3,10 @@ import { isEqual } from 'lodash';
 import { Logger } from 'winston';
 
 import { PolicyMetadataStorage } from '../database/policy-metadata-storage';
-import { RoleMetadataStorage } from '../database/role-metadata';
+import {
+  RoleMetadataDao,
+  RoleMetadataStorage,
+} from '../database/role-metadata';
 import { metadataStringToPolicy, transformArrayToPolicy } from '../helper';
 import { EnforcerDelegate } from '../service/enforcer-delegate';
 import { MODEL } from '../service/permission-model';
@@ -15,6 +18,8 @@ import {
   validatePolicy,
 } from '../service/policies-validation';
 
+export const CSV_PERMISSION_POLICY_FILE_AUTHOR = 'csv permission policy file';
+
 const addPolicy = async (
   policy: string[],
   enf: EnforcerDelegate,
@@ -204,7 +209,8 @@ export const loadFilteredGroupingPoliciesFromCSV = async (
       logger.warn(duplicateError.message);
     }
 
-    const err = validateEntityReference(role[1]);
+    const roleEntityRef = role[1];
+    const err = validateEntityReference(roleEntityRef);
     if (err) {
       logger.warn(
         `Failed to validate role from file ${policyFile}. Cause: ${err.message}`,
@@ -216,7 +222,12 @@ export const loadFilteredGroupingPoliciesFromCSV = async (
 
     // Role exists in the file but not the enforcer
     if (!(await enf.hasGroupingPolicy(...role))) {
-      await enf.addOrUpdateGroupingPolicy(role, 'csv-file');
+      await enf.addOrUpdateGroupingPolicy(role, {
+        roleEntityRef,
+        source: 'csv-file',
+        author: CSV_PERMISSION_POLICY_FILE_AUTHOR,
+        modifiedBy: CSV_PERMISSION_POLICY_FILE_AUTHOR,
+      });
     } else if (roleSource?.source !== 'csv-file') {
       logger.warn(
         `Duplicate role: ${role[0]}, ${role[1]} found with the source ${roleSource?.source}`,
@@ -238,7 +249,17 @@ export const loadFilteredGroupingPoliciesFromCSV = async (
       !(await tempEnforcer.hasGroupingPolicy(...role)) &&
       enfRoleSource?.source === 'csv-file'
     ) {
-      await enf.removeGroupingPolicy(role, 'csv-file', true, true);
+      await enf.removeGroupingPolicy(
+        role,
+        {
+          roleEntityRef: role[1],
+          source: 'csv-file',
+          author: CSV_PERMISSION_POLICY_FILE_AUTHOR,
+          modifiedBy: CSV_PERMISSION_POLICY_FILE_AUTHOR,
+        },
+        true,
+        true,
+      );
     }
   }
 
@@ -305,7 +326,12 @@ export const removedOldPermissionPoliciesFileData = async (
   }
 
   if (groupPoliciesToDelete.length > 0) {
-    await enf.removeGroupingPolicies(groupPoliciesToDelete, 'csv-file', true);
+    await enf.removeGroupingPolicies(
+      groupPoliciesToDelete,
+      'csv-file',
+      CSV_PERMISSION_POLICY_FILE_AUTHOR,
+      true,
+    );
   }
   if (policiesToDelete.length > 0) {
     await enf.removePolicies(policiesToDelete, 'csv-file', true);
@@ -360,6 +386,12 @@ export const addPermissionPoliciesFileData = async (
     if (duplicateError) {
       logger.warn(duplicateError.message);
     }
-    await enf.addOrUpdateGroupingPolicy(groupPolicy, 'csv-file', false);
+    const metadata: RoleMetadataDao = {
+      roleEntityRef: groupPolicy[1],
+      source: 'csv-file',
+      author: CSV_PERMISSION_POLICY_FILE_AUTHOR,
+      modifiedBy: CSV_PERMISSION_POLICY_FILE_AUTHOR,
+    };
+    await enf.addOrUpdateGroupingPolicy(groupPolicy, metadata, false);
   }
 };

plugins/rbac-backend/src/helper.test.ts

@@ -10,6 +10,7 @@ import {
 } from './helper';
 // Import the function to test
 import { EnforcerDelegate } from './service/enforcer-delegate';
+import { ADMIN_ROLE_AUTHOR } from './service/permission-policy';
 
 describe('helper.ts', () => {
   describe('policyToString', () => {
@@ -87,11 +88,16 @@ describe('helper.ts', () => {
         source,
         roleName,
         mockEnforcerDelegate as EnforcerDelegate,
+        ADMIN_ROLE_AUTHOR,
       );
 
       expect(mockEnforcerDelegate.removeGroupingPolicy).toHaveBeenCalledWith(
         ['user:default/admin', roleName],
-        source,
+        {
+          modifiedBy: ADMIN_ROLE_AUTHOR,
+          roleEntityRef: 'role:default/admin',
+          source: 'rest',
+        },
         false,
       );
     });
@@ -108,6 +114,7 @@ describe('helper.ts', () => {
         source,
         roleName,
         mockEnforcerDelegate as EnforcerDelegate,
+        ADMIN_ROLE_AUTHOR,
       );
 
       expect(mockEnforcerDelegate.removeGroupingPolicy).not.toHaveBeenCalled();
@@ -125,6 +132,7 @@ describe('helper.ts', () => {
         source,
         roleName,
         mockEnforcerDelegate as EnforcerDelegate,
+        ADMIN_ROLE_AUTHOR,
       );
 
       expect(mockEnforcerDelegate.removeGroupingPolicy).not.toHaveBeenCalled();
@@ -185,6 +193,34 @@ describe('helper.ts', () => {
       expect(deepSortedEqual(obj1, obj2)).toBe(true);
     });
 
+    it('should return true for identical objects with different ordering of top-level properties with exclude read only fields', () => {
+      const obj1: RoleMetadataDao = {
+        description: 'qa team',
+        id: 1,
+        roleEntityRef: 'role:default/qa',
+        source: 'rest',
+        // read only properties
+        author: 'role:default/some-role',
+        modifiedBy: 'role:default/some-role',
+        createdAt: '2024-02-26 12:25:31+00',
+        lastModified: '2024-02-26 12:25:31+00',
+      };
+      const obj2: RoleMetadataDao = {
+        id: 1,
+        description: 'qa team',
+        source: 'rest',
+        roleEntityRef: 'role:default/qa',
+      };
+      expect(
+        deepSortedEqual(obj1, obj2, [
+          'author',
+          'modifiedBy',
+          'createdAt',
+          'lastModified',
+        ]),
+      ).toBe(true);
+    });
+
     it('should return false for objects with different values', () => {
       const obj1: RoleMetadataDao = {
         description: 'qa',