From 3161dd8c9ec82264cf6145dfc92a20a00c2bd676 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 6 Aug 2024 15:05:04 -0400 Subject: [PATCH 01/17] added the catalog.entity.create permission to the validate-entity endpoint Signed-off-by: Kashish Mittal --- .../catalog-backend/src/processing/types.ts | 2 + ...AuthorizedCatalogProcessingOrchestrator.ts | 61 +++++++++++++++++++ .../src/service/CatalogBuilder.ts | 24 +++++--- .../src/service/createRouter.ts | 2 + 4 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts diff --git a/plugins/catalog-backend/src/processing/types.ts b/plugins/catalog-backend/src/processing/types.ts index 002747d4c4..db5f85415d 100644 --- a/plugins/catalog-backend/src/processing/types.ts +++ b/plugins/catalog-backend/src/processing/types.ts @@ -20,6 +20,7 @@ import { DeferredEntity, EntityRelationSpec, } from '@backstage/plugin-catalog-node'; +import { BackstageCredentials } from '@backstage/backend-plugin-api'; /** * The request to process an entity. @@ -28,6 +29,7 @@ import { export type EntityProcessingRequest = { entity: Entity; state?: JsonObject; // Versions for multiple deployments etc + credentials?: BackstageCredentials; }; /** diff --git a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts b/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts new file mode 100644 index 0000000000..e4657f01ba --- /dev/null +++ b/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts @@ -0,0 +1,61 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { NotAllowedError } from '@backstage/errors'; +import { catalogEntityCreatePermission } from '@backstage/plugin-catalog-common/alpha'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { + CatalogProcessingOrchestrator, + EntityProcessingRequest, + EntityProcessingResult, +} from '../processing/types'; +import { + BackstageCredentials, + PermissionsService, +} from '@backstage/backend-plugin-api'; + +export class AuthorizedCatalogProcessingOrchestrator + implements CatalogProcessingOrchestrator +{ + constructor( + private readonly service: CatalogProcessingOrchestrator, + private readonly permissionApi: PermissionsService, + ) {} + async process( + request: EntityProcessingRequest, + ): 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( + [ + { + permission: catalogEntityCreatePermission, + }, + ], + { credentials }, + ) + )[0]; + + if (authorizeDecision.result !== AuthorizeResult.ALLOW) { + throw new NotAllowedError(); + } + } +} diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index aabef51195..67c7bf74af 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -76,6 +76,7 @@ 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'; @@ -510,15 +511,7 @@ export class CatalogBuilder { }); const integrations = ScmIntegrations.fromConfig(config); const rulesEnforcer = DefaultCatalogRulesEnforcer.fromConfig(config); - const orchestrator = new DefaultCatalogProcessingOrchestrator({ - processors, - integrations, - rulesEnforcer, - logger, - parser, - policy, - legacySingleProcessorValidation: this.legacySingleProcessorValidation, - }); + const unauthorizedEntitiesCatalog = new DefaultEntitiesCatalog({ database: dbClient, logger, @@ -535,6 +528,19 @@ export class CatalogBuilder { permissionsService = toPermissionEvaluator(permissions); } + const orchestrator = new AuthorizedCatalogProcessingOrchestrator( + new DefaultCatalogProcessingOrchestrator({ + processors, + integrations, + rulesEnforcer, + logger, + parser, + policy, + legacySingleProcessorValidation: this.legacySingleProcessorValidation, + }), + permissionsService, + ); + const entitiesCatalog = new AuthorizedEntitiesCatalog( unauthorizedEntitiesCatalog, permissionsService, diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 774ee61a6d..a4fd29cf76 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -342,6 +342,7 @@ export async function createRouter( }); } + const credentials = await httpAuth.credentials(req); const processingResult = await orchestrator.process({ entity: { ...entity, @@ -354,6 +355,7 @@ export async function createRouter( }, }, }, + credentials: credentials, }); if (!processingResult.ok) From 8c3821c24a0659bde51852ca31247b14bf4cc60e Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 6 Aug 2024 15:17:03 -0400 Subject: [PATCH 02/17] added the catalog.location.create permission to the analyze-location endpoint Signed-off-by: Kashish Mittal --- .../src/service/AuthorizedLocationAnalyzer.ts | 49 +++++++++++++++++++ .../src/service/CatalogBuilder.ts | 6 ++- .../src/service/createRouter.ts | 6 ++- .../src/ingestion/LocationAnalyzer.ts | 2 + 4 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts diff --git a/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts new file mode 100644 index 0000000000..3968a0ab73 --- /dev/null +++ b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts @@ -0,0 +1,49 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NotAllowedError } from '@backstage/errors'; +import { catalogLocationCreatePermission } from '@backstage/plugin-catalog-common/alpha'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { PermissionsService } from '@backstage/backend-plugin-api'; +import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; +import { AnalyzeLocationRequest } from '@backstage/plugin-catalog-common'; +import { AnalyzeLocationResponse } from '@backstage/plugin-catalog-common'; + +export class AuthorizedLocationAnalyzer implements LocationAnalyzer { + constructor( + private readonly service: LocationAnalyzer, + private readonly permissionApi: PermissionsService, + ) {} + + async analyzeLocation( + request: AnalyzeLocationRequest, + ): Promise { + const authorizeDecision = ( + await this.permissionApi.authorize( + [ + { + permission: catalogLocationCreatePermission, + }, + ], + { credentials: request.credentials }, + ) + )[0]; + if (authorizeDecision.result !== AuthorizeResult.ALLOW) { + throw new NotAllowedError(); + } + return this.service.analyzeLocation(request); + } +} diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 67c7bf74af..8c304748e4 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -59,6 +59,7 @@ import { import { ConfigLocationEntityProvider } from '../modules/core/ConfigLocationEntityProvider'; import { DefaultLocationStore } from '../modules/core/DefaultLocationStore'; import { RepoLocationAnalyzer } from '../ingestion/LocationAnalyzer'; +import { AuthorizedLocationAnalyzer } from './AuthorizedLocationAnalyzer'; import { jsonPlaceholderResolver, textPlaceholderResolver, @@ -600,7 +601,10 @@ export class CatalogBuilder { const locationAnalyzer = this.locationAnalyzer ?? - new RepoLocationAnalyzer(logger, integrations, this.locationAnalyzers); + new AuthorizedLocationAnalyzer( + new RepoLocationAnalyzer(logger, integrations, this.locationAnalyzers), + permissionsService, + ); const locationService = new AuthorizedLocationService( new DefaultLocationService(locationStore, orchestrator, { allowedLocationTypes: this.allowedLocationType, diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index a4fd29cf76..abba18b0fe 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -301,9 +301,13 @@ export async function createRouter( location: locationInput, catalogFilename: z.string().optional(), }); + const credentials = await httpAuth.credentials(req); const parsedBody = schema.parse(body); + const analyzeLocationRequest = { ...parsedBody, credentials }; try { - const output = await locationAnalyzer.analyzeLocation(parsedBody); + const output = await locationAnalyzer.analyzeLocation( + analyzeLocationRequest, + ); res.status(200).json(output); } catch (err) { if ( diff --git a/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts b/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts index 08ae5ef0cb..8dd40a015e 100644 --- a/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts +++ b/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts @@ -17,11 +17,13 @@ import { LocationSpec } from '../common'; import { Entity } from '@backstage/catalog-model'; import { RecursivePartial } from './RecursivePartial'; +import { BackstageCredentials } from '@backstage/backend-plugin-api'; /** @public */ export type AnalyzeLocationRequest = { location: LocationSpec; catalogFilename?: string; + credentials: BackstageCredentials; }; /** @public */ From 379c7de70787ebb071af05e2eed4c7ca39a985c1 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 6 Aug 2024 16:46:05 -0400 Subject: [PATCH 03/17] added changeset Signed-off-by: Kashish Mittal --- .changeset/serious-tables-chew.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/serious-tables-chew.md diff --git a/.changeset/serious-tables-chew.md b/.changeset/serious-tables-chew.md new file mode 100644 index 0000000000..cc5a218f3d --- /dev/null +++ b/.changeset/serious-tables-chew.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend': patch +'@backstage/plugin-catalog-common': patch +--- + +- The `analyze-location` endpoint is now protected by the `catalog.location.create` permission. +- The `validate-entity` endpoint is now protected by the `catalog.entity.create` permission. From c63e2e0ae880a579fd97b41749d0cc201674edcc Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 6 Aug 2024 16:55:46 -0400 Subject: [PATCH 04/17] ran yarn --cwd plugins/catalog-common add @backstage/backend-plugin-api Signed-off-by: Kashish Mittal --- package.json | 1 + yarn.lock | 1 + 2 files changed, 2 insertions(+) diff --git a/package.json b/package.json index d9b384fb7d..0e60de31e1 100644 --- a/package.json +++ b/package.json @@ -92,6 +92,7 @@ "jest-haste-map@^29.7.0": "patch:jest-haste-map@npm%3A29.7.0#./.yarn/patches/jest-haste-map-npm-29.7.0-e3be419eff.patch" }, "dependencies": { + "@backstage/backend-plugin-api": "workspace:^", "@backstage/errors": "workspace:^", "@manypkg/get-packages": "^1.1.3", "@types/global-agent": "^2.1.3", diff --git a/yarn.lock b/yarn.lock index c14251269c..d489203875 100644 --- a/yarn.lock +++ b/yarn.lock @@ -40239,6 +40239,7 @@ __metadata: version: 0.0.0-use.local resolution: "root@workspace:." dependencies: + "@backstage/backend-plugin-api": "workspace:^" "@backstage/cli": "workspace:*" "@backstage/codemods": "workspace:*" "@backstage/create-app": "workspace:*" From d5815be7337c6e5b9c92177523f44ef290a1d6e8 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 6 Aug 2024 17:10:22 -0400 Subject: [PATCH 05/17] added @backstage/backend-plugin-api to plugins/catalog-common/package.json Signed-off-by: Kashish Mittal --- plugins/catalog-common/package.json | 1 + yarn.lock | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/catalog-common/package.json b/plugins/catalog-common/package.json index cfcfee74d8..e4eee51e7f 100644 --- a/plugins/catalog-common/package.json +++ b/plugins/catalog-common/package.json @@ -56,6 +56,7 @@ "test": "backstage-cli package test" }, "dependencies": { + "@backstage/backend-plugin-api": "workspace:^", "@backstage/catalog-model": "workspace:^", "@backstage/plugin-permission-common": "workspace:^", "@backstage/plugin-search-common": "workspace:^" diff --git a/yarn.lock b/yarn.lock index d489203875..c3d374b1cd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5742,6 +5742,7 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-catalog-common@workspace:plugins/catalog-common" dependencies: + "@backstage/backend-plugin-api": "workspace:^" "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/plugin-permission-common": "workspace:^" From 1b3a399a760ed169d73a743e4c5316ba7fefbbeb Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Wed, 7 Aug 2024 11:11:21 -0400 Subject: [PATCH 06/17] added credentials field to LocationAnalyzer's analyzeLocation method Signed-off-by: Kashish Mittal --- .../src/service/AuthorizedLocationAnalyzer.ts | 10 +++++++--- plugins/catalog-backend/src/service/createRouter.ts | 4 ++-- plugins/catalog-common/package.json | 1 - .../catalog-common/src/ingestion/LocationAnalyzer.ts | 2 -- plugins/catalog-node/api-report.md | 1 + plugins/catalog-node/src/processing/types.ts | 2 ++ yarn.lock | 1 - 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts index 3968a0ab73..de0cc189b3 100644 --- a/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts +++ b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts @@ -17,7 +17,10 @@ import { NotAllowedError } from '@backstage/errors'; import { catalogLocationCreatePermission } from '@backstage/plugin-catalog-common/alpha'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; -import { PermissionsService } from '@backstage/backend-plugin-api'; +import { + BackstageCredentials, + PermissionsService, +} from '@backstage/backend-plugin-api'; import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; import { AnalyzeLocationRequest } from '@backstage/plugin-catalog-common'; import { AnalyzeLocationResponse } from '@backstage/plugin-catalog-common'; @@ -30,6 +33,7 @@ export class AuthorizedLocationAnalyzer implements LocationAnalyzer { async analyzeLocation( request: AnalyzeLocationRequest, + credentials: BackstageCredentials, ): Promise { const authorizeDecision = ( await this.permissionApi.authorize( @@ -38,12 +42,12 @@ export class AuthorizedLocationAnalyzer implements LocationAnalyzer { permission: catalogLocationCreatePermission, }, ], - { credentials: request.credentials }, + { credentials: credentials }, ) )[0]; if (authorizeDecision.result !== AuthorizeResult.ALLOW) { throw new NotAllowedError(); } - return this.service.analyzeLocation(request); + return this.service.analyzeLocation(request, credentials); } } diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index abba18b0fe..70e8054909 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -303,10 +303,10 @@ export async function createRouter( }); const credentials = await httpAuth.credentials(req); const parsedBody = schema.parse(body); - const analyzeLocationRequest = { ...parsedBody, credentials }; try { const output = await locationAnalyzer.analyzeLocation( - analyzeLocationRequest, + parsedBody, + credentials, ); res.status(200).json(output); } catch (err) { diff --git a/plugins/catalog-common/package.json b/plugins/catalog-common/package.json index e4eee51e7f..cfcfee74d8 100644 --- a/plugins/catalog-common/package.json +++ b/plugins/catalog-common/package.json @@ -56,7 +56,6 @@ "test": "backstage-cli package test" }, "dependencies": { - "@backstage/backend-plugin-api": "workspace:^", "@backstage/catalog-model": "workspace:^", "@backstage/plugin-permission-common": "workspace:^", "@backstage/plugin-search-common": "workspace:^" diff --git a/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts b/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts index 8dd40a015e..08ae5ef0cb 100644 --- a/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts +++ b/plugins/catalog-common/src/ingestion/LocationAnalyzer.ts @@ -17,13 +17,11 @@ import { LocationSpec } from '../common'; import { Entity } from '@backstage/catalog-model'; import { RecursivePartial } from './RecursivePartial'; -import { BackstageCredentials } from '@backstage/backend-plugin-api'; /** @public */ export type AnalyzeLocationRequest = { location: LocationSpec; catalogFilename?: string; - credentials: BackstageCredentials; }; /** @public */ diff --git a/plugins/catalog-node/api-report.md b/plugins/catalog-node/api-report.md index 7c1c8b5911..383f1b5072 100644 --- a/plugins/catalog-node/api-report.md +++ b/plugins/catalog-node/api-report.md @@ -172,6 +172,7 @@ export type EntityRelationSpec = { export type LocationAnalyzer = { analyzeLocation( location: AnalyzeLocationRequest, + credentials: BackstageCredentials ): Promise; }; diff --git a/plugins/catalog-node/src/processing/types.ts b/plugins/catalog-node/src/processing/types.ts index e3d3c305fc..625effc4df 100644 --- a/plugins/catalog-node/src/processing/types.ts +++ b/plugins/catalog-node/src/processing/types.ts @@ -22,6 +22,7 @@ import { } from '@backstage/plugin-catalog-common'; import { JsonValue } from '@backstage/types'; import { CatalogProcessorEmit } from '../api'; +import { BackstageCredentials } from '@backstage/backend-plugin-api'; /** * Entities that are not yet processed. @@ -66,6 +67,7 @@ export type LocationAnalyzer = { */ analyzeLocation( location: AnalyzeLocationRequest, + credentials: BackstageCredentials, ): Promise; }; diff --git a/yarn.lock b/yarn.lock index c3d374b1cd..d489203875 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5742,7 +5742,6 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-catalog-common@workspace:plugins/catalog-common" dependencies: - "@backstage/backend-plugin-api": "workspace:^" "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/plugin-permission-common": "workspace:^" From 02549c68a454c3e64edd37efdf3f09dfa5d6885b Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Wed, 7 Aug 2024 11:38:21 -0400 Subject: [PATCH 07/17] ran yarn build:api-reports Signed-off-by: Kashish Mittal --- plugins/catalog-node/api-report.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-node/api-report.md b/plugins/catalog-node/api-report.md index 383f1b5072..43209aa862 100644 --- a/plugins/catalog-node/api-report.md +++ b/plugins/catalog-node/api-report.md @@ -8,6 +8,7 @@ import { AnalyzeLocationExistingEntity } from '@backstage/plugin-catalog-common'; import { AnalyzeLocationRequest } from '@backstage/plugin-catalog-common'; import { AnalyzeLocationResponse } from '@backstage/plugin-catalog-common'; +import { BackstageCredentials } from '@backstage/backend-plugin-api'; import { CompoundEntityRef } from '@backstage/catalog-model'; import { Entity } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/types'; @@ -172,7 +173,7 @@ export type EntityRelationSpec = { export type LocationAnalyzer = { analyzeLocation( location: AnalyzeLocationRequest, - credentials: BackstageCredentials + credentials: BackstageCredentials, ): Promise; }; From f2ea3de989e244bb3c4c5a8851dbc1a5331eeadf Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Wed, 7 Aug 2024 13:18:25 -0400 Subject: [PATCH 08/17] updated tests for validate-entity Signed-off-by: Kashish Mittal --- plugins/catalog-backend/src/service/createRouter.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 5687bc1f85..57d476ac10 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -742,6 +742,7 @@ 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', @@ -779,6 +780,7 @@ 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', From 9f4132400ec6a32e320f705682bbc3189e398059 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Wed, 7 Aug 2024 15:19:39 -0400 Subject: [PATCH 09/17] modified changeset Signed-off-by: Kashish Mittal --- .changeset/{serious-tables-chew.md => spicy-files-happen.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename .changeset/{serious-tables-chew.md => spicy-files-happen.md} (85%) diff --git a/.changeset/serious-tables-chew.md b/.changeset/spicy-files-happen.md similarity index 85% rename from .changeset/serious-tables-chew.md rename to .changeset/spicy-files-happen.md index cc5a218f3d..a6c12f296e 100644 --- a/.changeset/serious-tables-chew.md +++ b/.changeset/spicy-files-happen.md @@ -1,6 +1,6 @@ --- '@backstage/plugin-catalog-backend': patch -'@backstage/plugin-catalog-common': patch +'@backstage/plugin-catalog-node': patch --- - The `analyze-location` endpoint is now protected by the `catalog.location.create` permission. From e9e86ccbaa149214567bcd7feef9a230b596da6a Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Mon, 12 Aug 2024 10:36:30 -0400 Subject: [PATCH 10/17] added tests Signed-off-by: Kashish Mittal --- ...rizedCatalogProcessingOrchestrator.test.ts | 97 +++++++++++++++++++ .../AuthorizedLocationAnalyzer.test.ts | 86 ++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.test.ts create mode 100644 plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.test.ts diff --git a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.test.ts b/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.test.ts new file mode 100644 index 0000000000..72572b68e7 --- /dev/null +++ b/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.test.ts @@ -0,0 +1,97 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +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'; + +describe('AuthorizedCatalogProcessingOrchestrator', () => { + const orchestratorService = { + process: jest.fn(), + }; + const permissionApi = { + authorize: jest.fn(), + }; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('throws Authorization Error on deny', async () => { + permissionApi.authorize.mockResolvedValueOnce([ + { + result: AuthorizeResult.DENY, + }, + ]); + const authorizedService = new AuthorizedCatalogProcessingOrchestrator( + orchestratorService, + permissionApi as unknown as ServerPermissionClient, + ); + const entityProcessingRequest = { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'your-entity-name', + namespace: 'default', + description: 'your-entity-description', + }, + spec: { + type: 'service', + owner: 'team-a', + }, + }, + credentials: mockCredentials.none(), + }; + await expect(() => + authorizedService.process(entityProcessingRequest), + ).rejects.toThrow(NotAllowedError); + }); + + it('calls process on allow', async () => { + permissionApi.authorize.mockResolvedValueOnce([ + { + result: AuthorizeResult.ALLOW, + }, + ]); + const authorizedService = new AuthorizedCatalogProcessingOrchestrator( + orchestratorService, + permissionApi as unknown as ServerPermissionClient, + ); + const entityProcessingRequest = { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'your-entity-name', + namespace: 'default', + description: 'your-entity-description', + }, + spec: { + type: 'service', + owner: 'team-a', + }, + }, + credentials: mockCredentials.none(), + }; + await authorizedService.process(entityProcessingRequest); + expect(orchestratorService.process).toHaveBeenCalledWith( + entityProcessingRequest, + ); + }); +}); diff --git a/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.test.ts b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.test.ts new file mode 100644 index 0000000000..1583049080 --- /dev/null +++ b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.test.ts @@ -0,0 +1,86 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NotAllowedError } from '@backstage/errors'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { AuthorizedLocationAnalyzer } from './AuthorizedLocationAnalyzer'; +import { ServerPermissionClient } from '@backstage/plugin-permission-node'; +import { mockCredentials } from '@backstage/backend-test-utils'; + +describe('AuthorizedLocationAnalyzer', () => { + const locationAnalyzerService = { + analyzeLocation: jest.fn(), + }; + const permissionApi = { + authorize: jest.fn(), + }; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('throws Authorization Error on deny', async () => { + permissionApi.authorize.mockResolvedValueOnce([ + { + result: AuthorizeResult.DENY, + }, + ]); + const authorizedService = new AuthorizedLocationAnalyzer( + locationAnalyzerService, + permissionApi as unknown as ServerPermissionClient, + ); + await expect(() => + authorizedService.analyzeLocation( + { + location: { + type: 'url', + target: 'https://example.com/path/to/your/catalog-info.yaml', + presence: 'required', + }, + catalogFilename: 'catalog-info.yaml', + }, + mockCredentials.none(), + ), + ).rejects.toThrow(NotAllowedError); + }); + + it('calls analyzeLocation on allow', async () => { + permissionApi.authorize.mockResolvedValueOnce([ + { + result: AuthorizeResult.ALLOW, + }, + ]); + const authorizedService = new AuthorizedLocationAnalyzer( + locationAnalyzerService, + permissionApi as unknown as ServerPermissionClient, + ); + const analyzeLocationRequest = { + location: { + type: 'url', + target: 'https://example.com/path/to/your/catalog-info.yaml', + }, + catalogFilename: 'catalog-info.yaml', + }; + await authorizedService.analyzeLocation( + analyzeLocationRequest, + mockCredentials.none(), + ); + expect(locationAnalyzerService.analyzeLocation).toHaveBeenCalledWith( + analyzeLocationRequest, + mockCredentials.none(), + ); + }); +}); From fb57bb85a1ed7004345a1fc6a0bc06514e703789 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 20 Aug 2024 15:10:37 -0400 Subject: [PATCH 11/17] added catalog.location.analyze and catalog.entity.validate permissions Signed-off-by: Kashish Mittal --- ...AuthorizedCatalogProcessingOrchestrator.ts | 4 ++-- .../src/service/AuthorizedLocationAnalyzer.ts | 4 ++-- plugins/catalog-common/src/alpha.ts | 2 ++ plugins/catalog-common/src/permissions.ts | 20 +++++++++++++++++++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts b/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts index e4657f01ba..76fad3b33c 100644 --- a/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts +++ b/plugins/catalog-backend/src/service/AuthorizedCatalogProcessingOrchestrator.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { NotAllowedError } from '@backstage/errors'; -import { catalogEntityCreatePermission } from '@backstage/plugin-catalog-common/alpha'; +import { catalogEntityValidatePermission } from '@backstage/plugin-catalog-common/alpha'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { CatalogProcessingOrchestrator, @@ -47,7 +47,7 @@ export class AuthorizedCatalogProcessingOrchestrator await this.permissionApi.authorize( [ { - permission: catalogEntityCreatePermission, + permission: catalogEntityValidatePermission, }, ], { credentials }, diff --git a/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts index de0cc189b3..dc9af81dcd 100644 --- a/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts +++ b/plugins/catalog-backend/src/service/AuthorizedLocationAnalyzer.ts @@ -15,7 +15,7 @@ */ import { NotAllowedError } from '@backstage/errors'; -import { catalogLocationCreatePermission } from '@backstage/plugin-catalog-common/alpha'; +import { catalogLocationAnalyzePermission } from '@backstage/plugin-catalog-common/alpha'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { BackstageCredentials, @@ -39,7 +39,7 @@ export class AuthorizedLocationAnalyzer implements LocationAnalyzer { await this.permissionApi.authorize( [ { - permission: catalogLocationCreatePermission, + permission: catalogLocationAnalyzePermission, }, ], { credentials: credentials }, diff --git a/plugins/catalog-common/src/alpha.ts b/plugins/catalog-common/src/alpha.ts index 2dfc028332..8a3ed17431 100644 --- a/plugins/catalog-common/src/alpha.ts +++ b/plugins/catalog-common/src/alpha.ts @@ -20,9 +20,11 @@ export { catalogEntityCreatePermission, catalogEntityDeletePermission, catalogEntityRefreshPermission, + catalogEntityValidatePermission, catalogLocationReadPermission, catalogLocationCreatePermission, catalogLocationDeletePermission, + catalogLocationAnalyzePermission, catalogPermissions, } from './permissions'; export type { CatalogEntityPermission } from './permissions'; diff --git a/plugins/catalog-common/src/permissions.ts b/plugins/catalog-common/src/permissions.ts index 3082c0da70..fa86268f61 100644 --- a/plugins/catalog-common/src/permissions.ts +++ b/plugins/catalog-common/src/permissions.ts @@ -91,6 +91,15 @@ export const catalogEntityRefreshPermission = createPermission({ resourceType: RESOURCE_TYPE_CATALOG_ENTITY, }); +/** + * This permission is used to authorize validating catalog entities. + * @alpha + */ +export const catalogEntityValidatePermission = createPermission({ + name: 'catalog.entity.validate', + attributes: {}, +}); + /** * This permission is used to designate actions that involve reading one or more * locations from the catalog. @@ -118,6 +127,15 @@ export const catalogLocationCreatePermission = createPermission({ }, }); +/** + * This permission is used to authorize analyzing catalog locations. + * @alpha + */ +export const catalogLocationAnalyzePermission = createPermission({ + name: 'catalog.location.analyze', + attributes: {}, +}); + /** * This permission is used to designate actions that involve deleting locations * from the catalog. @@ -139,7 +157,9 @@ export const catalogPermissions = [ catalogEntityCreatePermission, catalogEntityDeletePermission, catalogEntityRefreshPermission, + catalogEntityValidatePermission, catalogLocationReadPermission, catalogLocationCreatePermission, catalogLocationDeletePermission, + catalogLocationAnalyzePermission, ]; From bd35cdbf36a560937df8ef6a93d32ae263958ce8 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 20 Aug 2024 15:16:54 -0400 Subject: [PATCH 12/17] added changeset Signed-off-by: Kashish Mittal --- .changeset/lucky-zoos-camp.md | 8 ++++++++ .changeset/spicy-files-happen.md | 7 ------- 2 files changed, 8 insertions(+), 7 deletions(-) create mode 100644 .changeset/lucky-zoos-camp.md delete mode 100644 .changeset/spicy-files-happen.md diff --git a/.changeset/lucky-zoos-camp.md b/.changeset/lucky-zoos-camp.md new file mode 100644 index 0000000000..e99e8dc7f3 --- /dev/null +++ b/.changeset/lucky-zoos-camp.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-catalog-backend': minor +'@backstage/plugin-catalog-common': minor +'@backstage/plugin-catalog-node': minor +--- + +The `analyze-location` endpoint is now protected by the `catalog.location.analyze` permission. +The `validate-entity` endpoint is now protected by the `catalog.entity.validate` permission. diff --git a/.changeset/spicy-files-happen.md b/.changeset/spicy-files-happen.md deleted file mode 100644 index a6c12f296e..0000000000 --- a/.changeset/spicy-files-happen.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -'@backstage/plugin-catalog-backend': patch -'@backstage/plugin-catalog-node': patch ---- - -- The `analyze-location` endpoint is now protected by the `catalog.location.create` permission. -- The `validate-entity` endpoint is now protected by the `catalog.entity.create` permission. From 9db2da19033527c2aed86c7eb728b0408d2fec83 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 20 Aug 2024 15:33:14 -0400 Subject: [PATCH 13/17] ran yarn prettier:fix Signed-off-by: Kashish Mittal --- beps/00010-event-auditor/README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/beps/00010-event-auditor/README.md b/beps/00010-event-auditor/README.md index 3bf7eb2e0d..44006599eb 100644 --- a/beps/00010-event-auditor/README.md +++ b/beps/00010-event-auditor/README.md @@ -115,12 +115,16 @@ export type AuditEventFailureStatus = { errors: E[]; }; -export type AuditEventStatus = AuditEventSuccessStatus | AuditEventFailureStatus | undefined; +export type AuditEventStatus = + | AuditEventSuccessStatus + | AuditEventFailureStatus + | undefined; ``` #### EventAuditor Interface This interface defines the functionalities of an `EventAuditor` class. This class provides methods for: + - Extracting the actor ID from an Express request (if available). - Creating detailed audit event information based on provided options. - Logging an audit event with a specific level (info, debug, warn, or error). @@ -139,7 +143,7 @@ export type AuditEventOptions = AuditEventStatus & { metadata?: JsonValue; response?: AuditResponse; request?: Request; -} & ({ actorId: string; } | { credentials: BackstageCredentials } | undefined); +} & ({ actorId: string } | { credentials: BackstageCredentials } | undefined); export type AuditEvent = { actor: ActorDetails; From c35a7c76f5bae2b15fe765e1408437970ddd82ae Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 20 Aug 2024 15:51:09 -0400 Subject: [PATCH 14/17] ran yarn build:api-reports Signed-off-by: Kashish Mittal --- plugins/catalog-common/api-report-alpha.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/plugins/catalog-common/api-report-alpha.md b/plugins/catalog-common/api-report-alpha.md index 12e0806a9c..86e606dd84 100644 --- a/plugins/catalog-common/api-report-alpha.md +++ b/plugins/catalog-common/api-report-alpha.md @@ -23,6 +23,12 @@ export const catalogEntityReadPermission: ResourcePermission<'catalog-entity'>; // @alpha export const catalogEntityRefreshPermission: ResourcePermission<'catalog-entity'>; +// @alpha +export const catalogEntityValidatePermission: BasicPermission; + +// @alpha +export const catalogLocationAnalyzePermission: BasicPermission; + // @alpha export const catalogLocationCreatePermission: BasicPermission; From fee43bd0732516181b5e699e02150cf086d2ad62 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 29 Aug 2024 10:16:22 -0400 Subject: [PATCH 15/17] removed backstage/backend-plugin-api from dependencies Signed-off-by: Kashish Mittal --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 309ae364fe..fe87d18027 100644 --- a/package.json +++ b/package.json @@ -92,7 +92,6 @@ "jest-haste-map@^29.7.0": "patch:jest-haste-map@npm%3A29.7.0#./.yarn/patches/jest-haste-map-npm-29.7.0-e3be419eff.patch" }, "dependencies": { - "@backstage/backend-plugin-api": "workspace:^", "@backstage/errors": "workspace:^", "@manypkg/get-packages": "^1.1.3", "@types/global-agent": "^2.1.3", From e59fa210c6f91d0680160d0ce145b11b48b2bd8e Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 29 Aug 2024 16:07:58 -0400 Subject: [PATCH 16/17] regen yarn.lock Signed-off-by: Kashish Mittal --- yarn.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 5fdc956ccc..c36b6da828 100644 --- a/yarn.lock +++ b/yarn.lock @@ -39912,7 +39912,6 @@ __metadata: version: 0.0.0-use.local resolution: "root@workspace:." dependencies: - "@backstage/backend-plugin-api": "workspace:^" "@backstage/cli": "workspace:*" "@backstage/codemods": "workspace:*" "@backstage/create-app": "workspace:*" From 7c837e21abe4006efc6ba7d028dd82106646a254 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Tue, 10 Sep 2024 10:36:00 -0400 Subject: [PATCH 17/17] handle CatalogProcessingOrchestrator permissions check at router level Signed-off-by: Kashish Mittal --- .../catalog-backend/src/processing/types.ts | 2 -- ...ts => AuthorizedValidationService.test.ts} | 20 +++++++----- ...ator.ts => AuthorizedValidationService.ts} | 14 +++----- .../src/service/CatalogBuilder.ts | 23 ++++++------- .../src/service/createRouter.test.ts | 32 +++++++++++++++++-- .../src/service/createRouter.ts | 32 ++++++++++++------- 6 files changed, 77 insertions(+), 46 deletions(-) rename plugins/catalog-backend/src/service/{AuthorizedCatalogProcessingOrchestrator.test.ts => AuthorizedValidationService.test.ts} (82%) rename plugins/catalog-backend/src/service/{AuthorizedCatalogProcessingOrchestrator.ts => AuthorizedValidationService.ts} (86%) 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({