Merge pull request #33743 from backstage/freben/permissions-cleanup-step-1

refactor(catalog-backend): permissions cleanup step 1

.changeset/catalog-backend-permissions-cleanup-step-1.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Removed deprecated `PermissionAuthorizer` support and the `createPermissionIntegrationRouter` fallback path from `CatalogBuilder`. The `permissionsRegistry` service is now required, and `permissions` is always a `PermissionsService`.

plugins/catalog-backend/src/service/CatalogBuilder.ts

@@ -43,10 +43,7 @@ import {
   UrlReaderService,
 } from '@backstage/backend-plugin-api';
 import { Config, readDurationFromConfig } from '@backstage/config';
-import {
-  catalogPermissions,
-  RESOURCE_TYPE_CATALOG_ENTITY,
-} from '@backstage/plugin-catalog-common/alpha';
+import { catalogPermissions } from '@backstage/plugin-catalog-common/alpha';
 import {
   CatalogProcessor,
   CatalogProcessorParser,
@@ -55,15 +52,8 @@ import {
   ScmLocationAnalyzer,
 } from '@backstage/plugin-catalog-node';
 import { EventsService } from '@backstage/plugin-events-node';
-import {
-  Permission,
-  PermissionAuthorizer,
-  toPermissionEvaluator,
-} from '@backstage/plugin-permission-common';
-import {
-  createConditionTransformer,
-  createPermissionIntegrationRouter,
-} from '@backstage/plugin-permission-node';
+import { Permission } from '@backstage/plugin-permission-common';
+import { createConditionTransformer } from '@backstage/plugin-permission-node';
 import { durationToMilliseconds } from '@backstage/types';
 import { DefaultCatalogDatabase } from '../database/DefaultCatalogDatabase';
 import { DefaultProcessingDatabase } from '../database/DefaultProcessingDatabase';
@@ -124,8 +114,8 @@ export type CatalogEnvironment = {
   database: DatabaseService;
   config: RootConfigService;
   reader: UrlReaderService;
-  permissions: PermissionsService | PermissionAuthorizer;
-  permissionsRegistry?: PermissionsRegistryService;
+  permissions: PermissionsService;
+  permissionsRegistry: PermissionsRegistryService;
   scheduler: SchedulerService;
   auth: AuthService;
   httpAuth: HttpAuthService;
@@ -479,16 +469,6 @@ export class CatalogBuilder {
       enableRelationsCompatibility,
     });
 
-    let permissionsService: PermissionsService;
-    if ('authorizeConditional' in permissions) {
-      permissionsService = permissions as PermissionsService;
-    } else {
-      logger.warn(
-        'PermissionAuthorizer is deprecated. Please use an instance of PermissionEvaluator instead of PermissionAuthorizer in PluginEnvironment#permissions',
-      );
-      permissionsService = toPermissionEvaluator(permissions);
-    }
-
     const orchestrator = new DefaultCatalogProcessingOrchestrator({
       processors,
       integrations,
@@ -500,14 +480,12 @@ export class CatalogBuilder {
 
     const entitiesCatalog = new AuthorizedEntitiesCatalog(
       unauthorizedEntitiesCatalog,
-      permissionsService,
-      permissionsRegistry
-        ? createConditionTransformer(
-            permissionsRegistry.getPermissionRuleset(
-              catalogEntityPermissionResourceRef,
-            ),
-          )
-        : createConditionTransformer(this.permissionRules),
+      permissions,
+      createConditionTransformer(
+        permissionsRegistry.getPermissionRuleset(
+          catalogEntityPermissionResourceRef,
+        ),
+      ),
     );
 
     const getResources = async (resourceRefs: string[]) => {
@@ -519,24 +497,12 @@ export class CatalogBuilder {
       return entitiesResponseToObjects(items).map(e => e || undefined);
     };
 
-    let permissionIntegrationRouter:
-      | ReturnType<typeof createPermissionIntegrationRouter>
-      | undefined;
-    if (permissionsRegistry) {
-      permissionsRegistry.addResourceType({
-        resourceRef: catalogEntityPermissionResourceRef,
-        getResources,
-        permissions: this.permissions,
-        rules: this.permissionRules,
-      });
-    } else {
-      permissionIntegrationRouter = createPermissionIntegrationRouter({
-        resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
-        getResources,
-        permissions: this.permissions,
-        rules: this.permissionRules,
-      });
-    }
+    permissionsRegistry.addResourceType({
+      resourceRef: catalogEntityPermissionResourceRef,
+      getResources,
+      permissions: this.permissions,
+      rules: this.permissionRules,
+    });
 
     const scmEventHandlingConfig = readScmEventHandlingConfig(config);
     const locationStore = new DefaultLocationStore(
@@ -589,7 +555,7 @@ export class CatalogBuilder {
       this.locationAnalyzer ??
       new AuthorizedLocationAnalyzer(
         new RepoLocationAnalyzer(logger, integrations, this.locationAnalyzers),
-        permissionsService,
+        permissions,
       );
     const locationService = new AuthorizedLocationService(
       new DefaultLocationService(locationStore, orchestrator, {
@@ -599,11 +565,11 @@ export class CatalogBuilder {
             'catalog.defaultLocationConflictStrategy',
           ) as 'refresh' | 'reject') || 'reject',
       }),
-      permissionsService,
+      permissions,
     );
     const refreshService = new AuthorizedRefreshService(
       new DefaultRefreshService({ database: catalogDatabase }),
-      permissionsService,
+      permissions,
     );
 
     const router = await createRouter({
@@ -614,10 +580,9 @@ export class CatalogBuilder {
       refreshService,
       logger,
       config,
-      permissionIntegrationRouter,
       auth,
       httpAuth,
-      permissionsService,
+      permissionsService: permissions,
       auditor,
       enableRelationsCompatibility,
     });

plugins/catalog-backend/src/service/createRouter.test.ts

@@ -31,17 +31,10 @@ import {
 } from '@backstage/catalog-model';
 import { ConfigReader } from '@backstage/config';
 import { NotFoundError } from '@backstage/errors';
-import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common/alpha';
 import { LocationAnalyzer } from '@backstage/plugin-catalog-node';
-import { AuthorizeResult } from '@backstage/plugin-permission-common';
-import {
-  createPermissionIntegrationRouter,
-  createPermissionRule,
-} from '@backstage/plugin-permission-node';
 import express from 'express';
 import { Server } from 'node:http';
 import request from 'supertest';
-import { z } from 'zod/v3';
 import { Cursor, EntitiesCatalog } from '../catalog/types';
 import { applyDatabaseMigrations } from '../database/migrations';
 import { DbLocationsRow } from '../database/tables';
@@ -98,7 +91,6 @@ describe('createRouter readonly disabled', () => {
       logger: mockServices.logger.mock(),
       refreshService,
       config: new ConfigReader(undefined),
-      permissionIntegrationRouter: express.Router(),
       auth: mockServices.auth(),
       httpAuth: mockServices.httpAuth(),
       locationAnalyzer,
@@ -1386,7 +1378,6 @@ describe('createRouter readonly and raw json enabled', () => {
           readonly: true,
         },
       }),
-      permissionIntegrationRouter: express.Router(),
       auth: mockServices.auth(),
       httpAuth: mockServices.httpAuth(),
       orchestrator: { process: jest.fn() },
@@ -1558,110 +1549,6 @@ describe('createRouter readonly and raw json enabled', () => {
   });
 });
 
-describe('NextRouter permissioning', () => {
-  let entitiesCatalog: jest.Mocked<EntitiesCatalog>;
-  let locationService: jest.Mocked<LocationService>;
-  let app: express.Express;
-  let refreshService: RefreshService;
-  const permissionsService = mockServices.permissions();
-
-  const fakeRule = createPermissionRule({
-    name: 'FAKE_RULE',
-    description: 'fake rule',
-    resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
-    paramsSchema: z.object({
-      foo: z.string(),
-    }),
-    apply: () => true,
-    toQuery: () => ({ key: '', values: [] }),
-  });
-
-  beforeAll(async () => {
-    entitiesCatalog = {
-      entities: jest.fn(),
-      entitiesBatch: jest.fn(),
-      removeEntityByUid: jest.fn(),
-      entityAncestry: jest.fn(),
-      facets: jest.fn(),
-      queryEntities: jest.fn(),
-    };
-    locationService = {
-      getLocation: jest.fn(),
-      createLocation: jest.fn(),
-      queryLocations: jest.fn(),
-      listLocations: jest.fn(),
-      deleteLocation: jest.fn(),
-      getLocationByEntity: jest.fn(),
-    };
-    refreshService = { refresh: jest.fn() };
-    const router = await createRouter({
-      entitiesCatalog,
-      locationService,
-      logger: mockServices.logger.mock(),
-      refreshService,
-      config: new ConfigReader(undefined),
-      permissionIntegrationRouter: createPermissionIntegrationRouter({
-        resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
-        rules: [fakeRule],
-        getResources: jest.fn((resourceRefs: string[]) =>
-          Promise.resolve(
-            resourceRefs.map(resourceRef => ({ id: resourceRef })),
-          ),
-        ),
-      }),
-      auth: mockServices.auth(),
-      httpAuth: mockServices.httpAuth(),
-      orchestrator: { process: jest.fn() },
-      permissionsService,
-      auditor: mockServices.auditor.mock(),
-    });
-    app = express().use(router);
-  });
-
-  afterEach(() => {
-    jest.clearAllMocks();
-  });
-
-  it('accepts and evaluates conditions at the apply-conditions endpoint', async () => {
-    const spideySense: Entity = {
-      apiVersion: 'a',
-      kind: 'component',
-      metadata: {
-        name: 'spidey-sense',
-      },
-    };
-    entitiesCatalog.entities.mockResolvedValueOnce({
-      entities: { type: 'object', entities: [spideySense] },
-      pageInfo: { hasNextPage: false },
-    });
-
-    const requestBody = {
-      items: [
-        {
-          id: '123',
-          resourceType: 'catalog-entity',
-          resourceRef: 'component:default/spidey-sense',
-          conditions: {
-            rule: 'FAKE_RULE',
-            resourceType: 'catalog-entity',
-            params: {
-              foo: 'user:default/spiderman',
-            },
-          },
-        },
-      ],
-    };
-    const response = await request(app)
-      .post('/.well-known/backstage/permissions/apply-conditions')
-      .send(requestBody);
-
-    expect(response.status).toBe(200);
-    expect(response.body).toEqual({
-      items: [{ id: '123', result: AuthorizeResult.ALLOW }],
-    });
-  });
-});
-
 describe('POST /locations/by-query works end to end', () => {
   const databases = TestDatabases.create();
 

plugins/catalog-backend/src/service/createRouter.ts

@@ -79,7 +79,6 @@ export interface RouterOptions {
   refreshService?: RefreshService;
   logger: LoggerService;
   config: Config;
-  permissionIntegrationRouter?: express.Router;
   auth: AuthService;
   httpAuth: HttpAuthService;
   permissionsService: PermissionsService;
@@ -108,7 +107,6 @@ export async function createRouter(
     refreshService,
     config,
     logger,
-    permissionIntegrationRouter,
     permissionsService,
     auth,
     httpAuth,
@@ -156,10 +154,6 @@ export async function createRouter(
     });
   }
 
-  if (permissionIntegrationRouter) {
-    router.use(permissionIntegrationRouter);
-  }
-
   if (entitiesCatalog) {
     router
       .get('/entities', async (req, res) => {