fix(rbac): prevent policy creation when rbac plugin is disabled (#2407)

* fix(rbac): prevent policy creation when rbac plugin is disabled

* fix(rbac): move permission-policy.ts to policies directory

* fix(rbac): clean up some options for policy server

* fix(rbac): include changeset for rbac backend plugin

* fix(rbac): fix some broken imports after rebase

.changeset/beige-parrots-act.md

@@ -0,0 +1,5 @@
+---
+"@janus-idp/backstage-plugin-rbac-backend": patch
+---
+
+Refactors the rbac backend plugin to prevent the creation of permission policies and roles whenever the plugin and permission framework is disabled

plugins/rbac-backend/__fixtures__/test-utils.ts

@@ -14,10 +14,10 @@ import { MockClient } from 'knex-mock-client';
 
 import { CasbinDBAdapterFactory } from '../src/database/casbin-adapter-factory';
 import { RoleMetadataStorage } from '../src/database/role-metadata';
+import { RBACPermissionPolicy } from '../src/policies/permission-policy';
 import { BackstageRoleManager } from '../src/role-manager/role-manager';
 import { EnforcerDelegate } from '../src/service/enforcer-delegate';
 import { MODEL } from '../src/service/permission-model';
-import { RBACPermissionPolicy } from '../src/service/permission-policy';
 import { PluginPermissionMetadataCollector } from '../src/service/plugin-endpoints';
 import {
   auditLoggerMock,

plugins/rbac-backend/src/policies/allow-all-policy.test.ts

@@ -0,0 +1,67 @@
+import {
+  AuthorizeResult,
+  createPermission,
+} from '@backstage/plugin-permission-common';
+import {
+  PermissionPolicy,
+  PolicyQuery,
+  PolicyQueryUser,
+} from '@backstage/plugin-permission-node';
+
+import { AllowAllPolicy } from './allow-all-policy';
+
+describe('Allow All Policy', () => {
+  describe('Allow all policy should allow all', () => {
+    let policy: PermissionPolicy;
+    beforeEach(() => {
+      policy = new AllowAllPolicy();
+    });
+
+    it('should be able to create an allow all permission policy', () => {
+      expect(policy).not.toBeNull();
+    });
+
+    it('should allow all when handle is called', async () => {
+      const result = await policy.handle(
+        newPolicyQueryWithBasicPermission('catalog.entity.create'),
+        newPolicyQueryUser('user:default/guest'),
+      );
+
+      expect(result).toStrictEqual({ result: AuthorizeResult.ALLOW });
+    });
+  });
+});
+
+function newPolicyQueryWithBasicPermission(name: string): PolicyQuery {
+  const mockPermission = createPermission({
+    name: name,
+    attributes: {},
+  });
+  return { permission: mockPermission };
+}
+
+function newPolicyQueryUser(
+  user?: string,
+  ownershipEntityRefs?: string[],
+): PolicyQueryUser | undefined {
+  if (user) {
+    return {
+      identity: {
+        ownershipEntityRefs: ownershipEntityRefs ?? [],
+        type: 'user',
+        userEntityRef: user,
+      },
+      credentials: {
+        $$type: '@backstage/BackstageCredentials',
+        principal: true,
+        expiresAt: new Date('2021-01-01T00:00:00Z'),
+      },
+      info: {
+        userEntityRef: user,
+        ownershipEntityRefs: ownershipEntityRefs ?? [],
+      },
+      token: 'token',
+    };
+  }
+  return undefined;
+}

plugins/rbac-backend/src/policies/allow-all-policy.ts

@@ -0,0 +1,18 @@
+import {
+  AuthorizeResult,
+  PolicyDecision,
+} from '@backstage/plugin-permission-common';
+import {
+  PermissionPolicy,
+  PolicyQuery,
+  PolicyQueryUser,
+} from '@backstage/plugin-permission-node';
+
+export class AllowAllPolicy implements PermissionPolicy {
+  async handle(
+    _request: PolicyQuery,
+    _user?: PolicyQueryUser,
+  ): Promise<PolicyDecision> {
+    return { result: AuthorizeResult.ALLOW };
+  }
+}

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

@@ -33,10 +33,10 @@ import {
   RoleMetadataStorage,
 } from '../database/role-metadata';
 import { BackstageRoleManager } from '../role-manager/role-manager';
-import { EnforcerDelegate } from './enforcer-delegate';
-import { MODEL } from './permission-model';
+import { EnforcerDelegate } from '../service/enforcer-delegate';
+import { MODEL } from '../service/permission-model';
+import { PluginPermissionMetadataCollector } from '../service/plugin-endpoints';
 import { RBACPermissionPolicy } from './permission-policy';
-import { PluginPermissionMetadataCollector } from './plugin-endpoints';
 
 type PermissionAction = 'create' | 'read' | 'update' | 'delete';
 

plugins/rbac-backend/src/service/permission-policy.ts → plugins/rbac-backend/src/policies/permission-policy.ts

@@ -43,8 +43,8 @@ import { ConditionalStorage } from '../database/conditional-storage';
 import { RoleMetadataStorage } from '../database/role-metadata';
 import { CSVFileWatcher } from '../file-permissions/csv-file-watcher';
 import { YamlConditinalPoliciesFileWatcher } from '../file-permissions/yaml-conditional-file-watcher';
-import { EnforcerDelegate } from './enforcer-delegate';
-import { PluginPermissionMetadataCollector } from './plugin-endpoints';
+import { EnforcerDelegate } from '../service/enforcer-delegate';
+import { PluginPermissionMetadataCollector } from '../service/plugin-endpoints';
 
 const evaluatePermMsg = (
   userEntityRef: string | undefined,

plugins/rbac-backend/src/service/policies-rest-api.test.ts

@@ -1,7 +1,6 @@
 import { MiddlewareFactory } from '@backstage/backend-defaults/rootHttpRouter';
 import { mockCredentials, mockServices } from '@backstage/backend-test-utils';
 import { InputError } from '@backstage/errors';
-import type { RouterOptions } from '@backstage/plugin-permission-backend';
 import { AuthorizeResult } from '@backstage/plugin-permission-common';
 import type { MetadataResponse } from '@backstage/plugin-permission-node';
 
@@ -28,13 +27,14 @@ import {
   RoleMetadataDao,
   RoleMetadataStorage,
 } from '../database/role-metadata';
+import { RBACPermissionPolicy } from '../policies/permission-policy';
 import { EnforcerDelegate } from './enforcer-delegate';
-import { RBACPermissionPolicy } from './permission-policy';
 import {
   PluginMetadataResponseSerializedRule,
   PluginPermissionMetadataCollector,
 } from './plugin-endpoints';
 import { PoliciesServer } from './policies-rest-api';
+import { RBACRouterOptions } from './policy-builder';
 
 jest.setTimeout(60000);
 
@@ -126,6 +126,7 @@ const mockHttpAuth = mockServices.httpAuth({
 });
 const mockAuthService = mockServices.auth();
 const credentials = mockCredentials.user('user:default/guest');
+const mockUserInfo = mockServices.userInfo();
 
 const conditions: RoleConditionalPolicyDecision<PermissionInfo>[] = [
   {
@@ -267,7 +268,7 @@ describe('REST policies api', () => {
 
     mockHttpAuth.credentials = jest.fn().mockImplementation(() => credentials);
 
-    const options: RouterOptions = {
+    const options: RBACRouterOptions = {
       config: config,
       logger,
       discovery: mockDiscovery,
@@ -284,15 +285,13 @@ describe('REST policies api', () => {
         pluginPermissionMetadataCollectorMock as PluginPermissionMetadataCollector,
         mockAuthService,
       ),
+      userInfo: mockUserInfo,
     };
 
     server = new PoliciesServer(
       mockPermissionEvaluator,
       options,
       enforcerDelegateMock as EnforcerDelegate,
-      config,
-      mockHttpAuth,
-      mockAuthService,
       conditionalStorageMock,
       pluginPermissionMetadataCollectorMock as PluginPermissionMetadataCollector,
       roleMetadataStorageMock,
@@ -3492,7 +3491,7 @@ describe('REST policies api', () => {
         { result: AuthorizeResult.ALLOW },
       ]);
 
-      const options: RouterOptions = {
+      const options: RBACRouterOptions = {
         config: config,
         logger,
         discovery: mockDiscovery,
@@ -3509,15 +3508,13 @@ describe('REST policies api', () => {
           pluginPermissionMetadataCollectorMock as PluginPermissionMetadataCollector,
           mockAuthService,
         ),
+        userInfo: mockUserInfo,
       };
 
       server = new PoliciesServer(
         mockPermissionEvaluator,
         options,
         enforcerDelegateMock as EnforcerDelegate,
-        config,
-        mockHttpAuth,
-        mockAuthService,
         conditionalStorageMock,
         pluginPermissionMetadataCollectorMock as PluginPermissionMetadataCollector,
         roleMetadataStorageMock,

plugins/rbac-backend/src/service/policies-rest-api.ts

@@ -1,20 +1,12 @@
-import type {
-  AuthService,
-  HttpAuthService,
-  PermissionsService,
-} from '@backstage/backend-plugin-api';
-import type { Config } from '@backstage/config';
+import type { PermissionsService } from '@backstage/backend-plugin-api';
 import {
   ConflictError,
   InputError,
   NotAllowedError,
   NotFoundError,
   ServiceUnavailableError,
 } from '@backstage/errors';
-import {
-  createRouter,
-  RouterOptions,
-} from '@backstage/plugin-permission-backend';
+import { createRouter } from '@backstage/plugin-permission-backend';
 import {
   AuthorizeResult,
   PolicyDecision,
@@ -76,15 +68,13 @@ import {
 } from '../validation/policies-validation';
 import { EnforcerDelegate } from './enforcer-delegate';
 import { PluginPermissionMetadataCollector } from './plugin-endpoints';
+import { RBACRouterOptions } from './policy-builder';
 
 export class PoliciesServer {
   constructor(
     private readonly permissions: PermissionsService,
-    private readonly options: RouterOptions,
+    private readonly options: RBACRouterOptions,
     private readonly enforcer: EnforcerDelegate,
-    private readonly config: Config,
-    private readonly httpAuth: HttpAuthService,
-    private readonly auth: AuthService,
     private readonly conditionalStorage: ConditionalStorage,
     private readonly pluginPermMetaData: PluginPermissionMetadataCollector,
     private readonly roleMetadata: RoleMetadataStorage,
@@ -96,13 +86,13 @@ export class PoliciesServer {
     request: Request,
     permission: ResourcePermission,
   ): Promise<PolicyDecision> {
-    const credentials = await this.httpAuth.credentials(request, {
+    const credentials = await this.options.httpAuth.credentials(request, {
       allow: ['user', 'service'],
     });
 
     // allow service to service communication, but only with read permission
     if (
-      this.auth.isPrincipal(credentials, 'service') &&
+      this.options.auth.isPrincipal(credentials, 'service') &&
       permission !== policyEntityReadPermission
     ) {
       throw new NotAllowedError(
@@ -138,7 +128,7 @@ export class PoliciesServer {
     router.use(permissionsIntegrationRouter);
 
     const isPluginEnabled =
-      this.config.getOptionalBoolean('permission.enabled');
+      this.options.config.getOptionalBoolean('permission.enabled');
     if (!isPluginEnabled) {
       return router;
     }
@@ -767,7 +757,9 @@ export class PoliciesServer {
         throw new NotAllowedError(); // 403
       }
 
-      const body = await this.pluginPermMetaData.getPluginPolicies(this.auth);
+      const body = await this.pluginPermMetaData.getPluginPolicies(
+        this.options.auth,
+      );
 
       await this.aLog.auditLog({
         message: `Return list plugin policies`,
@@ -792,7 +784,7 @@ export class PoliciesServer {
       }
 
       const body = await this.pluginPermMetaData.getPluginConditionRules(
-        this.auth,
+        this.options.auth,
       );
 
       await this.aLog.auditLog({
@@ -861,7 +853,7 @@ export class PoliciesServer {
       const conditionToCreate = await processConditionMapping(
         roleConditionPolicy,
         this.pluginPermMetaData,
-        this.auth,
+        this.options.auth,
       );
 
       const id =
@@ -982,7 +974,7 @@ export class PoliciesServer {
       const conditionToUpdate = await processConditionMapping(
         roleConditionPolicy,
         this.pluginPermMetaData,
-        this.auth,
+        this.options.auth,
       );
 
       await this.conditionalStorage.updateCondition(id, conditionToUpdate);

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

@@ -10,7 +10,7 @@ import type {
 } from '@janus-idp/backstage-plugin-rbac-node';
 
 import { CasbinDBAdapterFactory } from '../database/casbin-adapter-factory';
-import { RBACPermissionPolicy } from './permission-policy';
+import { RBACPermissionPolicy } from '../policies/permission-policy';
 import { PluginPermissionMetadataCollector } from './plugin-endpoints';
 import { PoliciesServer } from './policies-rest-api';
 import { PolicyBuilder } from './policy-builder';
@@ -78,7 +78,7 @@ jest.mock('./policies-rest-api', () => {
   };
 });
 
-jest.mock('./permission-policy', () => {
+jest.mock('../policies/permission-policy', () => {
   return {
     RBACPermissionPolicy: {
       build: jest.fn((): Promise<RBACPermissionPolicy> => {
@@ -221,7 +221,7 @@ describe('PolicyBuilder', () => {
     expect(CasbinDBAdapterFactory).toHaveBeenCalled();
     expect(enforcerMock.loadPolicy).toHaveBeenCalled();
     expect(enforcerMock.enableAutoSave).toHaveBeenCalled();
-    expect(RBACPermissionPolicy.build).toHaveBeenCalled();
+    expect(RBACPermissionPolicy.build).not.toHaveBeenCalled();
 
     expect(PoliciesServer).toHaveBeenCalled();
     expect(policiesServerMock.serve).toHaveBeenCalled();