fix(rbac): use loadPolicy to keep in sync enforcer for edit operations (

#2166) * fix(rbac): use loadPolicy to keep in sync enforcer for edit operations Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): fix permission evaluation when a lot of requests Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): fix error handling Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): remove not needed changes Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): fix up Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): add changeset Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): provide db pool size info to casbin adapter Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): handle code review feedback, improve changeset Signed-off-by: Oleksandr Andriienko <[email protected]> --------- Signed-off-by: Oleksandr Andriienko <[email protected]>
backstage · Jan 16, 2025 · ba4b3e9 · ba4b3e9
1 parent a763ac3
commit ba4b3e9
Show file tree

Hide file tree

Showing 14 changed files with 419 additions and 226 deletions.
diff --git a/workspaces/rbac/.changeset/red-glasses-reply.md b/workspaces/rbac/.changeset/red-glasses-reply.md
@@ -0,0 +1,5 @@
+---
+'@backstage-community/plugin-rbac-backend': patch
+---
+
+Use loadPolicy to keep the enforcer in sync for edit operations. It should keep the RBAC plugin in sync when the Backstage instance is scaled to multiple deployment replicas. Reuse the maximum database pool size value from the application configuration in the RBAC Casbin adapter.
diff --git a/workspaces/rbac/plugins/rbac-backend/__fixtures__/test-utils.ts b/workspaces/rbac/plugins/rbac-backend/__fixtures__/test-utils.ts
@@ -140,7 +140,12 @@ export async function newEnforcerDelegate(
     await enf.addGroupingPolicies(storedGroupingPolicies);
   }
 
-  return new EnforcerDelegate(enf, roleMetadataStorageMock, mockClientKnex);
+  return new EnforcerDelegate(
+    enf,
+    auditLogger(),
+    roleMetadataStorageMock,
+    mockClientKnex,
+  );
 }
 
 export async function newPermissionPolicy(

diff --git a/workspaces/rbac/plugins/rbac-backend/package.json b/workspaces/rbac/plugins/rbac-backend/package.json
@@ -64,6 +64,7 @@
     "@backstage/types": "1.1.1",
     "@spotify/prettier-config": "^15.0.0",
     "@types/express": "4.17.21",
+    "@types/knex": "^0.16.1",
     "@types/lodash": "^4.14.151",
     "@types/node": "18.19.68",
     "@types/supertest": "2.0.16",

diff --git a/workspaces/rbac/plugins/rbac-backend/src/audit-log/audit-logger.ts b/workspaces/rbac/plugins/rbac-backend/src/audit-log/audit-logger.ts
@@ -92,6 +92,10 @@ export type EvaluationAuditInfo = {
   decision?: PolicyDecision;
 };
 
+export const PoliciesData = {
+  FAILED_TO_FETCH_NEWER_PERMISSIONS: 'FailedToFetchNewerPermissions',
+};
+
 export const ConditionEvents = {
   CREATE_CONDITION: 'CreateCondition',
   UPDATE_CONDITION: 'UpdateCondition',
@@ -116,6 +120,9 @@ export const RBAC_BACKEND = 'rbac-backend';
 // Audit log stage for processing Role-Based Access Control (RBAC) data
 export const HANDLE_RBAC_DATA_STAGE = 'handleRBACData';
 
+// Audit log stage to refresh Role-Based Access Control (RBAC) data
+export const FETCH_NEWER_PERMISSIONS_STAGE = 'fetchNewerPermissions';
+
 // Audit log stage for determining access rights based on user permissions and resource information
 export const EVALUATE_PERMISSION_ACCESS_STAGE = 'evaluatePermissionAccess';
 

diff --git a/workspaces/rbac/plugins/rbac-backend/src/database/casbin-adapter-factory.ts b/workspaces/rbac/plugins/rbac-backend/src/database/casbin-adapter-factory.ts
@@ -54,6 +54,7 @@ export class CasbinDBAdapterFactory {
         ssl,
         database: dbName,
         schema: schema,
+        poolSize: databaseConfig?.getOptionalNumber('knexConfig.pool.max'),
       });
     }
 

diff --git a/workspaces/rbac/plugins/rbac-backend/src/file-permissions/csv-file-watcher.test.ts b/workspaces/rbac/plugins/rbac-backend/src/file-permissions/csv-file-watcher.test.ts
@@ -180,7 +180,12 @@ describe('CSVFileWatcher', () => {
 
     const knex = Knex.knex({ client: MockClient });
 
-    enforcerDelegate = new EnforcerDelegate(enf, roleMetadataStorageMock, knex);
+    enforcerDelegate = new EnforcerDelegate(
+      enf,
+      auditLoggerMock,
+      roleMetadataStorageMock,
+      knex,
+    );
 
     auditLoggerMock.auditLog.mockReset();
     (roleMetadataStorageMock.updateRoleMetadata as jest.Mock).mockClear();

diff --git a/workspaces/rbac/plugins/rbac-backend/src/policies/permission-policy.test.ts b/workspaces/rbac/plugins/rbac-backend/src/policies/permission-policy.test.ts
@@ -1750,6 +1750,7 @@ describe('Policy checks for conditional policies', () => {
 
     const enfDelegate = new EnforcerDelegate(
       enf,
+      auditLoggerMock,
       roleMetadataStorageMock,
       mockClientKnex,
     );
@@ -2173,7 +2174,12 @@ async function newEnforcerDelegate(
     await enf.addGroupingPolicies(storedGroupingPolicies);
   }
 
-  return new EnforcerDelegate(enf, roleMetadataStorageMock, mockClientKnex);
+  return new EnforcerDelegate(
+    enf,
+    auditLoggerMock,
+    roleMetadataStorageMock,
+    mockClientKnex,
+  );
 }
 
 async function newPermissionPolicy(

diff --git a/workspaces/rbac/plugins/rbac-backend/src/policies/permission-policy.ts b/workspaces/rbac/plugins/rbac-backend/src/policies/permission-policy.ts
@@ -216,6 +216,7 @@ export class RBACPermissionPolicy implements PermissionPolicy {
       }
 
       const permissionName = request.permission.name;
+      await this.enforcer.loadPolicy();
       const roles = await this.enforcer.getRolesForUser(userEntityRef);
 
       if (isResourcePermission(request.permission)) {

diff --git a/workspaces/rbac/plugins/rbac-backend/src/providers/connect-providers.test.ts b/workspaces/rbac/plugins/rbac-backend/src/providers/connect-providers.test.ts
@@ -184,7 +184,12 @@ describe('Connection', () => {
 
     const knex = Knex.knex({ client: MockClient });
 
-    enforcerDelegate = new EnforcerDelegate(enf, roleMetadataStorageMock, knex);
+    enforcerDelegate = new EnforcerDelegate(
+      enf,
+      auditLoggerMock,
+      roleMetadataStorageMock,
+      knex,
+    );
 
     await enforcerDelegate.addGroupingPolicy(
       roleToBeRemoved,
@@ -478,6 +483,7 @@ describe('connectRBACProviders', () => {
 
     const enforcerDelegate = new EnforcerDelegate(
       enf,
+      auditLoggerMock,
       roleMetadataStorageMock,
       knex,
     );

diff --git a/workspaces/rbac/plugins/rbac-backend/src/providers/connect-providers.ts b/workspaces/rbac/plugins/rbac-backend/src/providers/connect-providers.ts
@@ -67,6 +67,7 @@ export class Connection implements RBACProviderConnection {
 
     const providerRoles = await this.getProviderRoles();
 
+    await this.enforcer.loadPolicy();
     // Get the roles for this provider coming from rbac plugin
     for (const providerRole of providerRoles) {
       providerRolesforRemoval.push(
@@ -95,6 +96,7 @@ export class Connection implements RBACProviderConnection {
 
     const providerRoles = await this.getProviderRoles();
 
+    await this.enforcer.loadPolicy();
     // Get the roles for this provider coming from rbac plugin
     for (const providerRole of providerRoles) {
       providerPermissions.push(

diff --git a/workspaces/rbac/plugins/rbac-backend/src/service/enforcer-delegate.test.ts b/workspaces/rbac/plugins/rbac-backend/src/service/enforcer-delegate.test.ts
@@ -27,6 +27,7 @@ import {
 import { BackstageRoleManager } from '../role-manager/role-manager';
 import { EnforcerDelegate } from './enforcer-delegate';
 import { MODEL } from './permission-model';
+import { auditLoggerMock } from '../../__fixtures__/mock-utils';
 
 // TODO: Move to 'catalogServiceMock' from '@backstage/plugin-catalog-node/testUtils'
 // once '@backstage/plugin-catalog-node' is upgraded
@@ -179,7 +180,12 @@ describe('EnforcerDelegate', () => {
       await enf.addGroupingPolicies(groupingPolicies);
     }
 
-    return new EnforcerDelegate(enf, roleMetadataStorageMock, knex);
+    return new EnforcerDelegate(
+      enf,
+      auditLoggerMock,
+      roleMetadataStorageMock,
+      knex,
+    );
   }
 
   describe('hasPolicy', () => {
-Original file line number
+Diff line change
@@ Expand Up / @@ -54,6 +54,7 @@ export class CasbinDBAdapterFactory { @@
             ssl,
             database: dbName,
             schema: schema,
+            poolSize: databaseConfig?.getOptionalNumber('knexConfig.pool.max'),
           });
         }
@@ Expand Down @@