diff --git a/plugins/catalog-backend/src/processing/types.ts b/plugins/catalog-backend/src/processing/types.ts index db5f85415d..002747d4c4 100644 --- a/plugins/catalog-backend/src/processing/types.ts +++ b/plugins/catalog-backend/src/processing/types.ts @@ -20,7 +20,6 @@ import { DeferredEntity, EntityRelationSpec, } from '@backstage/plugin-catalog-node'; -import { BackstageCredentials } from '@backstage/backend-plugin-api'; /** * The request to process an entity. @@ -29,7 +28,6 @@ import { BackstageCredentials } from '@backstage/backend-plugin-api'; export type EntityProcessingRequest = { entity: Entity; state?: JsonObject; // Versions for multiple deployments etc - credentials?: BackstageCredentials; }; /** diff --git a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.test.ts b/plugins/catalog-backend/src/service/AuthorizedValidationService.test.ts similarity index 82% rename from plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.test.ts rename to plugins/catalog-backend/src/service/AuthorizedValidationService.test.ts index 72572b68e7..1e4f74a97b 100644 --- a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedValidationService.test.ts @@ -18,9 +18,9 @@ import { NotAllowedError } from '@backstage/errors'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { ServerPermissionClient } from '@backstage/plugin-permission-node'; import { mockCredentials } from '@backstage/backend-test-utils'; -import { AuthorizedCatalogProcessingOrchestrator } from './AuthorizedCatalogProcessingOrchestrator'; +import { AuthorizedValidationService } from './AuthorizedValidationService'; -describe('AuthorizedCatalogProcessingOrchestrator', () => { +describe('AuthorizedValidationService', () => { const orchestratorService = { process: jest.fn(), }; @@ -38,7 +38,7 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => { result: AuthorizeResult.DENY, }, ]); - const authorizedService = new AuthorizedCatalogProcessingOrchestrator( + const authorizedService = new AuthorizedValidationService( orchestratorService, permissionApi as unknown as ServerPermissionClient, ); @@ -56,10 +56,12 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => { owner: 'team-a', }, }, - credentials: mockCredentials.none(), }; await expect(() => - authorizedService.process(entityProcessingRequest), + authorizedService.process( + entityProcessingRequest, + mockCredentials.none(), + ), ).rejects.toThrow(NotAllowedError); }); @@ -69,7 +71,7 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => { result: AuthorizeResult.ALLOW, }, ]); - const authorizedService = new AuthorizedCatalogProcessingOrchestrator( + const authorizedService = new AuthorizedValidationService( orchestratorService, permissionApi as unknown as ServerPermissionClient, ); @@ -87,9 +89,11 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => { owner: 'team-a', }, }, - credentials: mockCredentials.none(), }; - await authorizedService.process(entityProcessingRequest); + await authorizedService.process( + entityProcessingRequest, + mockCredentials.none(), + ); expect(orchestratorService.process).toHaveBeenCalledWith( entityProcessingRequest, ); diff --git a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts b/plugins/catalog-backend/src/service/AuthorizedValidationService.ts similarity index 86% rename from plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts rename to plugins/catalog-backend/src/service/AuthorizedValidationService.ts index 76fad3b33c..870368c9ee 100644 --- a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts +++ b/plugins/catalog-backend/src/service/AuthorizedValidationService.ts @@ -26,23 +26,16 @@ import { PermissionsService, } from '@backstage/backend-plugin-api'; -export class AuthorizedCatalogProcessingOrchestrator - implements CatalogProcessingOrchestrator -{ +export class AuthorizedValidationService { constructor( private readonly service: CatalogProcessingOrchestrator, private readonly permissionApi: PermissionsService, ) {} + async process( request: EntityProcessingRequest, + credentials: BackstageCredentials, ): Promise { - if (request.credentials) { - await this.ensureAuthorized(request.credentials); - } - return this.service.process(request); - } - - private async ensureAuthorized(credentials: BackstageCredentials) { const authorizeDecision = ( await this.permissionApi.authorize( [ @@ -57,5 +50,6 @@ export class AuthorizedCatalogProcessingOrchestrator if (authorizeDecision.result !== AuthorizeResult.ALLOW) { throw new NotAllowedError(); } + return this.service.process(request); } } diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 0305fea3ad..dac0a64e59 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -74,7 +74,6 @@ import { DefaultCatalogProcessingEngine } from '../processing/DefaultCatalogProc import { DefaultLocationService } from './DefaultLocationService'; import { DefaultEntitiesCatalog } from './DefaultEntitiesCatalog'; import { DefaultCatalogProcessingOrchestrator } from '../processing/DefaultCatalogProcessingOrchestrator'; -import { AuthorizedCatalogProcessingOrchestrator } from './AuthorizedCatalogProcessingOrchestrator'; import { DefaultStitcher } from '../stitching/DefaultStitcher'; import { createRouter } from './createRouter'; import { DefaultRefreshService } from './DefaultRefreshService'; @@ -534,18 +533,15 @@ export class CatalogBuilder { permissionsService = toPermissionEvaluator(permissions); } - const orchestrator = new AuthorizedCatalogProcessingOrchestrator( - new DefaultCatalogProcessingOrchestrator({ - processors, - integrations, - rulesEnforcer, - logger, - parser, - policy, - legacySingleProcessorValidation: this.legacySingleProcessorValidation, - }), - permissionsService, - ); + const orchestrator = new DefaultCatalogProcessingOrchestrator({ + processors, + integrations, + rulesEnforcer, + logger, + parser, + policy, + legacySingleProcessorValidation: this.legacySingleProcessorValidation, + }); const entitiesCatalog = new AuthorizedEntitiesCatalog( unauthorizedEntitiesCatalog, @@ -632,6 +628,7 @@ export class CatalogBuilder { permissionIntegrationRouter, auth, httpAuth, + permissionsService, }); await connectEntityProviders(providerDatabase, entityProviders); diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 57d476ac10..9cc0717807 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -42,6 +42,7 @@ import { wrapInOpenApiTestServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; +import { PermissionsService } from '@backstage/backend-plugin-api'; describe('createRouter readonly disabled', () => { let entitiesCatalog: jest.Mocked; @@ -50,6 +51,7 @@ describe('createRouter readonly disabled', () => { let app: express.Express | Server; let refreshService: RefreshService; let locationAnalyzer: jest.Mocked; + let permissionsService: jest.Mocked; beforeAll(async () => { entitiesCatalog = { @@ -71,6 +73,11 @@ describe('createRouter readonly disabled', () => { locationAnalyzer = { analyzeLocation: jest.fn(), }; + permissionsService = { + authorize: jest.fn(), + authorizeConditional: jest.fn(), + }; + refreshService = { refresh: jest.fn() }; orchestrator = { process: jest.fn() }; const router = await createRouter({ @@ -84,6 +91,7 @@ describe('createRouter readonly disabled', () => { auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), locationAnalyzer, + permissionsService: permissionsService, }); app = wrapInOpenApiTestServer(express().use(router)); }); @@ -725,6 +733,12 @@ describe('createRouter readonly disabled', () => { metadata: { name: 'n' }, }; + permissionsService.authorize.mockResolvedValueOnce([ + { + result: AuthorizeResult.ALLOW, + }, + ]); + orchestrator.process.mockResolvedValueOnce({ ok: true, state: {}, @@ -742,7 +756,6 @@ describe('createRouter readonly disabled', () => { expect(response.status).toEqual(200); expect(orchestrator.process).toHaveBeenCalledTimes(1); expect(orchestrator.process).toHaveBeenCalledWith({ - credentials: mockCredentials.user(), entity: { apiVersion: 'a', kind: 'b', @@ -766,6 +779,12 @@ describe('createRouter readonly disabled', () => { metadata: { name: 'invalid*name' }, }; + permissionsService.authorize.mockResolvedValueOnce([ + { + result: AuthorizeResult.ALLOW, + }, + ]); + orchestrator.process.mockResolvedValueOnce({ ok: false, errors: [new Error('Invalid entity name')], @@ -780,7 +799,6 @@ describe('createRouter readonly disabled', () => { expect(response.body.errors[0].message).toEqual('Invalid entity name'); expect(orchestrator.process).toHaveBeenCalledTimes(1); expect(orchestrator.process).toHaveBeenCalledWith({ - credentials: mockCredentials.user(), entity: { apiVersion: 'a', kind: 'b', @@ -804,6 +822,12 @@ describe('createRouter readonly disabled', () => { metadata: { name: 'n' }, }; + permissionsService.authorize.mockResolvedValueOnce([ + { + result: AuthorizeResult.ALLOW, + }, + ]); + const response = await request(app) .post('/validate-entity') .send({ entity, location: null }); @@ -850,6 +874,7 @@ describe('createRouter readonly enabled', () => { let entitiesCatalog: jest.Mocked; let app: express.Express; let locationService: jest.Mocked; + let permissionsService: jest.Mocked; beforeAll(async () => { entitiesCatalog = { @@ -879,6 +904,7 @@ describe('createRouter readonly enabled', () => { permissionIntegrationRouter: express.Router(), auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), + permissionsService: permissionsService, }); app = express().use(router); }); @@ -1048,6 +1074,7 @@ describe('NextRouter permissioning', () => { let locationService: jest.Mocked; let app: express.Express; let refreshService: RefreshService; + let permissionsService: jest.Mocked; const fakeRule = createPermissionRule({ name: 'FAKE_RULE', @@ -1094,6 +1121,7 @@ describe('NextRouter permissioning', () => { }), auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), + permissionsService: permissionsService, }); app = express().use(router); }); diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 279267bd71..d2b25fe226 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -53,8 +53,10 @@ import { HttpAuthService, LoggerService, SchedulerService, + PermissionsService, } from '@backstage/backend-plugin-api'; import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; +import { AuthorizedValidationService } from './AuthorizedValidationService'; /** * Options used by {@link createRouter}. @@ -74,6 +76,7 @@ export interface RouterOptions { permissionIntegrationRouter?: express.Router; auth: AuthService; httpAuth: HttpAuthService; + permissionsService: PermissionsService; } /** @@ -98,6 +101,7 @@ export async function createRouter( config, logger, permissionIntegrationRouter, + permissionsService, auth, httpAuth, } = options; @@ -346,20 +350,26 @@ export async function createRouter( } const credentials = await httpAuth.credentials(req); - const processingResult = await orchestrator.process({ - entity: { - ...entity, - metadata: { - ...entity.metadata, - annotations: { - [ANNOTATION_LOCATION]: body.location, - [ANNOTATION_ORIGIN_LOCATION]: body.location, - ...entity.metadata.annotations, + const authorizedValidationService = new AuthorizedValidationService( + orchestrator, + permissionsService, + ); + const processingResult = await authorizedValidationService.process( + { + entity: { + ...entity, + metadata: { + ...entity.metadata, + annotations: { + [ANNOTATION_LOCATION]: body.location, + [ANNOTATION_ORIGIN_LOCATION]: body.location, + ...entity.metadata.annotations, + }, }, }, }, - credentials: credentials, - }); + credentials, + ); if (!processingResult.ok) res.status(400).json({