From c1802eb603dfcfd2fd15d249180ec26eb611d273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 3 Apr 2026 16:03:15 +0200 Subject: [PATCH] refactor(catalog-backend): permissions cleanup step 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make `permissionsRegistry` required and `permissions` always a `PermissionsService` in `CatalogEnvironment`. Remove the deprecated `PermissionAuthorizer` fallback path and the `createPermissionIntegrationRouter` fallback — catalog now exclusively uses `permissionsRegistry.addResourceType` to register its permission resource type. Signed-off-by: Fredrik Adelöw Made-with: Cursor --- .../src/service/CatalogBuilder.ts | 77 ++++-------- .../src/service/createRouter.test.ts | 113 ------------------ .../src/service/createRouter.ts | 6 - 3 files changed, 21 insertions(+), 175 deletions(-) diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index d3bf47874d..190969d113 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/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 - | 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, }); diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 9d5944f664..d6c565ab5a 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/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; - let locationService: jest.Mocked; - 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(); diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index c2711a00ed..f996469a41 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/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) => {