From 6f8b6c7069fcbc2154ac725284df73a0c7251fc0 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 11 Jan 2023 11:17:50 +0100 Subject: [PATCH 01/55] scaffolder: add permissions Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-common/package.json | 1 + plugins/scaffolder-common/src/index.ts | 1 + plugins/scaffolder-common/src/permissions.ts | 29 ++++++++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 plugins/scaffolder-common/src/permissions.ts diff --git a/plugins/scaffolder-common/package.json b/plugins/scaffolder-common/package.json index c08763d98b..d315754462 100644 --- a/plugins/scaffolder-common/package.json +++ b/plugins/scaffolder-common/package.json @@ -39,6 +39,7 @@ }, "dependencies": { "@backstage/catalog-model": "workspace:^", + "@backstage/plugin-permission-common": "workspace:^", "@backstage/types": "workspace:^" }, "devDependencies": { diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index 7f9792a1ba..4953a27f9e 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -25,4 +25,5 @@ export { templateEntityV1beta3Validator, isTemplateEntityV1beta3, } from './TemplateEntityV1beta3'; +export * from './permissions'; export type { TemplateEntityV1beta3 } from './TemplateEntityV1beta3'; diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts new file mode 100644 index 0000000000..578ef09311 --- /dev/null +++ b/plugins/scaffolder-common/src/permissions.ts @@ -0,0 +1,29 @@ +/* + * Copyright 2022 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 { createPermission } from '@backstage/plugin-permission-common'; + +export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-field'; + +export const templateSchemaExecutePermission = createPermission({ + name: 'scaffolder.template.schema.execute', + attributes: { + action: 'create', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, +}); + +export const scaffolderPermissions = [templateSchemaExecutePermission]; From 2d34af80efd461485dfac362e71aebee6064042e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 11 Jan 2023 11:18:22 +0100 Subject: [PATCH 02/55] scaffolder: add conditional rules Signed-off-by: Vincenzo Scamporlino --- .../src/service/conditionExports.ts | 47 ++++++++ .../scaffolder-backend/src/service/rules.ts | 113 ++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 plugins/scaffolder-backend/src/service/conditionExports.ts create mode 100644 plugins/scaffolder-backend/src/service/rules.ts diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts new file mode 100644 index 0000000000..d627284e76 --- /dev/null +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -0,0 +1,47 @@ +/* + * Copyright 2022 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 { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common'; +import { createConditionExports } from '@backstage/plugin-permission-node'; +import { scaffolderRules } from './rules'; +import { + AllOfCriteria, + ConditionalPolicyDecision, + PermissionCondition, + PermissionRuleParams, + ResourcePermission, +} from '@backstage/plugin-permission-common'; + +const { conditions: scaffolderConditions, createConditionalDecision } = + createConditionExports({ + pluginId: 'scaffolder', + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + rules: scaffolderRules, + }); + +export { scaffolderConditions }; + +export function createScaffolderConditionalDecision( + permission: ResourcePermission, + conditions: AllOfCriteria< + PermissionCondition< + typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + PermissionRuleParams + > + >['allOf'], +): ConditionalPolicyDecision { + return createConditionalDecision(permission, { allOf: conditions }); +} diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts new file mode 100644 index 0000000000..11c938cab4 --- /dev/null +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -0,0 +1,113 @@ +/* + * Copyright 2022 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 { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; +import { + RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + TemplateEntityV1beta3, +} from '@backstage/plugin-scaffolder-common'; +import { z } from 'zod'; + +import get from 'lodash/get'; +import unset from 'lodash/unset'; +import { TemplateTransform } from './helpers'; +import { + SecureTemplater, + SecureTemplateRenderer, +} from '../lib/templating/SecureTemplater'; + +export const createScaffolderPermissionRule = makeCreatePermissionRule< + TemplateEntityV1beta3, + TemplateTransform, + typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE +>(); + +let render: SecureTemplateRenderer; + +export const allowCapabilities = createScaffolderPermissionRule({ + name: 'ALLOW_CAPABILITIES', + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + description: 'Allow capabilities based on permissions', + paramsSchema: z.object({ + capabilities: z.array(z.string()), + templateRef: z.string().optional(), + }), + apply: () => true, + toQuery: + ({ capabilities }) => + template => { + const capabilitiesObj = capabilities.reduce((acc, c) => { + acc[c] = true; + return acc; + }, {} as Record); + + const parametersPath = ['spec', 'parameters']; + const capabilitiesKey = 'backstage:capabilities'; + + if (Array.isArray(template.spec.parameters)) { + template.spec.parameters.forEach((parameter, index) => { + const parameterPath = [...parametersPath, index.toString()]; + stripParams(parameterPath); + if (parameter.properties) { + Object.keys(parameter.properties).forEach(p => { + if (stripParams([...parameterPath, 'properties', p])) { + const requiredProperties = get(template, [ + ...parameterPath, + 'required', + ]); + if (Array.isArray(requiredProperties)) { + requiredProperties.splice(requiredProperties.indexOf(p), 1); + } + } + }); + } + }); + } else { + stripParams(parametersPath); + } + + template.spec.steps.forEach((_p, index) => + stripParams(['spec', 'steps', index.toString()]), + ); + + function stripParams(path: string[]) { + const jsonObject = get(template, path); + + if ( + typeof jsonObject[capabilitiesKey] === 'string' && + render(jsonObject[capabilitiesKey], { + capabilities: capabilitiesObj, + }) !== 'true' + ) { + const parent = get(template, [...path].splice(0, path.length - 1)); + if (Array.isArray(parent)) { + parent.splice(parent.indexOf(jsonObject), 1); + } else { + unset(template, path); + } + + return path[path.length - 1]; + } + return undefined; + } + + return template; + }, +}); + +SecureTemplater.loadRenderer().then(r => (render = r)); + +export const scaffolderRules = { allowCapabilities }; From 1424f89d4dc05697dd86c5e4dca0bbca2412878b Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 11 Jan 2023 11:19:28 +0100 Subject: [PATCH 03/55] scaffolder: authorize permissions Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/scaffolder.ts | 1 + plugins/scaffolder-backend/package.json | 2 + plugins/scaffolder-backend/src/index.ts | 1 + .../scaffolder-backend/src/service/helpers.ts | 4 + .../src/service/router.test.ts | 13 ++ .../scaffolder-backend/src/service/router.ts | 126 ++++++++++++------ 6 files changed, 108 insertions(+), 39 deletions(-) diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index d079b64c28..813092c9b9 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -34,5 +34,6 @@ export default async function createPlugin( reader: env.reader, identity: env.identity, scheduler: env.scheduler, + permissionApi: env.permissions, }); } diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 336f5d1286..ba72d9e071 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -58,6 +58,8 @@ "@backstage/plugin-catalog-backend": "workspace:^", "@backstage/plugin-catalog-common": "workspace:^", "@backstage/plugin-catalog-node": "workspace:^", + "@backstage/plugin-permission-common": "workspace:^", + "@backstage/plugin-permission-node": "workspace:^", "@backstage/plugin-scaffolder-common": "workspace:^", "@backstage/plugin-scaffolder-node": "workspace:^", "@backstage/types": "workspace:^", diff --git a/plugins/scaffolder-backend/src/index.ts b/plugins/scaffolder-backend/src/index.ts index ba2c25f80a..3dc91f18b9 100644 --- a/plugins/scaffolder-backend/src/index.ts +++ b/plugins/scaffolder-backend/src/index.ts @@ -21,6 +21,7 @@ */ export * from './scaffolder'; +export * from './service/conditionExports'; export * from './service/router'; export * from './lib'; export * from './processor'; diff --git a/plugins/scaffolder-backend/src/service/helpers.ts b/plugins/scaffolder-backend/src/service/helpers.ts index 3412fec2d3..97d4ab93aa 100644 --- a/plugins/scaffolder-backend/src/service/helpers.ts +++ b/plugins/scaffolder-backend/src/service/helpers.ts @@ -107,3 +107,7 @@ export async function findTemplate(options: { return template as TemplateEntityV1beta3; } + +export type TemplateTransform = ( + template: TemplateEntityV1beta3, +) => TemplateEntityV1beta3; diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 5e3c1c7278..6ebe988e9f 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -44,6 +44,7 @@ import { IdentityApiGetIdentityRequest, BackstageIdentityResponse, } from '@backstage/plugin-auth-node'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; const mockAccess = jest.fn(); @@ -139,6 +140,11 @@ describe('createRouter', () => { }); taskBroker = new StorageTaskBroker(databaseTaskStore, logger); + const permissionApi: PermissionEvaluator = { + authorize: jest.fn(), + authorizeConditional: jest.fn(), + }; + jest.spyOn(taskBroker, 'dispatch'); jest.spyOn(taskBroker, 'get'); jest.spyOn(taskBroker, 'list'); @@ -152,6 +158,7 @@ describe('createRouter', () => { catalogClient, reader: mockUrlReader, taskBroker, + permissionApi, }); app = express().use(router); @@ -744,6 +751,11 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }, ); + const permissionApi: PermissionEvaluator = { + authorize: jest.fn(), + authorizeConditional: jest.fn(), + }; + const router = await createRouter({ logger: logger, config: new ConfigReader({}), @@ -752,6 +764,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ reader: mockUrlReader, taskBroker, identity: { getIdentity }, + permissionApi, }); app = express().use(router); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index b2be3e2749..9fe7a98a8a 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -18,19 +18,26 @@ import { PluginDatabaseManager, UrlReader } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; import { CatalogApi } from '@backstage/catalog-client'; import { + CompoundEntityRef, Entity, parseEntityRef, stringifyEntityRef, UserEntity, } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; +import { + InputError, + NotAllowedError, + NotFoundError, + stringifyError, +} from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { JsonObject, JsonValue } from '@backstage/types'; import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, + templateSchemaExecutePermission, } from '@backstage/plugin-scaffolder-common'; import express from 'express'; import Router from 'express-promise-router'; @@ -47,12 +54,27 @@ import { } from '../scaffolder'; import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; -import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; +import { + findTemplate, + getEntityBaseUrl, + getWorkingDirectory, + TemplateTransform, +} from './helpers'; import { IdentityApi, IdentityApiGetIdentityRequest, } from '@backstage/plugin-auth-node'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; +import { + AuthorizeResult, + PermissionEvaluator, +} from '@backstage/plugin-permission-common'; +import { + ConditionTransformer, + createConditionTransformer, + isAndCriteria, +} from '@backstage/plugin-permission-node'; +import { scaffolderRules } from './rules'; /** * RouterOptions @@ -80,6 +102,7 @@ export interface RouterOptions { taskBroker?: TaskBroker; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; + permissionApi: PermissionEvaluator; identity?: IdentityApi; } @@ -174,6 +197,7 @@ export async function createRouter( scheduler, additionalTemplateFilters, additionalTemplateGlobals, + permissionApi, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); @@ -249,41 +273,31 @@ export async function createRouter( additionalTemplateGlobals, }); + const transformConditions: ConditionTransformer = + createConditionTransformer(Object.values(scaffolderRules)); + router .get( '/v2/templates/:namespace/:kind/:name/parameter-schema', async (req, res) => { - const { namespace, kind, name } = req.params; - const userIdentity = await identity.getIdentity({ request: req, }); const token = userIdentity?.token; - const template = await findTemplate({ - catalogApi: catalogClient, - entityRef: { kind, namespace, name }, - token, + const template = await authorizeTemplate(req.params, token); + + const parameters = [template.spec.parameters ?? []].flat(); + res.json({ + title: template.metadata.title ?? template.metadata.name, + description: template.metadata.description, + 'ui:options': template.metadata['ui:options'], + steps: parameters.map(schema => ({ + title: schema.title ?? 'Please enter the following information', + description: schema.description, + schema, + })), }); - if (isSupportedTemplate(template)) { - const parameters = [template.spec.parameters ?? []].flat(); - res.json({ - title: template.metadata.title ?? template.metadata.name, - description: template.metadata.description, - 'ui:options': template.metadata['ui:options'], - steps: parameters.map(schema => ({ - title: schema.title ?? 'Please enter the following information', - description: schema.description, - schema, - })), - }); - } else { - throw new InputError( - `Unsupported apiVersion field in schema entity, ${ - (template as Entity).apiVersion - }`, - ); - } }, ) .get('/v2/actions', async (_req, res) => { @@ -321,19 +335,10 @@ export async function createRouter( const values = req.body.values; - const template = await findTemplate({ - catalogApi: catalogClient, - entityRef: { kind, namespace, name }, + const template = await authorizeTemplate( + { kind, namespace, name }, token, - }); - - if (!isSupportedTemplate(template)) { - throw new InputError( - `Unsupported apiVersion field in schema entity, ${ - (template as Entity).apiVersion - }`, - ); - } + ); for (const parameters of [template.spec.parameters ?? []].flat()) { const result = validate(values, parameters); @@ -567,5 +572,48 @@ export async function createRouter( app.set('logger', logger); app.use('/', router); + async function authorizeTemplate( + entityRef: CompoundEntityRef, + token: string | undefined, + ) { + let template = await findTemplate({ + catalogApi: catalogClient, + entityRef, + token, + }); + if (!isSupportedTemplate(template)) { + throw new InputError( + `Unsupported apiVersion field in schema entity, ${ + (template as Entity).apiVersion + }`, + ); + } + const authorizeDecision = ( + await permissionApi.authorizeConditional( + [{ permission: templateSchemaExecutePermission }], + { + token, + }, + ) + )[0]; + + if (authorizeDecision.result === AuthorizeResult.DENY) { + throw new NotAllowedError( + `Not allowed to execute template ${entityRef.kind}:${entityRef.namespace}/${entityRef.name}`, + ); + } + if (authorizeDecision.result === AuthorizeResult.CONDITIONAL) { + const scaffolderFilter = transformConditions( + authorizeDecision.conditions, + ); + if (isAndCriteria(scaffolderFilter)) { + template = scaffolderFilter.allOf.reduce( + (acc, filter) => (filter as TemplateTransform)(acc), + template, + ); + } + } + return template; + } return app; } From 766298715c70418114c3b1d17ee49237fbde56eb Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 11 Jan 2023 11:21:13 +0100 Subject: [PATCH 04/55] backend: permission policy scaffolder example Signed-off-by: Vincenzo Scamporlino --- packages/backend/package.json | 1 + packages/backend/src/plugins/permission.ts | 21 +++++++++++++-------- yarn.lock | 4 ++++ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index 54dbe640df..aa4e660fdd 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -60,6 +60,7 @@ "@backstage/plugin-rollbar-backend": "workspace:^", "@backstage/plugin-scaffolder-backend": "workspace:^", "@backstage/plugin-scaffolder-backend-module-rails": "workspace:^", + "@backstage/plugin-scaffolder-common": "workspace:^", "@backstage/plugin-search-backend": "workspace:^", "@backstage/plugin-search-backend-module-elasticsearch": "workspace:^", "@backstage/plugin-search-backend-module-pg": "workspace:^", diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index 7192a1ddec..708f342265 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -18,28 +18,33 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { createRouter } from '@backstage/plugin-permission-backend'; import { AuthorizeResult, + isPermission, PolicyDecision, } from '@backstage/plugin-permission-common'; import { PermissionPolicy, PolicyQuery, } from '@backstage/plugin-permission-node'; + import { - DefaultPlaylistPermissionPolicy, - isPlaylistPermission, -} from '@backstage/plugin-playlist-backend'; + createScaffolderConditionalDecision, + scaffolderConditions, +} from '@backstage/plugin-scaffolder-backend'; +import { templateSchemaExecutePermission } from '@backstage/plugin-scaffolder-common'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; class ExamplePermissionPolicy implements PermissionPolicy { - private playlistPermissionPolicy = new DefaultPlaylistPermissionPolicy(); - async handle( request: PolicyQuery, - user?: BackstageIdentityResponse, + _user?: BackstageIdentityResponse, ): Promise { - if (isPlaylistPermission(request.permission)) { - return this.playlistPermissionPolicy.handle(request, user); + if (isPermission(request.permission, templateSchemaExecutePermission)) { + return createScaffolderConditionalDecision(request.permission, [ + scaffolderConditions.allowCapabilities({ + capabilities: ['example'], + }), + ]); } return { diff --git a/yarn.lock b/yarn.lock index 542918e285..0d63604827 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7585,6 +7585,8 @@ __metadata: "@backstage/plugin-catalog-backend": "workspace:^" "@backstage/plugin-catalog-common": "workspace:^" "@backstage/plugin-catalog-node": "workspace:^" + "@backstage/plugin-permission-common": "workspace:^" + "@backstage/plugin-permission-node": "workspace:^" "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/plugin-scaffolder-node": "workspace:^" "@backstage/types": "workspace:^" @@ -7644,6 +7646,7 @@ __metadata: dependencies: "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^" + "@backstage/plugin-permission-common": "workspace:^" "@backstage/types": "workspace:^" languageName: unknown linkType: soft @@ -22940,6 +22943,7 @@ __metadata: "@backstage/plugin-rollbar-backend": "workspace:^" "@backstage/plugin-scaffolder-backend": "workspace:^" "@backstage/plugin-scaffolder-backend-module-rails": "workspace:^" + "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/plugin-search-backend": "workspace:^" "@backstage/plugin-search-backend-module-elasticsearch": "workspace:^" "@backstage/plugin-search-backend-module-pg": "workspace:^" From f8540c1049b6dc333565364adeddc6f126048b1d Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 11 Jan 2023 11:22:46 +0100 Subject: [PATCH 05/55] backend: add example of template with capabilities Signed-off-by: Vincenzo Scamporlino --- template.yaml | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 template.yaml diff --git a/template.yaml b/template.yaml new file mode 100644 index 0000000000..0bb67735a9 --- /dev/null +++ b/template.yaml @@ -0,0 +1,56 @@ +apiVersion: scaffolder.backstage.io/v1beta3 +kind: Template +metadata: + name: my_custom_template + title: My custom template + description: Just testing +spec: + owner: web@example.com + type: website + parameters: + - title: Provide some simple information + required: + - component_id + - owner + properties: + component_id: + backstage:capabilities: ${{ capabilities.cap1 }} + title: Name + type: string + description: Unique name of the component + ui:field: EntityNamePicker + description: + title: Description + type: string + description: Help others understand what this website is for. + owner: + title: Owner + type: string + description: Owner of the component + ui:field: OwnerPicker + ui:options: + allowedKinds: + - Group + - title: Choose a location + required: + - repoUrl + properties: + repoUrl: + title: Repository Location + type: string + ui:field: RepoUrlPicker + ui:options: + allowedHosts: + - github.com + steps: + - id: one + backstage:capabilities: ${{ capabilities.cap1 }} + name: First log + action: debug:log + input: + message: hello + - id: two + name: Second log + action: debug:log + input: + message: world From 277847e064b2911902f64e586bebf11b0644929c Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Thu, 12 Jan 2023 12:11:18 +0000 Subject: [PATCH 06/55] Reworked permission rules and filtering to follow similar pattern to catalog Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- app-config.yaml | 3 + packages/backend/src/plugins/permission.ts | 24 ++-- .../createPermissionIntegrationRouter.ts | 2 +- .../permission-node/src/integration/index.ts | 7 +- .../src/service/conditionExports.ts | 62 +++++----- .../scaffolder-backend/src/service/router.ts | 55 +++------ .../scaffolder-backend/src/service/rules.ts | 106 ++++-------------- .../src/TemplateEntityV1beta3.ts | 22 ++-- plugins/scaffolder-common/src/index.ts | 5 +- plugins/scaffolder-common/src/permissions.ts | 40 ++++++- template.yaml | 5 +- 11 files changed, 153 insertions(+), 178 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 385c9c2e0e..d886d86896 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -223,6 +223,7 @@ catalog: - System - Domain - Location + - Template processors: ldapOrg: @@ -279,6 +280,8 @@ catalog: # Backstage end-to-end tests of TechDocs - type: file target: ../../cypress/e2e-fixture.catalog.info.yaml + - type: file + target: ../../template.yaml scaffolder: # Use to customize default commit author info used when new components are created # defaultAuthor: diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index 708f342265..84a102d6be 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -18,7 +18,7 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { createRouter } from '@backstage/plugin-permission-backend'; import { AuthorizeResult, - isPermission, + isResourcePermission, PolicyDecision, } from '@backstage/plugin-permission-common'; import { @@ -27,10 +27,10 @@ import { } from '@backstage/plugin-permission-node'; import { - createScaffolderConditionalDecision, - scaffolderConditions, + createScaffolderStepConditionalDecision, + scaffolderStepConditions, } from '@backstage/plugin-scaffolder-backend'; -import { templateSchemaExecutePermission } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_STEP } from '@backstage/plugin-scaffolder-common'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; @@ -39,12 +39,16 @@ class ExamplePermissionPolicy implements PermissionPolicy { request: PolicyQuery, _user?: BackstageIdentityResponse, ): Promise { - if (isPermission(request.permission, templateSchemaExecutePermission)) { - return createScaffolderConditionalDecision(request.permission, [ - scaffolderConditions.allowCapabilities({ - capabilities: ['example'], - }), - ]); + /** + * This is an example of how to use the scaffolder step conditions. + */ + if ( + isResourcePermission(request.permission, RESOURCE_TYPE_SCAFFOLDER_STEP) + ) { + return createScaffolderStepConditionalDecision( + request.permission, + scaffolderStepConditions.hasTag({ tag: 'example' }), + ); } return { diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 087de23198..e3f4b33707 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -126,7 +126,7 @@ export type MetadataResponse = { rules: MetadataResponseSerializedRule[]; }; -const applyConditions = ( +export const applyConditions = ( criteria: PermissionCriteria>, resource: TResource | undefined, getRule: (name: string) => PermissionRule, diff --git a/plugins/permission-node/src/integration/index.ts b/plugins/permission-node/src/integration/index.ts index 7702fea95b..f3b0edad9b 100644 --- a/plugins/permission-node/src/integration/index.ts +++ b/plugins/permission-node/src/integration/index.ts @@ -19,4 +19,9 @@ export * from './createConditionExports'; export * from './createConditionTransformer'; export * from './createPermissionIntegrationRouter'; export * from './createPermissionRule'; -export { isAndCriteria, isOrCriteria, isNotCriteria } from './util'; +export { + createGetRule, + isAndCriteria, + isOrCriteria, + isNotCriteria, +} from './util'; diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index d627284e76..2a7c0d382f 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -14,34 +14,42 @@ * limitations under the License. */ -import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_STEP } from '@backstage/plugin-scaffolder-common'; import { createConditionExports } from '@backstage/plugin-permission-node'; -import { scaffolderRules } from './rules'; -import { - AllOfCriteria, - ConditionalPolicyDecision, - PermissionCondition, - PermissionRuleParams, - ResourcePermission, -} from '@backstage/plugin-permission-common'; +import { scaffolderStepRules } from './rules'; -const { conditions: scaffolderConditions, createConditionalDecision } = - createConditionExports({ - pluginId: 'scaffolder', - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - rules: scaffolderRules, - }); +// const { +// conditions: scaffolderTemplateConditions, +// createConditionalDecision: createScaffolderTemplateConditionalDecision, +// } = createConditionExports({ +// pluginId: 'scaffolder', +// resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, +// rules: scaffolderRules, +// }); -export { scaffolderConditions }; +// const { +// conditions: scaffolderPropertyConditions, +// createConditionalDecision: createScaffolderPropertyConditionalDecision, +// } = createConditionExports({ +// pluginId: 'scaffolder', +// resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, +// rules: scaffolderRules, +// }); -export function createScaffolderConditionalDecision( - permission: ResourcePermission, - conditions: AllOfCriteria< - PermissionCondition< - typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - PermissionRuleParams - > - >['allOf'], -): ConditionalPolicyDecision { - return createConditionalDecision(permission, { allOf: conditions }); -} +const { + conditions: scaffolderStepConditions, + createConditionalDecision: createScaffolderStepConditionalDecision, +} = createConditionExports({ + pluginId: 'scaffolder', + resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, + rules: scaffolderStepRules, +}); + +export { + // scaffolderTemplateConditions, + // createScaffolderTemplateConditionalDecision, + // scaffolderPropertyConditions, + // createScaffolderPropertyConditionalDecision, + scaffolderStepConditions, + createScaffolderStepConditionalDecision, +}; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 9fe7a98a8a..d5a9e17601 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -25,19 +25,14 @@ import { UserEntity, } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { - InputError, - NotAllowedError, - NotFoundError, - stringifyError, -} from '@backstage/errors'; +import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { JsonObject, JsonValue } from '@backstage/types'; import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, - templateSchemaExecutePermission, + templateStepReadPermission, } from '@backstage/plugin-scaffolder-common'; import express from 'express'; import Router from 'express-promise-router'; @@ -54,12 +49,7 @@ import { } from '../scaffolder'; import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; -import { - findTemplate, - getEntityBaseUrl, - getWorkingDirectory, - TemplateTransform, -} from './helpers'; +import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; import { IdentityApi, IdentityApiGetIdentityRequest, @@ -70,11 +60,10 @@ import { PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { - ConditionTransformer, - createConditionTransformer, - isAndCriteria, + applyConditions, + createGetRule, } from '@backstage/plugin-permission-node'; -import { scaffolderRules } from './rules'; +import { scaffolderStepRules } from './rules'; /** * RouterOptions @@ -273,8 +262,7 @@ export async function createRouter( additionalTemplateGlobals, }); - const transformConditions: ConditionTransformer = - createConditionTransformer(Object.values(scaffolderRules)); + const getRule = createGetRule(Object.values(scaffolderStepRules)); router .get( @@ -576,11 +564,12 @@ export async function createRouter( entityRef: CompoundEntityRef, token: string | undefined, ) { - let template = await findTemplate({ + const template = await findTemplate({ catalogApi: catalogClient, entityRef, token, }); + if (!isSupportedTemplate(template)) { throw new InputError( `Unsupported apiVersion field in schema entity, ${ @@ -588,32 +577,24 @@ export async function createRouter( }`, ); } + const authorizeDecision = ( await permissionApi.authorizeConditional( - [{ permission: templateSchemaExecutePermission }], - { - token, - }, + [{ permission: templateStepReadPermission }], + { token }, ) )[0]; if (authorizeDecision.result === AuthorizeResult.DENY) { - throw new NotAllowedError( - `Not allowed to execute template ${entityRef.kind}:${entityRef.namespace}/${entityRef.name}`, + template.spec.steps = []; + } else if (authorizeDecision.result === AuthorizeResult.CONDITIONAL) { + template.spec.steps = template.spec.steps.filter(step => + applyConditions(authorizeDecision.conditions, step, getRule), ); } - if (authorizeDecision.result === AuthorizeResult.CONDITIONAL) { - const scaffolderFilter = transformConditions( - authorizeDecision.conditions, - ); - if (isAndCriteria(scaffolderFilter)) { - template = scaffolderFilter.allOf.reduce( - (acc, filter) => (filter as TemplateTransform)(acc), - template, - ); - } - } + return template; } + return app; } diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 11c938cab4..5166c86723 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -14,100 +14,34 @@ * limitations under the License. */ +import { EntitiesSearchFilter } from '@backstage/plugin-catalog-backend'; import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { - RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - TemplateEntityV1beta3, + RESOURCE_TYPE_SCAFFOLDER_STEP, + TemplateEntityStepV1beta3, } from '@backstage/plugin-scaffolder-common'; import { z } from 'zod'; -import get from 'lodash/get'; -import unset from 'lodash/unset'; -import { TemplateTransform } from './helpers'; -import { - SecureTemplater, - SecureTemplateRenderer, -} from '../lib/templating/SecureTemplater'; - -export const createScaffolderPermissionRule = makeCreatePermissionRule< - TemplateEntityV1beta3, - TemplateTransform, - typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE +export const createScaffolderStepPermissionRule = makeCreatePermissionRule< + TemplateEntityStepV1beta3, + EntitiesSearchFilter, + typeof RESOURCE_TYPE_SCAFFOLDER_STEP >(); -let render: SecureTemplateRenderer; - -export const allowCapabilities = createScaffolderPermissionRule({ - name: 'ALLOW_CAPABILITIES', - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - description: 'Allow capabilities based on permissions', +const hasTag = createScaffolderStepPermissionRule({ + name: 'HAS_TAG', + resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, + description: 'Match a scaffolder step with the given tag', paramsSchema: z.object({ - capabilities: z.array(z.string()), - templateRef: z.string().optional(), + tag: z.string(), + }), + apply: (resource, { tag }) => { + return resource.metadata?.tags?.includes(tag) ?? false; + }, + toQuery: ({ tag }) => ({ + key: 'metadata.tags', + values: [tag], }), - apply: () => true, - toQuery: - ({ capabilities }) => - template => { - const capabilitiesObj = capabilities.reduce((acc, c) => { - acc[c] = true; - return acc; - }, {} as Record); - - const parametersPath = ['spec', 'parameters']; - const capabilitiesKey = 'backstage:capabilities'; - - if (Array.isArray(template.spec.parameters)) { - template.spec.parameters.forEach((parameter, index) => { - const parameterPath = [...parametersPath, index.toString()]; - stripParams(parameterPath); - if (parameter.properties) { - Object.keys(parameter.properties).forEach(p => { - if (stripParams([...parameterPath, 'properties', p])) { - const requiredProperties = get(template, [ - ...parameterPath, - 'required', - ]); - if (Array.isArray(requiredProperties)) { - requiredProperties.splice(requiredProperties.indexOf(p), 1); - } - } - }); - } - }); - } else { - stripParams(parametersPath); - } - - template.spec.steps.forEach((_p, index) => - stripParams(['spec', 'steps', index.toString()]), - ); - - function stripParams(path: string[]) { - const jsonObject = get(template, path); - - if ( - typeof jsonObject[capabilitiesKey] === 'string' && - render(jsonObject[capabilitiesKey], { - capabilities: capabilitiesObj, - }) !== 'true' - ) { - const parent = get(template, [...path].splice(0, path.length - 1)); - if (Array.isArray(parent)) { - parent.splice(parent.indexOf(jsonObject), 1); - } else { - unset(template, path); - } - - return path[path.length - 1]; - } - return undefined; - } - - return template; - }, }); -SecureTemplater.loadRenderer().then(r => (render = r)); - -export const scaffolderRules = { allowCapabilities }; +export const scaffolderStepRules = { hasTag }; diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index aaba35911e..d83a1e3be5 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -55,13 +55,7 @@ export interface TemplateEntityV1beta3 extends Entity { * A list of steps to be executed in sequence which are defined by the template. These steps are a list of the underlying * javascript action and some optional input parameters that may or may not have been collected from the end user. */ - steps: Array<{ - id?: string; - name?: string; - action: string; - input?: JsonObject; - if?: string | boolean; - }>; + steps: Array; /** * The output is an object where template authors can pull out information from template actions and return them in a known standard way. */ @@ -73,6 +67,20 @@ export interface TemplateEntityV1beta3 extends Entity { }; } +/** + * TODO + */ +export interface TemplateEntityStepV1beta3 extends JsonObject { + id?: string; + name?: string; + action: string; + input?: JsonObject; + if?: string | boolean; + metadata: { + tags?: string[]; + }; +} + const validator = entityKindSchemaValidator(schema); /** diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index 4953a27f9e..e4207c96fd 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -26,4 +26,7 @@ export { isTemplateEntityV1beta3, } from './TemplateEntityV1beta3'; export * from './permissions'; -export type { TemplateEntityV1beta3 } from './TemplateEntityV1beta3'; +export type { + TemplateEntityV1beta3, + TemplateEntityStepV1beta3, +} from './TemplateEntityV1beta3'; diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 578ef09311..bbf465c525 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -16,14 +16,42 @@ import { createPermission } from '@backstage/plugin-permission-common'; -export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-field'; +export const RESOURCE_TYPE_SCAFFOLDER_PARAMETER = 'scaffolder-parameter'; +export const RESOURCE_TYPE_SCAFFOLDER_PROPERTY = 'scaffolder-property'; +export const RESOURCE_TYPE_SCAFFOLDER_STEP = 'scaffolder-step'; -export const templateSchemaExecutePermission = createPermission({ - name: 'scaffolder.template.schema.execute', +export const templateParameterReadPermission = createPermission({ + name: 'scaffolder.template.parameter.read', attributes: { - action: 'create', + action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PARAMETER, }); -export const scaffolderPermissions = [templateSchemaExecutePermission]; +export const templatePropertyReadPermission = createPermission({ + name: 'scaffolder.template.property.read', + attributes: { + action: 'read', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, +}); + +export const templateStepReadPermission = createPermission({ + name: 'scaffolder.template.step.read', + attributes: { + action: 'read', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, +}); + +export const scaffolderParameterPermissions = [templateParameterReadPermission]; +export const scaffolderPropertyPermissions = [templatePropertyReadPermission]; +export const scaffolderStepPermissions = [templateStepReadPermission]; + +/** + * TODOs: + * 1. Implement for Parameters & Properties + * 2. What metadata should be included in the template? + * 3. Write tests + * 4. Write documentation + */ diff --git a/template.yaml b/template.yaml index 0bb67735a9..36932eb4a6 100644 --- a/template.yaml +++ b/template.yaml @@ -14,7 +14,6 @@ spec: - owner properties: component_id: - backstage:capabilities: ${{ capabilities.cap1 }} title: Name type: string description: Unique name of the component @@ -44,11 +43,13 @@ spec: - github.com steps: - id: one - backstage:capabilities: ${{ capabilities.cap1 }} name: First log action: debug:log input: message: hello + metadata: + tags: + - example - id: two name: Second log action: debug:log From 133915942a1f99d00a9bc3b49a94b8242af1945c Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 16 Jan 2023 11:35:17 +0100 Subject: [PATCH 07/55] scaffolder: single resource type Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/permission.ts | 7 +++++-- plugins/scaffolder-common/src/permissions.ts | 16 ++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index 84a102d6be..fdff4018d2 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -30,7 +30,7 @@ import { createScaffolderStepConditionalDecision, scaffolderStepConditions, } from '@backstage/plugin-scaffolder-backend'; -import { RESOURCE_TYPE_SCAFFOLDER_STEP } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_PROPERTY } from '@backstage/plugin-scaffolder-common'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; @@ -43,7 +43,10 @@ class ExamplePermissionPolicy implements PermissionPolicy { * This is an example of how to use the scaffolder step conditions. */ if ( - isResourcePermission(request.permission, RESOURCE_TYPE_SCAFFOLDER_STEP) + isResourcePermission( + request.permission, + RESOURCE_TYPE_SCAFFOLDER_PROPERTY, + ) ) { return createScaffolderStepConditionalDecision( request.permission, diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index bbf465c525..334eb084b1 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -16,16 +16,14 @@ import { createPermission } from '@backstage/plugin-permission-common'; -export const RESOURCE_TYPE_SCAFFOLDER_PARAMETER = 'scaffolder-parameter'; export const RESOURCE_TYPE_SCAFFOLDER_PROPERTY = 'scaffolder-property'; -export const RESOURCE_TYPE_SCAFFOLDER_STEP = 'scaffolder-step'; export const templateParameterReadPermission = createPermission({ name: 'scaffolder.template.parameter.read', attributes: { action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_PARAMETER, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, }); export const templatePropertyReadPermission = createPermission({ @@ -41,16 +39,18 @@ export const templateStepReadPermission = createPermission({ attributes: { action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, }); -export const scaffolderParameterPermissions = [templateParameterReadPermission]; -export const scaffolderPropertyPermissions = [templatePropertyReadPermission]; -export const scaffolderStepPermissions = [templateStepReadPermission]; +export const scaffolderPermissions = [ + templatePropertyReadPermission, + templateParameterReadPermission, + templateStepReadPermission, +]; /** * TODOs: - * 1. Implement for Parameters & Properties + * 1. ~Implement for Parameters & Properties~ * 2. What metadata should be included in the template? * 3. Write tests * 4. Write documentation From 562611ef56f61c55fbedbb780a84e2e4669d22bc Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 16 Jan 2023 11:36:03 +0100 Subject: [PATCH 08/55] scaffolder: make rule generic Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-backend/src/service/rules.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 5166c86723..0ca83fd5fc 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -14,23 +14,24 @@ * limitations under the License. */ -import { EntitiesSearchFilter } from '@backstage/plugin-catalog-backend'; import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { - RESOURCE_TYPE_SCAFFOLDER_STEP, + RESOURCE_TYPE_SCAFFOLDER_PROPERTY, TemplateEntityStepV1beta3, + TemplateParameter, } from '@backstage/plugin-scaffolder-common'; +import { TemplateProperty } from '@backstage/plugin-scaffolder-common'; import { z } from 'zod'; export const createScaffolderStepPermissionRule = makeCreatePermissionRule< - TemplateEntityStepV1beta3, - EntitiesSearchFilter, - typeof RESOURCE_TYPE_SCAFFOLDER_STEP + TemplateEntityStepV1beta3 | TemplateProperty | TemplateParameter, + {}, + typeof RESOURCE_TYPE_SCAFFOLDER_PROPERTY >(); const hasTag = createScaffolderStepPermissionRule({ name: 'HAS_TAG', - resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, description: 'Match a scaffolder step with the given tag', paramsSchema: z.object({ tag: z.string(), @@ -38,10 +39,7 @@ const hasTag = createScaffolderStepPermissionRule({ apply: (resource, { tag }) => { return resource.metadata?.tags?.includes(tag) ?? false; }, - toQuery: ({ tag }) => ({ - key: 'metadata.tags', - values: [tag], - }), + toQuery: () => ({}), }); export const scaffolderStepRules = { hasTag }; From 031781fb20cc9f962956c3cec3528cc03cd17d4b Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 16 Jan 2023 11:36:53 +0100 Subject: [PATCH 09/55] scaffolder: expose template subtypes Signed-off-by: Vincenzo Scamporlino --- .../src/TemplateEntityV1beta3.ts | 28 ++++++++++++++++--- plugins/scaffolder-common/src/index.ts | 3 ++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index d83a1e3be5..63dae930c2 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -50,7 +50,7 @@ export interface TemplateEntityV1beta3 extends Entity { * to collect user input and validate it against that schema. This can then be used in the `steps` part below to template * variables passed from the user into each action in the template. */ - parameters?: JsonObject | JsonObject[]; + parameters?: TemplateParameter | TemplateParameter[]; /** * A list of steps to be executed in sequence which are defined by the template. These steps are a list of the underlying * javascript action and some optional input parameters that may or may not have been collected from the end user. @@ -76,9 +76,29 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { action: string; input?: JsonObject; if?: string | boolean; - metadata: { - tags?: string[]; - }; + metadata?: TemplateSpecValuesMetadata; +} + +/** + * TODO + */ +export interface TemplateSpecValuesMetadata extends JsonObject { + tags?: string[]; +} + +/** + * TODO + */ +export interface TemplateParameter extends JsonObject { + metadata?: TemplateSpecValuesMetadata; + properties?: { [name: string]: TemplateProperty }; +} + +/** + * TODO + */ +export interface TemplateProperty extends JsonObject { + metadata?: TemplateSpecValuesMetadata; } const validator = entityKindSchemaValidator(schema); diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index e4207c96fd..c418823e12 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -29,4 +29,7 @@ export * from './permissions'; export type { TemplateEntityV1beta3, TemplateEntityStepV1beta3, + TemplateParameter, + TemplateProperty, + TemplateSpecValuesMetadata, } from './TemplateEntityV1beta3'; From 2e7d060ef96dc1dfdbd65154be3d6066f23796fb Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 16 Jan 2023 11:37:53 +0100 Subject: [PATCH 10/55] scaffolder: remove conditions wrapper Signed-off-by: Vincenzo Scamporlino --- .../src/service/conditionExports.ts | 31 ++----------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 2a7c0d382f..2fb0d5d554 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -14,42 +14,17 @@ * limitations under the License. */ -import { RESOURCE_TYPE_SCAFFOLDER_STEP } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_PROPERTY } from '@backstage/plugin-scaffolder-common'; import { createConditionExports } from '@backstage/plugin-permission-node'; import { scaffolderStepRules } from './rules'; -// const { -// conditions: scaffolderTemplateConditions, -// createConditionalDecision: createScaffolderTemplateConditionalDecision, -// } = createConditionExports({ -// pluginId: 'scaffolder', -// resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, -// rules: scaffolderRules, -// }); - -// const { -// conditions: scaffolderPropertyConditions, -// createConditionalDecision: createScaffolderPropertyConditionalDecision, -// } = createConditionExports({ -// pluginId: 'scaffolder', -// resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, -// rules: scaffolderRules, -// }); - const { conditions: scaffolderStepConditions, createConditionalDecision: createScaffolderStepConditionalDecision, } = createConditionExports({ pluginId: 'scaffolder', - resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, rules: scaffolderStepRules, }); -export { - // scaffolderTemplateConditions, - // createScaffolderTemplateConditionalDecision, - // scaffolderPropertyConditions, - // createScaffolderPropertyConditionalDecision, - scaffolderStepConditions, - createScaffolderStepConditionalDecision, -}; +export { scaffolderStepConditions, createScaffolderStepConditionalDecision }; From 55c49d2b6c587e9cbfabbbd2ab6618f569033e37 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 16 Jan 2023 11:38:31 +0100 Subject: [PATCH 11/55] scaffolder: authorize parameters and properties Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-backend/src/service/router.ts | 82 +++++++++++++++++-- 1 file changed, 75 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index d5a9e17601..660398fbd0 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -32,6 +32,10 @@ import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, + TemplateParameter, + templateParameterReadPermission, + TemplateProperty, + templatePropertyReadPermission, templateStepReadPermission, } from '@backstage/plugin-scaffolder-common'; import express from 'express'; @@ -578,18 +582,82 @@ export async function createRouter( ); } - const authorizeDecision = ( + const [parameterDecision, propertyDecision, stepDecision] = await permissionApi.authorizeConditional( - [{ permission: templateStepReadPermission }], + [ + { permission: templateParameterReadPermission }, + { permission: templatePropertyReadPermission }, + { permission: templateStepReadPermission }, + ], { token }, - ) - )[0]; + ); - if (authorizeDecision.result === AuthorizeResult.DENY) { + // authorize parameters + if (parameterDecision.result === AuthorizeResult.DENY) { + template.spec.parameters = []; + } else if (parameterDecision.result === AuthorizeResult.CONDITIONAL) { + if (Array.isArray(template.spec.parameters)) { + template.spec.parameters = template.spec.parameters.filter(step => + applyConditions(parameterDecision.conditions, step, getRule), + ); + } else { + if ( + template.spec.parameters && + !applyConditions( + parameterDecision.conditions, + template.spec.parameters, + getRule, + ) + ) { + template.spec.parameters = undefined; + } + } + } + + // authorize properties + if (propertyDecision.result === AuthorizeResult.DENY) { + if (Array.isArray(template.spec.parameters)) { + template.spec.parameters.forEach(parameter => { + parameter.properties = {}; + }); + } else { + if (template.spec.parameters) { + template.spec.parameters.properties = {}; + } + } + } else if (propertyDecision.result === AuthorizeResult.CONDITIONAL) { + if (Array.isArray(template.spec.parameters)) { + template.spec.parameters.forEach(parameter => { + parameter.properties = Object.entries( + parameter.properties || {}, + ).reduce>((acc, [key, value]) => { + if (applyConditions(propertyDecision.conditions, value, getRule)) { + acc[key] = value; + } + return acc; + }, {}); + }); + } else { + // TODO extract this to a generic method and use it in the above if block + if (template.spec.parameters) { + template.spec.parameters.properties = Object.entries( + template.spec.parameters.properties || {}, + ).reduce>((acc, [key, value]) => { + if (applyConditions(propertyDecision.conditions, value, getRule)) { + acc[key] = value; + } + return acc; + }, {}); + } + } + } + + // authorize steps + if (stepDecision.result === AuthorizeResult.DENY) { template.spec.steps = []; - } else if (authorizeDecision.result === AuthorizeResult.CONDITIONAL) { + } else if (stepDecision.result === AuthorizeResult.CONDITIONAL) { template.spec.steps = template.spec.steps.filter(step => - applyConditions(authorizeDecision.conditions, step, getRule), + applyConditions(stepDecision.conditions, step, getRule), ); } From 53c0aa00292f491b9c822bf8e83eedf4d3a2bd5b Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 16 Jan 2023 12:52:30 +0000 Subject: [PATCH 12/55] scaffolder: added back in playlist permission handling Signed-off-by: Harry Hogg --- packages/backend/src/plugins/permission.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index fdff4018d2..c3d81a52cf 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -25,6 +25,10 @@ import { PermissionPolicy, PolicyQuery, } from '@backstage/plugin-permission-node'; +import { + DefaultPlaylistPermissionPolicy, + isPlaylistPermission, +} from '@backstage/plugin-playlist-backend'; import { createScaffolderStepConditionalDecision, @@ -35,13 +39,16 @@ import { Router } from 'express'; import { PluginEnvironment } from '../types'; class ExamplePermissionPolicy implements PermissionPolicy { + private playlistPermissionPolicy = new DefaultPlaylistPermissionPolicy(); + async handle( request: PolicyQuery, - _user?: BackstageIdentityResponse, + user?: BackstageIdentityResponse, ): Promise { - /** - * This is an example of how to use the scaffolder step conditions. - */ + if (isPlaylistPermission(request.permission)) { + return this.playlistPermissionPolicy.handle(request, user); + } + if ( isResourcePermission( request.permission, From 2b8ad06250fe2054d910b5fad4ebde999ddb1eac Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 16 Jan 2023 12:56:10 +0000 Subject: [PATCH 13/55] scaffolder: added description to hasTag rule Signed-off-by: Harry Hogg --- plugins/scaffolder-backend/src/service/rules.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 0ca83fd5fc..b9d9e62c1b 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -34,7 +34,7 @@ const hasTag = createScaffolderStepPermissionRule({ resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, description: 'Match a scaffolder step with the given tag', paramsSchema: z.object({ - tag: z.string(), + tag: z.string().describe('Name of the tag to match on'), }), apply: (resource, { tag }) => { return resource.metadata?.tags?.includes(tag) ?? false; From 2b124bc24ad62a8782a9f7321847ea4882640ef5 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 16 Jan 2023 15:05:49 +0000 Subject: [PATCH 14/55] Reworked authorization of conditions to use a single export by combing getRule and applyConditions into createIsAuthorized Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.ts | 23 +++++++++++++++++- .../permission-node/src/integration/index.ts | 7 +----- .../scaffolder-backend/src/service/router.ts | 24 +++++++------------ 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index e3f4b33707..a4f14e6cbc 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -126,7 +126,7 @@ export type MetadataResponse = { rules: MetadataResponseSerializedRule[]; }; -export const applyConditions = ( +const applyConditions = ( criteria: PermissionCriteria>, resource: TResource | undefined, getRule: (name: string) => PermissionRule, @@ -160,6 +160,27 @@ export const applyConditions = ( return rule.apply(resource, criteria.params ?? {}); }; +/** + + * Takes some permission conditions and returns a definitive authorization result + * on the resource to which they apply. + * + * @public + */ +export const createIsAuthorized = < + TResourceType extends string, + TResource, + TQuery, +>( + rules: PermissionRule[], +) => { + const getRule = createGetRule(rules); + return ( + criteria: PermissionCriteria>, + resource: TResource | undefined, + ) => applyConditions(criteria, resource, getRule); +}; + /** * Options for creating a permission integration router specific * for a particular resource type. diff --git a/plugins/permission-node/src/integration/index.ts b/plugins/permission-node/src/integration/index.ts index f3b0edad9b..7702fea95b 100644 --- a/plugins/permission-node/src/integration/index.ts +++ b/plugins/permission-node/src/integration/index.ts @@ -19,9 +19,4 @@ export * from './createConditionExports'; export * from './createConditionTransformer'; export * from './createPermissionIntegrationRouter'; export * from './createPermissionRule'; -export { - createGetRule, - isAndCriteria, - isOrCriteria, - isNotCriteria, -} from './util'; +export { isAndCriteria, isOrCriteria, isNotCriteria } from './util'; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 660398fbd0..6442adbc4b 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -32,7 +32,6 @@ import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, - TemplateParameter, templateParameterReadPermission, TemplateProperty, templatePropertyReadPermission, @@ -63,12 +62,11 @@ import { AuthorizeResult, PermissionEvaluator, } from '@backstage/plugin-permission-common'; -import { - applyConditions, - createGetRule, -} from '@backstage/plugin-permission-node'; +import { createIsAuthorized } from '@backstage/plugin-permission-node'; import { scaffolderStepRules } from './rules'; +const isAuthorized = createIsAuthorized(Object.values(scaffolderStepRules)); + /** * RouterOptions * @@ -266,8 +264,6 @@ export async function createRouter( additionalTemplateGlobals, }); - const getRule = createGetRule(Object.values(scaffolderStepRules)); - router .get( '/v2/templates/:namespace/:kind/:name/parameter-schema', @@ -598,16 +594,12 @@ export async function createRouter( } else if (parameterDecision.result === AuthorizeResult.CONDITIONAL) { if (Array.isArray(template.spec.parameters)) { template.spec.parameters = template.spec.parameters.filter(step => - applyConditions(parameterDecision.conditions, step, getRule), + isAuthorized(parameterDecision.conditions, step), ); } else { if ( template.spec.parameters && - !applyConditions( - parameterDecision.conditions, - template.spec.parameters, - getRule, - ) + !isAuthorized(parameterDecision.conditions, template.spec.parameters) ) { template.spec.parameters = undefined; } @@ -631,7 +623,7 @@ export async function createRouter( parameter.properties = Object.entries( parameter.properties || {}, ).reduce>((acc, [key, value]) => { - if (applyConditions(propertyDecision.conditions, value, getRule)) { + if (isAuthorized(propertyDecision.conditions, value)) { acc[key] = value; } return acc; @@ -643,7 +635,7 @@ export async function createRouter( template.spec.parameters.properties = Object.entries( template.spec.parameters.properties || {}, ).reduce>((acc, [key, value]) => { - if (applyConditions(propertyDecision.conditions, value, getRule)) { + if (isAuthorized(propertyDecision.conditions, value)) { acc[key] = value; } return acc; @@ -657,7 +649,7 @@ export async function createRouter( template.spec.steps = []; } else if (stepDecision.result === AuthorizeResult.CONDITIONAL) { template.spec.steps = template.spec.steps.filter(step => - applyConditions(stepDecision.conditions, step, getRule), + isAuthorized(stepDecision.conditions, step), ); } From 906be330351ffa7b1d1827b4b28659b84efcdd4f Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 16 Jan 2023 15:15:59 +0000 Subject: [PATCH 15/55] Added some todos for cleanup Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- app-config.yaml | 2 ++ template.yaml | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/app-config.yaml b/app-config.yaml index d886d86896..38364d73dc 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -280,6 +280,8 @@ catalog: # Backstage end-to-end tests of TechDocs - type: file target: ../../cypress/e2e-fixture.catalog.info.yaml + + # TODO: Remove once approach is validated on RFC. - type: file target: ../../template.yaml scaffolder: diff --git a/template.yaml b/template.yaml index 36932eb4a6..83273faea6 100644 --- a/template.yaml +++ b/template.yaml @@ -1,3 +1,4 @@ +# TODO: Remove once approach is validated on RFC. apiVersion: scaffolder.backstage.io/v1beta3 kind: Template metadata: @@ -12,12 +13,20 @@ spec: required: - component_id - owner + # Example of annotating a parameter + metadata: + tags: + - example properties: component_id: title: Name type: string description: Unique name of the component ui:field: EntityNamePicker + # Example of annotating a property + metadata: + tags: + - example description: title: Description type: string @@ -47,6 +56,7 @@ spec: action: debug:log input: message: hello + # Example of annotating a step metadata: tags: - example From 2624c1afff8ac2f56ec18e10ecc33f7afaf220d0 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 17 Jan 2023 12:47:27 +0100 Subject: [PATCH 16/55] permissions: rollback default policy Co-authored-by: Harry Hogg Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/permission.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index c3d81a52cf..45368bf2f0 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -18,7 +18,6 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { createRouter } from '@backstage/plugin-permission-backend'; import { AuthorizeResult, - isResourcePermission, PolicyDecision, } from '@backstage/plugin-permission-common'; import { @@ -30,11 +29,6 @@ import { isPlaylistPermission, } from '@backstage/plugin-playlist-backend'; -import { - createScaffolderStepConditionalDecision, - scaffolderStepConditions, -} from '@backstage/plugin-scaffolder-backend'; -import { RESOURCE_TYPE_SCAFFOLDER_PROPERTY } from '@backstage/plugin-scaffolder-common'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; @@ -49,18 +43,6 @@ class ExamplePermissionPolicy implements PermissionPolicy { return this.playlistPermissionPolicy.handle(request, user); } - if ( - isResourcePermission( - request.permission, - RESOURCE_TYPE_SCAFFOLDER_PROPERTY, - ) - ) { - return createScaffolderStepConditionalDecision( - request.permission, - scaffolderStepConditions.hasTag({ tag: 'example' }), - ); - } - return { result: AuthorizeResult.ALLOW, }; From 3e109f452e414a8b4a95e9ddab99dfd4ea0dd8ff Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 17 Jan 2023 12:50:38 +0100 Subject: [PATCH 17/55] scaffolder: rollback template and config Co-authored-by: Harry Hogg Signed-off-by: Vincenzo Scamporlino --- app-config.yaml | 5 ---- template.yaml | 67 ------------------------------------------------- 2 files changed, 72 deletions(-) delete mode 100644 template.yaml diff --git a/app-config.yaml b/app-config.yaml index 38364d73dc..385c9c2e0e 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -223,7 +223,6 @@ catalog: - System - Domain - Location - - Template processors: ldapOrg: @@ -280,10 +279,6 @@ catalog: # Backstage end-to-end tests of TechDocs - type: file target: ../../cypress/e2e-fixture.catalog.info.yaml - - # TODO: Remove once approach is validated on RFC. - - type: file - target: ../../template.yaml scaffolder: # Use to customize default commit author info used when new components are created # defaultAuthor: diff --git a/template.yaml b/template.yaml deleted file mode 100644 index 83273faea6..0000000000 --- a/template.yaml +++ /dev/null @@ -1,67 +0,0 @@ -# TODO: Remove once approach is validated on RFC. -apiVersion: scaffolder.backstage.io/v1beta3 -kind: Template -metadata: - name: my_custom_template - title: My custom template - description: Just testing -spec: - owner: web@example.com - type: website - parameters: - - title: Provide some simple information - required: - - component_id - - owner - # Example of annotating a parameter - metadata: - tags: - - example - properties: - component_id: - title: Name - type: string - description: Unique name of the component - ui:field: EntityNamePicker - # Example of annotating a property - metadata: - tags: - - example - description: - title: Description - type: string - description: Help others understand what this website is for. - owner: - title: Owner - type: string - description: Owner of the component - ui:field: OwnerPicker - ui:options: - allowedKinds: - - Group - - title: Choose a location - required: - - repoUrl - properties: - repoUrl: - title: Repository Location - type: string - ui:field: RepoUrlPicker - ui:options: - allowedHosts: - - github.com - steps: - - id: one - name: First log - action: debug:log - input: - message: hello - # Example of annotating a step - metadata: - tags: - - example - - id: two - name: Second log - action: debug:log - input: - message: world From faba894d8b4af730f0931858abcfff9e64b6287e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 17 Jan 2023 12:52:00 +0100 Subject: [PATCH 18/55] scaffolder: rename resource type Co-authored-by: Harry Hogg Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-backend/src/service/conditionExports.ts | 10 +++++----- plugins/scaffolder-backend/src/service/rules.ts | 6 +++--- plugins/scaffolder-common/src/permissions.ts | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 2fb0d5d554..70285c8f5f 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -14,17 +14,17 @@ * limitations under the License. */ -import { RESOURCE_TYPE_SCAFFOLDER_PROPERTY } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common'; import { createConditionExports } from '@backstage/plugin-permission-node'; import { scaffolderStepRules } from './rules'; const { - conditions: scaffolderStepConditions, - createConditionalDecision: createScaffolderStepConditionalDecision, + conditions: scaffolderConditions, + createConditionalDecision: createScaffolderConditionalDecision, } = createConditionExports({ pluginId: 'scaffolder', - resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, rules: scaffolderStepRules, }); -export { scaffolderStepConditions, createScaffolderStepConditionalDecision }; +export { scaffolderConditions, createScaffolderConditionalDecision }; diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index b9d9e62c1b..dad0b84097 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -16,7 +16,7 @@ import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { - RESOURCE_TYPE_SCAFFOLDER_PROPERTY, + RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, TemplateEntityStepV1beta3, TemplateParameter, } from '@backstage/plugin-scaffolder-common'; @@ -26,12 +26,12 @@ import { z } from 'zod'; export const createScaffolderStepPermissionRule = makeCreatePermissionRule< TemplateEntityStepV1beta3 | TemplateProperty | TemplateParameter, {}, - typeof RESOURCE_TYPE_SCAFFOLDER_PROPERTY + typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); const hasTag = createScaffolderStepPermissionRule({ name: 'HAS_TAG', - resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, description: 'Match a scaffolder step with the given tag', paramsSchema: z.object({ tag: z.string().describe('Name of the tag to match on'), diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 334eb084b1..1117fb268b 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -16,14 +16,14 @@ import { createPermission } from '@backstage/plugin-permission-common'; -export const RESOURCE_TYPE_SCAFFOLDER_PROPERTY = 'scaffolder-property'; +export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; export const templateParameterReadPermission = createPermission({ name: 'scaffolder.template.parameter.read', attributes: { action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, }); export const templatePropertyReadPermission = createPermission({ @@ -31,7 +31,7 @@ export const templatePropertyReadPermission = createPermission({ attributes: { action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, }); export const templateStepReadPermission = createPermission({ @@ -39,7 +39,7 @@ export const templateStepReadPermission = createPermission({ attributes: { action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, }); export const scaffolderPermissions = [ From a91abdf0618471a090ad26947e2d54ee771cb284 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 17 Jan 2023 15:13:32 +0100 Subject: [PATCH 19/55] chore: cleanup Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/permission.ts | 1 - plugins/scaffolder-common/src/permissions.ts | 8 -------- 2 files changed, 9 deletions(-) diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index 45368bf2f0..7192a1ddec 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -28,7 +28,6 @@ import { DefaultPlaylistPermissionPolicy, isPlaylistPermission, } from '@backstage/plugin-playlist-backend'; - import { Router } from 'express'; import { PluginEnvironment } from '../types'; diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 1117fb268b..5b0d7ff00c 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -47,11 +47,3 @@ export const scaffolderPermissions = [ templateParameterReadPermission, templateStepReadPermission, ]; - -/** - * TODOs: - * 1. ~Implement for Parameters & Properties~ - * 2. What metadata should be included in the template? - * 3. Write tests - * 4. Write documentation - */ From 97be4a96ed7707d84f53ededd0a3af2044e1607f Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 30 Jan 2023 14:27:21 +0000 Subject: [PATCH 20/55] Refactored createIsAuthorized to take a decision Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.ts | 18 +++-- .../scaffolder-backend/src/service/router.ts | 79 ++++--------------- 2 files changed, 26 insertions(+), 71 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index a4f14e6cbc..5b0c2c25f1 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -27,6 +27,7 @@ import { Permission, PermissionCondition, PermissionCriteria, + PolicyDecision, } from '@backstage/plugin-permission-common'; import { PermissionRule } from '../types'; import { @@ -167,18 +168,21 @@ const applyConditions = ( * * @public */ -export const createIsAuthorized = < - TResourceType extends string, - TResource, - TQuery, ->( +export const createIsAuthorized = ( rules: PermissionRule[], ) => { const getRule = createGetRule(rules); + return ( - criteria: PermissionCriteria>, + decision: PolicyDecision, resource: TResource | undefined, - ) => applyConditions(criteria, resource, getRule); + ): boolean => { + if (decision.result === AuthorizeResult.CONDITIONAL) { + return applyConditions(decision.conditions, resource, getRule); + } + + return decision.result === AuthorizeResult.ALLOW; + }; }; /** diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 6442adbc4b..74b3d7a095 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -578,81 +578,32 @@ export async function createRouter( ); } - const [parameterDecision, propertyDecision, stepDecision] = + const [parameterDecision, stepDecision] = await permissionApi.authorizeConditional( [ { permission: templateParameterReadPermission }, - { permission: templatePropertyReadPermission }, { permission: templateStepReadPermission }, ], { token }, ); - // authorize parameters - if (parameterDecision.result === AuthorizeResult.DENY) { - template.spec.parameters = []; - } else if (parameterDecision.result === AuthorizeResult.CONDITIONAL) { - if (Array.isArray(template.spec.parameters)) { - template.spec.parameters = template.spec.parameters.filter(step => - isAuthorized(parameterDecision.conditions, step), - ); - } else { - if ( - template.spec.parameters && - !isAuthorized(parameterDecision.conditions, template.spec.parameters) - ) { - template.spec.parameters = undefined; - } - } - } - - // authorize properties - if (propertyDecision.result === AuthorizeResult.DENY) { - if (Array.isArray(template.spec.parameters)) { - template.spec.parameters.forEach(parameter => { - parameter.properties = {}; - }); - } else { - if (template.spec.parameters) { - template.spec.parameters.properties = {}; - } - } - } else if (propertyDecision.result === AuthorizeResult.CONDITIONAL) { - if (Array.isArray(template.spec.parameters)) { - template.spec.parameters.forEach(parameter => { - parameter.properties = Object.entries( - parameter.properties || {}, - ).reduce>((acc, [key, value]) => { - if (isAuthorized(propertyDecision.conditions, value)) { - acc[key] = value; - } - return acc; - }, {}); - }); - } else { - // TODO extract this to a generic method and use it in the above if block - if (template.spec.parameters) { - template.spec.parameters.properties = Object.entries( - template.spec.parameters.properties || {}, - ).reduce>((acc, [key, value]) => { - if (isAuthorized(propertyDecision.conditions, value)) { - acc[key] = value; - } - return acc; - }, {}); - } - } - } - - // authorize steps - if (stepDecision.result === AuthorizeResult.DENY) { - template.spec.steps = []; - } else if (stepDecision.result === AuthorizeResult.CONDITIONAL) { - template.spec.steps = template.spec.steps.filter(step => - isAuthorized(stepDecision.conditions, step), + // Authorize parameters + if (Array.isArray(template.spec.parameters)) { + template.spec.parameters = template.spec.parameters.filter(step => + isAuthorized(parameterDecision, step), ); + } else if ( + template.spec.parameters && + !isAuthorized(parameterDecision, template.spec.parameters) + ) { + template.spec.parameters = undefined; } + // Authorize steps + template.spec.steps = template.spec.steps.filter(step => + isAuthorized(stepDecision, step), + ); + return template; } From 09e28e2c719c8264d1ccef0ae43d6841792040b1 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 30 Jan 2023 14:28:08 +0000 Subject: [PATCH 21/55] Removed the property scaffolder rule Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/service/router.ts | 2 -- plugins/scaffolder-common/src/permissions.ts | 9 --------- 2 files changed, 11 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 74b3d7a095..3386bb51b4 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -33,8 +33,6 @@ import { TemplateEntityV1beta3, templateEntityV1beta3Validator, templateParameterReadPermission, - TemplateProperty, - templatePropertyReadPermission, templateStepReadPermission, } from '@backstage/plugin-scaffolder-common'; import express from 'express'; diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 5b0d7ff00c..b6f2e1ebf6 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -26,14 +26,6 @@ export const templateParameterReadPermission = createPermission({ resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, }); -export const templatePropertyReadPermission = createPermission({ - name: 'scaffolder.template.property.read', - attributes: { - action: 'read', - }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, -}); - export const templateStepReadPermission = createPermission({ name: 'scaffolder.template.step.read', attributes: { @@ -43,7 +35,6 @@ export const templateStepReadPermission = createPermission({ }); export const scaffolderPermissions = [ - templatePropertyReadPermission, templateParameterReadPermission, templateStepReadPermission, ]; From e2559f8b1ecd0567df29f3e8791005521209ccf8 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 1 Feb 2023 11:08:45 +0100 Subject: [PATCH 22/55] scaffolder: rename metadata to accessControl Co-authored-by: Harry Hogg Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/service/rules.ts | 5 ++--- .../src/TemplateEntityV1beta3.ts | 15 ++++----------- plugins/scaffolder-common/src/index.ts | 2 +- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index dad0b84097..85bcca3e05 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -20,11 +20,10 @@ import { TemplateEntityStepV1beta3, TemplateParameter, } from '@backstage/plugin-scaffolder-common'; -import { TemplateProperty } from '@backstage/plugin-scaffolder-common'; import { z } from 'zod'; export const createScaffolderStepPermissionRule = makeCreatePermissionRule< - TemplateEntityStepV1beta3 | TemplateProperty | TemplateParameter, + TemplateEntityStepV1beta3 | TemplateParameter, {}, typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); @@ -37,7 +36,7 @@ const hasTag = createScaffolderStepPermissionRule({ tag: z.string().describe('Name of the tag to match on'), }), apply: (resource, { tag }) => { - return resource.metadata?.tags?.includes(tag) ?? false; + return resource['backstage:accessControl']?.tags?.includes(tag) ?? false; }, toQuery: () => ({}), }); diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index 63dae930c2..f8c47c7881 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -76,13 +76,13 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { action: string; input?: JsonObject; if?: string | boolean; - metadata?: TemplateSpecValuesMetadata; + 'backstage:accessControl'?: TemplateAccessControl; } /** * TODO */ -export interface TemplateSpecValuesMetadata extends JsonObject { +export interface TemplateAccessControl extends JsonObject { tags?: string[]; } @@ -90,15 +90,8 @@ export interface TemplateSpecValuesMetadata extends JsonObject { * TODO */ export interface TemplateParameter extends JsonObject { - metadata?: TemplateSpecValuesMetadata; - properties?: { [name: string]: TemplateProperty }; -} - -/** - * TODO - */ -export interface TemplateProperty extends JsonObject { - metadata?: TemplateSpecValuesMetadata; + 'backstage:accessControl'?: TemplateAccessControl; + properties?: { [name: string]: JsonObject }; } const validator = entityKindSchemaValidator(schema); diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index c418823e12..ec84b0e022 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -31,5 +31,5 @@ export type { TemplateEntityStepV1beta3, TemplateParameter, TemplateProperty, - TemplateSpecValuesMetadata, + TemplateAccessControl as TemplateSpecValuesMetadata, } from './TemplateEntityV1beta3'; From a16166c24f477082555233466f8a8858d422256d Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 1 Feb 2023 11:20:38 +0100 Subject: [PATCH 23/55] scaffolder: cleanup types Co-authored-by: Harry Hogg Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/service/router.ts | 5 +---- plugins/scaffolder-common/src/index.ts | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 3386bb51b4..fc410ed634 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -56,10 +56,7 @@ import { IdentityApiGetIdentityRequest, } from '@backstage/plugin-auth-node'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; -import { - AuthorizeResult, - PermissionEvaluator, -} from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { createIsAuthorized } from '@backstage/plugin-permission-node'; import { scaffolderStepRules } from './rules'; diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index ec84b0e022..14f89c2bc9 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -30,6 +30,5 @@ export type { TemplateEntityV1beta3, TemplateEntityStepV1beta3, TemplateParameter, - TemplateProperty, TemplateAccessControl as TemplateSpecValuesMetadata, } from './TemplateEntityV1beta3'; From 66935d8c315f2b28f80d7724c4b4e012dc574b2a Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 1 Feb 2023 11:21:51 +0100 Subject: [PATCH 24/55] scaffolder: pass permissions dependency Co-authored-by: Harry Hogg Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/ScaffolderPlugin.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/scaffolder-backend/src/ScaffolderPlugin.ts b/plugins/scaffolder-backend/src/ScaffolderPlugin.ts index 182bb60223..97abf41828 100644 --- a/plugins/scaffolder-backend/src/ScaffolderPlugin.ts +++ b/plugins/scaffolder-backend/src/ScaffolderPlugin.ts @@ -94,6 +94,7 @@ export const scaffolderPlugin = createBackendPlugin( database, httpRouter, catalogClient, + permissions, }) { const { additionalTemplateFilters, @@ -131,6 +132,7 @@ export const scaffolderPlugin = createBackendPlugin( taskWorkers, additionalTemplateFilters, additionalTemplateGlobals, + permissionApi: permissions, }); httpRouter.use(router); }, From b79dee1cf8240f8ad0cbebfef9f0410440a27771 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 1 Feb 2023 14:55:45 +0100 Subject: [PATCH 25/55] permission-node: test createIsAuthorized Signed-off-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.test.ts | 80 ++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 1a7462665e..e7091e7936 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -25,6 +25,7 @@ import { z } from 'zod'; import { createPermissionIntegrationRouter, CreatePermissionIntegrationRouterResourceOptions, + createIsAuthorized, } from './createPermissionIntegrationRouter'; import { createPermissionRule } from './createPermissionRule'; @@ -33,6 +34,9 @@ const testPermission: Permission = createPermission({ attributes: {}, }); +const mockTestRule1Apply = jest + .fn() + .mockImplementation((_resource: any, _params) => true); const testRule1 = createPermissionRule({ name: 'test-rule-1', description: 'Test rule 1', @@ -41,15 +45,18 @@ const testRule1 = createPermissionRule({ foo: z.string(), bar: z.number().describe('bar'), }), - apply: (_resource: any, _params) => true, + apply: mockTestRule1Apply, toQuery: _params => ({}), }); +const mockTestRule2Apply = jest + .fn() + .mockImplementation((_resource: any) => false); const testRule2 = createPermissionRule({ name: 'test-rule-2', description: 'Test rule 2', resourceType: 'test-resource', - apply: (_resource: any) => false, + apply: mockTestRule2Apply, toQuery: () => ({}), }); @@ -625,3 +632,72 @@ describe('createPermissionIntegrationRouter', () => { }); }); }); + +describe('createIsAuthorized', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should return true in case of allowed decision', () => { + const isAuthorized = createIsAuthorized([testRule1, testRule2]); + expect( + isAuthorized( + { + result: AuthorizeResult.ALLOW, + }, + {}, + ), + ).toBe(true); + + expect(mockTestRule1Apply).not.toHaveBeenCalled(); + expect(mockTestRule2Apply).not.toHaveBeenCalled(); + }); + it('should return false in case of denied decision', () => { + const isAuthorized = createIsAuthorized([testRule1, testRule2]); + expect( + isAuthorized( + { + result: AuthorizeResult.DENY, + }, + {}, + ), + ).toBe(false); + + expect(mockTestRule1Apply).not.toHaveBeenCalled(); + expect(mockTestRule2Apply).not.toHaveBeenCalled(); + }); + + it('should apply conditions to a resource in case of conditional decision', () => { + const isAuthorized = createIsAuthorized([testRule1, testRule2]); + expect( + isAuthorized( + { + pluginId: 'plugin', + resourceType: 'test-resource', + result: AuthorizeResult.CONDITIONAL, + conditions: { + allOf: [ + { + rule: 'test-rule-1', + resourceType: 'test-resource', + params: { + foo: 'a', + bar: 1, + }, + }, + { + rule: 'test-rule-2', + resourceType: 'test-resource', + params: {}, + }, + ], + }, + }, + {}, + ), + ).toBe(false); + + expect(mockTestRule1Apply).toHaveBeenCalledTimes(1); + expect(mockTestRule2Apply).toHaveBeenCalledTimes(1); + }); +}); From 8dbacc222833e8c3abe08a0babde9e430b21c8fd Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 1 Feb 2023 14:56:05 +0100 Subject: [PATCH 26/55] scaffolder-backend: test hasTag rule Signed-off-by: Vincenzo Scamporlino --- .../src/service/rules.test.ts | 81 +++++++++++++++++++ .../scaffolder-backend/src/service/rules.ts | 6 +- 2 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 plugins/scaffolder-backend/src/service/rules.test.ts diff --git a/plugins/scaffolder-backend/src/service/rules.test.ts b/plugins/scaffolder-backend/src/service/rules.test.ts new file mode 100644 index 0000000000..0f6f4851b4 --- /dev/null +++ b/plugins/scaffolder-backend/src/service/rules.test.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2023 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 { hasTag } from './rules'; + +describe('hasTag', () => { + describe('apply', () => { + it('returns false when the tag is not present', () => { + expect( + hasTag.apply( + { + 'backstage:accessControl': { + tags: ['foo', 'bar'], + }, + }, + { + tag: 'baz', + }, + ), + ).toEqual(false); + }); + + it('returns false when accessControl is missing', () => { + expect( + hasTag.apply( + {}, + { + tag: 'baz', + }, + ), + ).toEqual(false); + }); + + it('returns false when tags is an empty array', () => { + expect( + hasTag.apply( + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + 'backstage:accessControl': { + tags: [], + }, + }, + }, + { + tag: 'baz', + }, + ), + ).toEqual(false); + }); + + it('returns true when the tag is present', () => { + expect( + hasTag.apply( + { + 'backstage:accessControl': { + tags: ['foo', 'bar'], + }, + }, + { + tag: 'bar', + }, + ), + ).toEqual(true); + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 85bcca3e05..2dcf82dc09 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -22,16 +22,16 @@ import { } from '@backstage/plugin-scaffolder-common'; import { z } from 'zod'; -export const createScaffolderStepPermissionRule = makeCreatePermissionRule< +export const createScaffolderPermissionRule = makeCreatePermissionRule< TemplateEntityStepV1beta3 | TemplateParameter, {}, typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); -const hasTag = createScaffolderStepPermissionRule({ +export const hasTag = createScaffolderPermissionRule({ name: 'HAS_TAG', resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - description: 'Match a scaffolder step with the given tag', + description: `Match parameters or steps with the given tag`, paramsSchema: z.object({ tag: z.string().describe('Name of the tag to match on'), }), From 47e5d7712ab301c9359b57406a5aa05e05fde7f9 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Thu, 2 Feb 2023 14:58:03 +0000 Subject: [PATCH 27/55] Fixed older router tests Signed-off-by: Harry Hogg --- .../src/service/router.test.ts | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 6ebe988e9f..a7d1862f7a 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -44,7 +44,10 @@ import { IdentityApiGetIdentityRequest, BackstageIdentityResponse, } from '@backstage/plugin-auth-node'; -import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { + AuthorizeResult, + PermissionEvaluator, +} from '@backstage/plugin-permission-common'; const mockAccess = jest.fn(); @@ -142,7 +145,14 @@ describe('createRouter', () => { const permissionApi: PermissionEvaluator = { authorize: jest.fn(), - authorizeConditional: jest.fn(), + authorizeConditional: jest.fn().mockResolvedValue([ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]), }; jest.spyOn(taskBroker, 'dispatch'); @@ -753,7 +763,14 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ const permissionApi: PermissionEvaluator = { authorize: jest.fn(), - authorizeConditional: jest.fn(), + authorizeConditional: jest.fn().mockResolvedValue([ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]), }; const router = await createRouter({ From 6663e57fa7116ff0803fa5f524017facc3af52c3 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Fri, 3 Feb 2023 10:44:10 +0000 Subject: [PATCH 28/55] Added tests to check authorized template on get and put endpoint Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- .../src/service/router.test.ts | 188 +++++++++++++++--- .../scaffolder-backend/src/service/router.ts | 1 + 2 files changed, 157 insertions(+), 32 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index a7d1862f7a..2bf9203056 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -89,8 +89,12 @@ describe('createRouter', () => { let loggerSpy: jest.SpyInstance; let taskBroker: TaskBroker; const catalogClient = { getEntityByRef: jest.fn() } as unknown as CatalogApi; + const permissionApi = { + authorize: jest.fn(), + authorizeConditional: jest.fn(), + } as unknown as PermissionEvaluator; - const mockTemplate: TemplateEntityV1beta3 = { + const getMockTemplate = (): TemplateEntityV1beta3 => ({ apiVersion: 'scaffolder.backstage.io/v1beta3', kind: 'Template', metadata: { @@ -117,7 +121,7 @@ describe('createRouter', () => { }, }, }, - }; + }); const mockUser: UserEntity = { apiVersion: 'backstage.io/v1alpha1', @@ -143,18 +147,6 @@ describe('createRouter', () => { }); taskBroker = new StorageTaskBroker(databaseTaskStore, logger); - const permissionApi: PermissionEvaluator = { - authorize: jest.fn(), - authorizeConditional: jest.fn().mockResolvedValue([ - { - result: AuthorizeResult.ALLOW, - }, - { - result: AuthorizeResult.ALLOW, - }, - ]), - }; - jest.spyOn(taskBroker, 'dispatch'); jest.spyOn(taskBroker, 'get'); jest.spyOn(taskBroker, 'list'); @@ -177,15 +169,27 @@ describe('createRouter', () => { .mockImplementation(async ref => { const { kind } = parseEntityRef(ref); - if (kind === 'template') { - return mockTemplate; + if (kind.toLocaleLowerCase() === 'template') { + return getMockTemplate(); } - if (kind === 'user') { + if (kind.toLocaleLowerCase() === 'user') { return mockUser; } + throw new Error(`no mock found for kind: ${kind}`); }); + + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]); }); afterEach(() => { @@ -246,6 +250,7 @@ describe('createRouter', () => { taskBroker.dispatch as jest.Mocked['dispatch']; const mockToken = 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + const mockTemplate = getMockTemplate(); await request(app) .post('/v2/tasks') @@ -301,6 +306,7 @@ describe('createRouter', () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; const mockToken = 'blob.eyJzdWIiOiIiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + const mockTemplate = getMockTemplate(); await request(app) .post('/v2/tasks') @@ -730,6 +736,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); }); }); + describe('providing an identity api', () => { beforeEach(async () => { const logger = getVoidLogger(); @@ -761,18 +768,6 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }, ); - const permissionApi: PermissionEvaluator = { - authorize: jest.fn(), - authorizeConditional: jest.fn().mockResolvedValue([ - { - result: AuthorizeResult.ALLOW, - }, - { - result: AuthorizeResult.ALLOW, - }, - ]), - }; - const router = await createRouter({ logger: logger, config: new ConfigReader({}), @@ -790,15 +785,26 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ .mockImplementation(async ref => { const { kind } = parseEntityRef(ref); - if (kind === 'template') { - return mockTemplate; + if (kind.toLocaleLowerCase() === 'template') { + return getMockTemplate(); } - if (kind === 'user') { + if (kind.toLocaleLowerCase() === 'user') { return mockUser; } throw new Error(`no mock found for kind: ${kind}`); }); + + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]); }); afterEach(() => { @@ -814,6 +820,62 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); }); + describe('GET /v2/templates/:namespace/:kind/:name/parameter-schema', () => { + it('returns the parameter schema', async () => { + const response = await request(app) + .get( + '/v2/templates/default/Template/create-react-app-template/parameter-schema', + ) + .send(); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + title: 'Create React App Template', + description: 'Create a new CRA website project', + steps: [ + { + title: 'Please enter the following information', + schema: { + required: ['required'], + type: 'object', + properties: { + required: { + description: 'Required parameter', + type: 'string', + }, + }, + }, + }, + ], + }); + }); + + it('filters parameters that the user is not authorized to see', async () => { + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + result: AuthorizeResult.DENY, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]); + const response = await request(app) + .get( + '/v2/templates/default/Template/create-react-app-template/parameter-schema', + ) + .send(); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + title: 'Create React App Template', + description: 'Create a new CRA website project', + steps: [], + }); + }); + }); + describe('POST /v2/tasks', () => { it('rejects template values which do not match the template schema definition', async () => { const response = await request(app) @@ -831,6 +893,67 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ expect(response.status).toEqual(400); }); + it('filters steps that the user is not authorized to see', async () => { + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.DENY, + }, + ]); + + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + const mockTemplate = getMockTemplate(); + + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: 'user:default/guest', + secrets: { + backstageToken: 'token', + }, + + spec: { + apiVersion: mockTemplate.apiVersion, + steps: [], + output: mockTemplate.spec.output ?? {}, + parameters: { + required: 'required-value', + }, + user: { + entity: mockUser, + ref: 'user:default/guest', + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind: 'Template', + namespace: 'Default', + name: mockTemplate.metadata?.name, + }), + baseUrl: 'https://dev.azure.com', + entity: { + metadata: mockTemplate.metadata, + }, + }, + }, + }), + ); + }); + it('return the template id', async () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; @@ -857,6 +980,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ it('should call the broker with a correct spec', async () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; + const mockTemplate = getMockTemplate(); await request(app) .post('/v2/tasks') diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index fc410ed634..6bd0b93cc9 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -325,6 +325,7 @@ export async function createRouter( for (const parameters of [template.spec.parameters ?? []].flat()) { const result = validate(values, parameters); + if (!result.valid) { res.status(400).json({ errors: result.errors }); return; From 7fc6227c14255470965920cd017be807c0278664 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Fri, 3 Feb 2023 10:47:48 +0000 Subject: [PATCH 29/55] Updated the backend template to pass permissionApi into scaffolder Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- .../default-app/packages/backend/src/plugins/scaffolder.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts index ef46f07870..fd424a3257 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts @@ -17,5 +17,6 @@ export default async function createPlugin( reader: env.reader, catalogClient, identity: env.identity, + permissionApi: env.permissions, }); } From 691ef9a4dad31791bbd877e503685ca89ff3566f Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Fri, 3 Feb 2023 12:58:08 +0000 Subject: [PATCH 30/55] Updated scaffolder-common api-reports Signed-off-by: Harry Hogg --- plugins/scaffolder-common/api-report.md | 55 ++++++++++++++++--- .../src/TemplateEntityV1beta3.ts | 24 +++++--- plugins/scaffolder-common/src/permissions.ts | 29 ++++++++++ 3 files changed, 91 insertions(+), 17 deletions(-) diff --git a/plugins/scaffolder-common/api-report.md b/plugins/scaffolder-common/api-report.md index fb21ac450f..10e1b09ce6 100644 --- a/plugins/scaffolder-common/api-report.md +++ b/plugins/scaffolder-common/api-report.md @@ -8,8 +8,15 @@ import type { EntityMeta } from '@backstage/catalog-model'; import { JsonObject } from '@backstage/types'; import type { JsonValue } from '@backstage/types'; import { KindValidator } from '@backstage/catalog-model'; +import { ResourcePermission } from '@backstage/plugin-permission-common'; import type { UserEntity } from '@backstage/catalog-model'; +// @alpha +export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; + +// @alpha +export const scaffolderPermissions: ResourcePermission<'scaffolder-template'>[]; + // @public export const isTemplateEntityV1beta3: ( entity: Entity, @@ -42,20 +49,30 @@ export interface TaskStep { name: string; } +// @public +export interface TemplateEntityStepV1beta3 extends JsonObject { + // (undocumented) + 'backstage:accessControl'?: TemplateSpecValuesMetadata; + // (undocumented) + action: string; + // (undocumented) + id?: string; + // (undocumented) + if?: string | boolean; + // (undocumented) + input?: JsonObject; + // (undocumented) + name?: string; +} + // @public export interface TemplateEntityV1beta3 extends Entity { apiVersion: 'scaffolder.backstage.io/v1beta3'; kind: 'Template'; spec: { type: string; - parameters?: JsonObject | JsonObject[]; - steps: Array<{ - id?: string; - name?: string; - action: string; - input?: JsonObject; - if?: string | boolean; - }>; + parameters?: TemplateParameter | TemplateParameter[]; + steps: Array; output?: { [name: string]: string; }; @@ -74,4 +91,26 @@ export type TemplateInfo = { metadata: EntityMeta; }; }; + +// @public +export interface TemplateParameter extends JsonObject { + // (undocumented) + 'backstage:accessControl'?: TemplateSpecValuesMetadata; + // (undocumented) + properties?: { + [name: string]: JsonObject; + }; +} + +// @alpha +export const templateParameterReadPermission: ResourcePermission<'scaffolder-template'>; + +// @public +export interface TemplateSpecValuesMetadata extends JsonObject { + // (undocumented) + tags?: string[]; +} + +// @alpha +export const templateStepReadPermission: ResourcePermission<'scaffolder-template'>; ``` diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index f8c47c7881..99ffdcb7c2 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -68,7 +68,9 @@ export interface TemplateEntityV1beta3 extends Entity { } /** - * TODO + * Step that is part of a Template Entity. + * + * @public */ export interface TemplateEntityStepV1beta3 extends JsonObject { id?: string; @@ -80,20 +82,24 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { } /** - * TODO - */ -export interface TemplateAccessControl extends JsonObject { - tags?: string[]; -} - -/** - * TODO + * Parameter that is part of a Template Entity. + * + * @public */ export interface TemplateParameter extends JsonObject { 'backstage:accessControl'?: TemplateAccessControl; properties?: { [name: string]: JsonObject }; } +/** + * Access control properties for parts of a template. + * + * @public + */ +export interface TemplateAccessControl extends JsonObject { + tags?: string[]; +} + const validator = entityKindSchemaValidator(schema); /** diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index b6f2e1ebf6..858430eaf3 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -16,8 +16,23 @@ import { createPermission } from '@backstage/plugin-permission-common'; +/** + * Permission resource type which corresponds to a scaffolder templates. + * + * @alpha + */ export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; +/** + * This permission is used to authorize actions that involve reading + * one or more parameters from a template. + * + * If this permission is not authorized, it will appear that the + * parameter does not exist in the template — both in the frontend + * and in API responses. + * + * @alpha + */ export const templateParameterReadPermission = createPermission({ name: 'scaffolder.template.parameter.read', attributes: { @@ -26,6 +41,16 @@ export const templateParameterReadPermission = createPermission({ resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, }); +/** + * This permission is used to authorize actions that involve reading + * one or more steps from a template. + * + * If this permission is not authorized, it will appear that the + * step does not exist in the template — both in the frontend + * and in API responses. Steps will also not be executed. + * + * @alpha + */ export const templateStepReadPermission = createPermission({ name: 'scaffolder.template.step.read', attributes: { @@ -34,6 +59,10 @@ export const templateStepReadPermission = createPermission({ resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, }); +/** + * List of all the scaffolder permissions + * @alpha + */ export const scaffolderPermissions = [ templateParameterReadPermission, templateStepReadPermission, From aac120c30d7053fce2bc34dda637964bdbb5c1c7 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Fri, 3 Feb 2023 13:01:32 +0000 Subject: [PATCH 31/55] Updated permission-node api-reports Signed-off-by: Harry Hogg --- plugins/permission-node/api-report.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index f89ff0a8d5..6af78fcebe 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -111,6 +111,11 @@ export const createConditionTransformer: < permissionRules: [...TRules], ) => ConditionTransformer; +// @public +export const createIsAuthorized: ( + rules: PermissionRule[], +) => (decision: PolicyDecision, resource: TResource | undefined) => boolean; + // @public export function createPermissionIntegrationRouter< TResourceType extends string, From 3c2d54e9fb0514bb9d944b674be690a6ecdaabf6 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Fri, 3 Feb 2023 13:10:50 +0000 Subject: [PATCH 32/55] Updated scaffolder-backend api-reports Signed-off-by: Harry Hogg --- plugins/scaffolder-backend/api-report.md | 32 +++++++++++++ .../src/service/conditionExports.ts | 47 +++++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index f0eb714ab3..b4f7a6ee73 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -9,6 +9,8 @@ import { ActionContext as ActionContext_2 } from '@backstage/plugin-scaffolder-n import { CatalogApi } from '@backstage/catalog-client'; import { CatalogProcessor } from '@backstage/plugin-catalog-node'; import { CatalogProcessorEmit } from '@backstage/plugin-catalog-node'; +import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; +import { Conditions } from '@backstage/plugin-permission-node'; import { Config } from '@backstage/config'; import { createPullRequest } from 'octokit-plugin-create-pull-request'; import { Duration } from 'luxon'; @@ -24,8 +26,14 @@ import { LocationSpec } from '@backstage/plugin-catalog-common'; import { Logger } from 'winston'; import { Observable } from '@backstage/types'; import { Octokit } from 'octokit'; +import { PermissionCondition } from '@backstage/plugin-permission-common'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { PermissionRule } from '@backstage/plugin-permission-node'; +import { PermissionRuleParams } from '@backstage/plugin-permission-common'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; +import { ResourcePermission } from '@backstage/plugin-permission-common'; import { Schema } from 'jsonschema'; import { ScmIntegrationRegistry } from '@backstage/integration'; import { ScmIntegrations } from '@backstage/integration'; @@ -35,6 +43,8 @@ import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { TaskSpecV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateAction as TemplateAction_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateActionOptions } from '@backstage/plugin-scaffolder-node'; +import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; +import { TemplateParameter } from '@backstage/plugin-scaffolder-common'; import { UrlReader } from '@backstage/backend-common'; import { Writable } from 'stream'; import { ZodType } from 'zod'; @@ -518,6 +528,14 @@ export const createPublishGitlabMergeRequestAction: (options: { // @public export function createRouter(options: RouterOptions): Promise; +// @alpha +export const createScaffolderConditionalDecision: ( + permission: ResourcePermission<'scaffolder-template'>, + conditions: PermissionCriteria< + PermissionCondition<'scaffolder-template', PermissionRuleParams> + >, +) => ConditionalPolicyDecision; + // @public @deprecated (undocumented) export const createTemplateAction: < TParams, @@ -656,6 +674,8 @@ export interface RouterOptions { // (undocumented) logger: Logger; // (undocumented) + permissionApi: PermissionEvaluator; + // (undocumented) reader: UrlReader; // (undocumented) scheduler?: PluginTaskScheduler; @@ -673,6 +693,18 @@ export type RunCommandOptions = { logStream?: Writable; }; +// @alpha +export const scaffolderConditions: Conditions<{ + hasTag: PermissionRule< + TemplateParameter | TemplateEntityStepV1beta3, + {}, + 'scaffolder-template', + { + tag: string; + } + >; +}>; + // @public (undocumented) export class ScaffolderEntitiesProcessor implements CatalogProcessor { // (undocumented) diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 70285c8f5f..240cbaf858 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -18,13 +18,50 @@ import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder- import { createConditionExports } from '@backstage/plugin-permission-node'; import { scaffolderStepRules } from './rules'; -const { - conditions: scaffolderConditions, - createConditionalDecision: createScaffolderConditionalDecision, -} = createConditionExports({ +const { conditions, createConditionalDecision } = createConditionExports({ pluginId: 'scaffolder', resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, rules: scaffolderStepRules, }); -export { scaffolderConditions, createScaffolderConditionalDecision }; +/** + * These conditions are used when creating conditional decisions for scaffolder + * templates that are returned by authorization policies. + * + * @alpha + */ +export const scaffolderConditions = conditions; + +/** + * `createScaffolderConditionalDecision` can be used when authoring policies to + * create conditional decisions. It requires a permission of type + * `ResourcePermission<'scaffolder-template'>` to be passed as the first parameter. + * It's recommended that you use the provided `isResourcePermission` and + * `isPermission` helper methods to narrow the type of the permission passed to + * the handle method as shown below. + * + * ``` + * // MyAuthorizationPolicy.ts + * ... + * import { createScaffolderPolicyDecision } from '@backstage/plugin-scaffolder-backend'; + * import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common'; + * + * class MyAuthorizationPolicy implements PermissionPolicy { + * async handle(request, user) { + * ... + * + * if (isResourcePermission(request.permission, RESOURCE_TYPE_SCAFFOLDER_TEMPLATE)) { + * return createScaffolderConditionalDecision( + * request.permission, + * { anyOf: [...insert conditions here...] } + * ); + * } + * + * ... + * } + * + * ``` + * + * @alpha + */ +export const createScaffolderConditionalDecision = createConditionalDecision; From 1f03603d8d2e74fe245ceeb7c3131b53dc4bc5b2 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 13 Feb 2023 15:17:52 +0000 Subject: [PATCH 33/55] Renamed 'scaffolderStepRules' to 'scaffolderTemplateRules' Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/service/conditionExports.ts | 4 ++-- plugins/scaffolder-backend/src/service/router.ts | 4 ++-- plugins/scaffolder-backend/src/service/rules.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 240cbaf858..1a36b4d7f3 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -16,12 +16,12 @@ import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common'; import { createConditionExports } from '@backstage/plugin-permission-node'; -import { scaffolderStepRules } from './rules'; +import { scaffolderTemplateRules } from './rules'; const { conditions, createConditionalDecision } = createConditionExports({ pluginId: 'scaffolder', resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - rules: scaffolderStepRules, + rules: scaffolderTemplateRules, }); /** diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 6bd0b93cc9..0a0c45653d 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -58,9 +58,9 @@ import { import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { createIsAuthorized } from '@backstage/plugin-permission-node'; -import { scaffolderStepRules } from './rules'; +import { scaffolderTemplateRules } from './rules'; -const isAuthorized = createIsAuthorized(Object.values(scaffolderStepRules)); +const isAuthorized = createIsAuthorized(Object.values(scaffolderTemplateRules)); /** * RouterOptions diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 2dcf82dc09..0eb012acf3 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -41,4 +41,4 @@ export const hasTag = createScaffolderPermissionRule({ toQuery: () => ({}), }); -export const scaffolderStepRules = { hasTag }; +export const scaffolderTemplateRules = { hasTag }; From 06f58a20f97fe1d960b1c7168e5e3033cb664400 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 27 Feb 2023 16:41:55 +0100 Subject: [PATCH 34/55] scaffolder: accessControl to permissions Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/service/rules.test.ts | 8 ++++---- plugins/scaffolder-backend/src/service/rules.ts | 2 +- plugins/scaffolder-common/api-report.md | 4 ++-- plugins/scaffolder-common/src/TemplateEntityV1beta3.ts | 6 +++--- plugins/scaffolder-common/src/index.ts | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/rules.test.ts b/plugins/scaffolder-backend/src/service/rules.test.ts index 0f6f4851b4..7d8e0eb445 100644 --- a/plugins/scaffolder-backend/src/service/rules.test.ts +++ b/plugins/scaffolder-backend/src/service/rules.test.ts @@ -22,7 +22,7 @@ describe('hasTag', () => { expect( hasTag.apply( { - 'backstage:accessControl': { + 'backstage:permissions': { tags: ['foo', 'bar'], }, }, @@ -33,7 +33,7 @@ describe('hasTag', () => { ).toEqual(false); }); - it('returns false when accessControl is missing', () => { + it('returns false when backstage:permissions is missing', () => { expect( hasTag.apply( {}, @@ -51,7 +51,7 @@ describe('hasTag', () => { apiVersion: 'backstage.io/v1alpha1', kind: 'Component', metadata: { - 'backstage:accessControl': { + 'backstage:permissions': { tags: [], }, }, @@ -67,7 +67,7 @@ describe('hasTag', () => { expect( hasTag.apply( { - 'backstage:accessControl': { + 'backstage:permissions': { tags: ['foo', 'bar'], }, }, diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 0eb012acf3..ba91b18bf5 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -36,7 +36,7 @@ export const hasTag = createScaffolderPermissionRule({ tag: z.string().describe('Name of the tag to match on'), }), apply: (resource, { tag }) => { - return resource['backstage:accessControl']?.tags?.includes(tag) ?? false; + return resource['backstage:permissions']?.tags?.includes(tag) ?? false; }, toQuery: () => ({}), }); diff --git a/plugins/scaffolder-common/api-report.md b/plugins/scaffolder-common/api-report.md index 10e1b09ce6..aba929228f 100644 --- a/plugins/scaffolder-common/api-report.md +++ b/plugins/scaffolder-common/api-report.md @@ -52,7 +52,7 @@ export interface TaskStep { // @public export interface TemplateEntityStepV1beta3 extends JsonObject { // (undocumented) - 'backstage:accessControl'?: TemplateSpecValuesMetadata; + 'backstage:permissions'?: TemplateSpecValuesMetadata; // (undocumented) action: string; // (undocumented) @@ -95,7 +95,7 @@ export type TemplateInfo = { // @public export interface TemplateParameter extends JsonObject { // (undocumented) - 'backstage:accessControl'?: TemplateSpecValuesMetadata; + 'backstage:permissions'?: TemplateSpecValuesMetadata; // (undocumented) properties?: { [name: string]: JsonObject; diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index 99ffdcb7c2..790a0e4409 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -78,7 +78,7 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { action: string; input?: JsonObject; if?: string | boolean; - 'backstage:accessControl'?: TemplateAccessControl; + 'backstage:permissions'?: TemplatePermissions; } /** @@ -87,7 +87,7 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { * @public */ export interface TemplateParameter extends JsonObject { - 'backstage:accessControl'?: TemplateAccessControl; + 'backstage:permissions'?: TemplatePermissions; properties?: { [name: string]: JsonObject }; } @@ -96,7 +96,7 @@ export interface TemplateParameter extends JsonObject { * * @public */ -export interface TemplateAccessControl extends JsonObject { +export interface TemplatePermissions extends JsonObject { tags?: string[]; } diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index 14f89c2bc9..1b154f9ba7 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -30,5 +30,5 @@ export type { TemplateEntityV1beta3, TemplateEntityStepV1beta3, TemplateParameter, - TemplateAccessControl as TemplateSpecValuesMetadata, + TemplatePermissions as TemplateSpecValuesMetadata, } from './TemplateEntityV1beta3'; From dcbd264860256e475a5be19cbc338ad64d47b4ba Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 1 Mar 2023 13:55:21 +0100 Subject: [PATCH 35/55] scaffolder-backend: mark rules utilities as alpha Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-backend/alpha-api-report.md | 29 +++++++++++++++++++ plugins/scaffolder-backend/api-report.md | 29 ------------------- plugins/scaffolder-backend/src/alpha.ts | 1 + plugins/scaffolder-backend/src/index.ts | 1 - 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/plugins/scaffolder-backend/alpha-api-report.md b/plugins/scaffolder-backend/alpha-api-report.md index 3199444c6b..208714a266 100644 --- a/plugins/scaffolder-backend/alpha-api-report.md +++ b/plugins/scaffolder-backend/alpha-api-report.md @@ -4,14 +4,43 @@ ```ts import { BackendFeature } from '@backstage/backend-plugin-api'; +import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; +import { Conditions } from '@backstage/plugin-permission-node'; +import { PermissionCondition } from '@backstage/plugin-permission-common'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; +import { PermissionRule } from '@backstage/plugin-permission-node'; +import { PermissionRuleParams } from '@backstage/plugin-permission-common'; +import { ResourcePermission } from '@backstage/plugin-permission-common'; import { TaskBroker } from '@backstage/plugin-scaffolder-backend'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; +import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateFilter } from '@backstage/plugin-scaffolder-backend'; import { TemplateGlobal } from '@backstage/plugin-scaffolder-backend'; +import { TemplateParameter } from '@backstage/plugin-scaffolder-common'; // @alpha export const catalogModuleTemplateKind: () => BackendFeature; +// @alpha +export const createScaffolderConditionalDecision: ( + permission: ResourcePermission<'scaffolder-template'>, + conditions: PermissionCriteria< + PermissionCondition<'scaffolder-template', PermissionRuleParams> + >, +) => ConditionalPolicyDecision; + +// @alpha +export const scaffolderConditions: Conditions<{ + hasTag: PermissionRule< + TemplateParameter | TemplateEntityStepV1beta3, + {}, + 'scaffolder-template', + { + tag: string; + } + >; +}>; + // @alpha export const scaffolderPlugin: ( options: ScaffolderPluginOptions, diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index b4f7a6ee73..7c5de39d90 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -9,8 +9,6 @@ import { ActionContext as ActionContext_2 } from '@backstage/plugin-scaffolder-n import { CatalogApi } from '@backstage/catalog-client'; import { CatalogProcessor } from '@backstage/plugin-catalog-node'; import { CatalogProcessorEmit } from '@backstage/plugin-catalog-node'; -import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; -import { Conditions } from '@backstage/plugin-permission-node'; import { Config } from '@backstage/config'; import { createPullRequest } from 'octokit-plugin-create-pull-request'; import { Duration } from 'luxon'; @@ -26,14 +24,9 @@ import { LocationSpec } from '@backstage/plugin-catalog-common'; import { Logger } from 'winston'; import { Observable } from '@backstage/types'; import { Octokit } from 'octokit'; -import { PermissionCondition } from '@backstage/plugin-permission-common'; -import { PermissionCriteria } from '@backstage/plugin-permission-common'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; -import { PermissionRule } from '@backstage/plugin-permission-node'; -import { PermissionRuleParams } from '@backstage/plugin-permission-common'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; -import { ResourcePermission } from '@backstage/plugin-permission-common'; import { Schema } from 'jsonschema'; import { ScmIntegrationRegistry } from '@backstage/integration'; import { ScmIntegrations } from '@backstage/integration'; @@ -43,8 +36,6 @@ import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { TaskSpecV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateAction as TemplateAction_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateActionOptions } from '@backstage/plugin-scaffolder-node'; -import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; -import { TemplateParameter } from '@backstage/plugin-scaffolder-common'; import { UrlReader } from '@backstage/backend-common'; import { Writable } from 'stream'; import { ZodType } from 'zod'; @@ -528,14 +519,6 @@ export const createPublishGitlabMergeRequestAction: (options: { // @public export function createRouter(options: RouterOptions): Promise; -// @alpha -export const createScaffolderConditionalDecision: ( - permission: ResourcePermission<'scaffolder-template'>, - conditions: PermissionCriteria< - PermissionCondition<'scaffolder-template', PermissionRuleParams> - >, -) => ConditionalPolicyDecision; - // @public @deprecated (undocumented) export const createTemplateAction: < TParams, @@ -693,18 +676,6 @@ export type RunCommandOptions = { logStream?: Writable; }; -// @alpha -export const scaffolderConditions: Conditions<{ - hasTag: PermissionRule< - TemplateParameter | TemplateEntityStepV1beta3, - {}, - 'scaffolder-template', - { - tag: string; - } - >; -}>; - // @public (undocumented) export class ScaffolderEntitiesProcessor implements CatalogProcessor { // (undocumented) diff --git a/plugins/scaffolder-backend/src/alpha.ts b/plugins/scaffolder-backend/src/alpha.ts index ac96f081ad..6d6be0c156 100644 --- a/plugins/scaffolder-backend/src/alpha.ts +++ b/plugins/scaffolder-backend/src/alpha.ts @@ -15,5 +15,6 @@ */ export * from './modules'; +export * from './service/conditionExports'; export { scaffolderPlugin } from './ScaffolderPlugin'; export type { ScaffolderPluginOptions } from './ScaffolderPlugin'; diff --git a/plugins/scaffolder-backend/src/index.ts b/plugins/scaffolder-backend/src/index.ts index 3dc91f18b9..ba2c25f80a 100644 --- a/plugins/scaffolder-backend/src/index.ts +++ b/plugins/scaffolder-backend/src/index.ts @@ -21,7 +21,6 @@ */ export * from './scaffolder'; -export * from './service/conditionExports'; export * from './service/router'; export * from './lib'; export * from './processor'; From 35c564396bce6d776080db95b9d2bf5fe260e6ea Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 1 Mar 2023 14:17:57 +0100 Subject: [PATCH 36/55] scaffolder: export common permissions utilities as alpha Signed-off-by: Vincenzo Scamporlino --- .../src/service/conditionExports.ts | 2 +- .../scaffolder-backend/src/service/router.ts | 4 +++- .../scaffolder-backend/src/service/rules.ts | 3 ++- plugins/scaffolder-common/alpha-api-report.md | 21 +++++++++++++++++++ plugins/scaffolder-common/api-report.md | 13 ------------ plugins/scaffolder-common/package.json | 15 +++++++++++++ plugins/scaffolder-common/src/alpha.ts | 16 ++++++++++++++ plugins/scaffolder-common/src/index.ts | 2 +- 8 files changed, 59 insertions(+), 17 deletions(-) create mode 100644 plugins/scaffolder-common/alpha-api-report.md create mode 100644 plugins/scaffolder-common/src/alpha.ts diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 1a36b4d7f3..0c502c6075 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; import { createConditionExports } from '@backstage/plugin-permission-node'; import { scaffolderTemplateRules } from './rules'; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0a0c45653d..6a22547b12 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -32,9 +32,11 @@ import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, +} from '@backstage/plugin-scaffolder-common'; +import { templateParameterReadPermission, templateStepReadPermission, -} from '@backstage/plugin-scaffolder-common'; +} from '@backstage/plugin-scaffolder-common/alpha'; import express from 'express'; import Router from 'express-promise-router'; import { validate } from 'jsonschema'; diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index ba91b18bf5..8c9dde50e5 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -16,10 +16,11 @@ import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { - RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, TemplateEntityStepV1beta3, TemplateParameter, } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; + import { z } from 'zod'; export const createScaffolderPermissionRule = makeCreatePermissionRule< diff --git a/plugins/scaffolder-common/alpha-api-report.md b/plugins/scaffolder-common/alpha-api-report.md new file mode 100644 index 0000000000..e89999ad85 --- /dev/null +++ b/plugins/scaffolder-common/alpha-api-report.md @@ -0,0 +1,21 @@ +## API Report File for "@backstage/plugin-scaffolder-common" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts +import { ResourcePermission } from '@backstage/plugin-permission-common'; + +// @alpha +export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; + +// @alpha +export const scaffolderPermissions: ResourcePermission<'scaffolder-template'>[]; + +// @alpha +export const templateParameterReadPermission: ResourcePermission<'scaffolder-template'>; + +// @alpha +export const templateStepReadPermission: ResourcePermission<'scaffolder-template'>; + +// (No @packageDocumentation comment for this package) +``` diff --git a/plugins/scaffolder-common/api-report.md b/plugins/scaffolder-common/api-report.md index aba929228f..5417504eb8 100644 --- a/plugins/scaffolder-common/api-report.md +++ b/plugins/scaffolder-common/api-report.md @@ -8,15 +8,8 @@ import type { EntityMeta } from '@backstage/catalog-model'; import { JsonObject } from '@backstage/types'; import type { JsonValue } from '@backstage/types'; import { KindValidator } from '@backstage/catalog-model'; -import { ResourcePermission } from '@backstage/plugin-permission-common'; import type { UserEntity } from '@backstage/catalog-model'; -// @alpha -export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; - -// @alpha -export const scaffolderPermissions: ResourcePermission<'scaffolder-template'>[]; - // @public export const isTemplateEntityV1beta3: ( entity: Entity, @@ -102,15 +95,9 @@ export interface TemplateParameter extends JsonObject { }; } -// @alpha -export const templateParameterReadPermission: ResourcePermission<'scaffolder-template'>; - // @public export interface TemplateSpecValuesMetadata extends JsonObject { // (undocumented) tags?: string[]; } - -// @alpha -export const templateStepReadPermission: ResourcePermission<'scaffolder-template'>; ``` diff --git a/plugins/scaffolder-common/package.json b/plugins/scaffolder-common/package.json index d315754462..cc79e676fd 100644 --- a/plugins/scaffolder-common/package.json +++ b/plugins/scaffolder-common/package.json @@ -11,6 +11,21 @@ "module": "dist/index.esm.js", "types": "dist/index.d.ts" }, + "exports": { + ".": "./src/index.ts", + "./alpha": "./src/alpha.ts", + "./package.json": "./package.json" + }, + "typesVersions": { + "*": { + "alpha": [ + "src/alpha.ts" + ], + "package.json": [ + "package.json" + ] + } + }, "backstage": { "role": "common-library" }, diff --git a/plugins/scaffolder-common/src/alpha.ts b/plugins/scaffolder-common/src/alpha.ts new file mode 100644 index 0000000000..9a8c021d15 --- /dev/null +++ b/plugins/scaffolder-common/src/alpha.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2023 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. + */ +export * from './permissions'; diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index 1b154f9ba7..a0133632db 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -21,11 +21,11 @@ */ export * from './TaskSpec'; + export { templateEntityV1beta3Validator, isTemplateEntityV1beta3, } from './TemplateEntityV1beta3'; -export * from './permissions'; export type { TemplateEntityV1beta3, TemplateEntityStepV1beta3, From bc82c96a1827901bd1315056beb680af4c00196f Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 7 Mar 2023 22:39:10 +0100 Subject: [PATCH 37/55] scaffolder-backend: make permissionApi optional Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/service/router.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 6a22547b12..d74bfca5b1 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -90,7 +90,7 @@ export interface RouterOptions { taskBroker?: TaskBroker; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; - permissionApi: PermissionEvaluator; + permissionApi?: PermissionEvaluator; identity?: IdentityApi; } @@ -576,6 +576,10 @@ export async function createRouter( ); } + if (!permissionApi) { + return template; + } + const [parameterDecision, stepDecision] = await permissionApi.authorizeConditional( [ From 49d886a151a80a960eaf83db0ef37b630c628e5e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 7 Mar 2023 22:39:53 +0100 Subject: [PATCH 38/55] backend: remove scaffolder-common as dependency Signed-off-by: Vincenzo Scamporlino --- packages/backend/package.json | 1 - yarn.lock | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index aa4e660fdd..54dbe640df 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -60,7 +60,6 @@ "@backstage/plugin-rollbar-backend": "workspace:^", "@backstage/plugin-scaffolder-backend": "workspace:^", "@backstage/plugin-scaffolder-backend-module-rails": "workspace:^", - "@backstage/plugin-scaffolder-common": "workspace:^", "@backstage/plugin-search-backend": "workspace:^", "@backstage/plugin-search-backend-module-elasticsearch": "workspace:^", "@backstage/plugin-search-backend-module-pg": "workspace:^", diff --git a/yarn.lock b/yarn.lock index 0d63604827..33a2cfbda5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -22943,7 +22943,6 @@ __metadata: "@backstage/plugin-rollbar-backend": "workspace:^" "@backstage/plugin-scaffolder-backend": "workspace:^" "@backstage/plugin-scaffolder-backend-module-rails": "workspace:^" - "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/plugin-search-backend": "workspace:^" "@backstage/plugin-search-backend-module-elasticsearch": "workspace:^" "@backstage/plugin-search-backend-module-pg": "workspace:^" From 7ebb4f4349990cda9b757d06070151589065bf81 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 7 Mar 2023 22:42:54 +0100 Subject: [PATCH 39/55] scaffolder-common: rollback properties Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-common/src/TemplateEntityV1beta3.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index 790a0e4409..03563dc3b5 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -88,7 +88,6 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { */ export interface TemplateParameter extends JsonObject { 'backstage:permissions'?: TemplatePermissions; - properties?: { [name: string]: JsonObject }; } /** From e676507491026b223f66541c064147f51d2a1e5f Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 7 Mar 2023 22:43:47 +0100 Subject: [PATCH 40/55] scaffolder: api report Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/api-report.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 7c5de39d90..0f22365c27 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -224,7 +224,7 @@ export function createGithubRepoCreateAction(options: { requiredStatusCheckContexts?: string[] | undefined; requireBranchesToBeUpToDate?: boolean | undefined; requiredConversationResolution?: boolean | undefined; - repoVisibility?: 'internal' | 'private' | 'public' | undefined; + repoVisibility?: 'public' | 'private' | 'internal' | undefined; collaborators?: | ( | { @@ -327,7 +327,7 @@ export function createPublishBitbucketAction(options: { repoUrl: string; description?: string | undefined; defaultBranch?: string | undefined; - repoVisibility?: 'private' | 'public' | undefined; + repoVisibility?: 'public' | 'private' | undefined; sourcePath?: string | undefined; enableLFS?: boolean | undefined; token?: string | undefined; @@ -344,7 +344,7 @@ export function createPublishBitbucketCloudAction(options: { repoUrl: string; description?: string | undefined; defaultBranch?: string | undefined; - repoVisibility?: 'private' | 'public' | undefined; + repoVisibility?: 'public' | 'private' | undefined; sourcePath?: string | undefined; token?: string | undefined; }>; @@ -357,7 +357,7 @@ export function createPublishBitbucketServerAction(options: { repoUrl: string; description?: string | undefined; defaultBranch?: string | undefined; - repoVisibility?: 'private' | 'public' | undefined; + repoVisibility?: 'public' | 'private' | undefined; sourcePath?: string | undefined; enableLFS?: boolean | undefined; token?: string | undefined; @@ -441,7 +441,7 @@ export function createPublishGithubAction(options: { requiredStatusCheckContexts?: string[] | undefined; requireBranchesToBeUpToDate?: boolean | undefined; requiredConversationResolution?: boolean | undefined; - repoVisibility?: 'internal' | 'private' | 'public' | undefined; + repoVisibility?: 'public' | 'private' | 'internal' | undefined; collaborators?: | ( | { @@ -489,7 +489,7 @@ export function createPublishGitlabAction(options: { }): TemplateAction_2<{ repoUrl: string; defaultBranch?: string | undefined; - repoVisibility?: 'internal' | 'private' | 'public' | undefined; + repoVisibility?: 'public' | 'private' | 'internal' | undefined; sourcePath?: string | undefined; token?: string | undefined; gitCommitMessage?: string | undefined; @@ -510,7 +510,7 @@ export const createPublishGitlabMergeRequestAction: (options: { sourcePath?: string | undefined; targetPath?: string | undefined; token?: string | undefined; - commitAction?: 'update' | 'delete' | 'create' | undefined; + commitAction?: 'create' | 'update' | 'delete' | undefined; projectid?: string | undefined; removeSourceBranch?: boolean | undefined; assignee?: string | undefined; @@ -657,7 +657,7 @@ export interface RouterOptions { // (undocumented) logger: Logger; // (undocumented) - permissionApi: PermissionEvaluator; + permissionApi?: PermissionEvaluator; // (undocumented) reader: UrlReader; // (undocumented) From 65e989f4018c29d46097af1166b9c558fc6dce26 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 7 Mar 2023 22:44:22 +0100 Subject: [PATCH 41/55] add changesets Signed-off-by: Vincenzo Scamporlino --- .changeset/clean-dolls-search.md | 5 +++++ .changeset/olive-months-talk.md | 7 +++++++ .changeset/tiny-apes-scream.md | 5 +++++ plugins/scaffolder-common/src/Template.v1beta3.schema.json | 6 ++++++ 4 files changed, 23 insertions(+) create mode 100644 .changeset/clean-dolls-search.md create mode 100644 .changeset/olive-months-talk.md create mode 100644 .changeset/tiny-apes-scream.md diff --git a/.changeset/clean-dolls-search.md b/.changeset/clean-dolls-search.md new file mode 100644 index 0000000000..a8f0da3c4d --- /dev/null +++ b/.changeset/clean-dolls-search.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-common': patch +--- + +Added permissions for authorizing parameters and steps diff --git a/.changeset/olive-months-talk.md b/.changeset/olive-months-talk.md new file mode 100644 index 0000000000..e37832736e --- /dev/null +++ b/.changeset/olive-months-talk.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Added the possibility to authorize parameters and steps of a template + +The scaffolder plugin is now integrated with the permission framework. diff --git a/.changeset/tiny-apes-scream.md b/.changeset/tiny-apes-scream.md new file mode 100644 index 0000000000..b4a28e0745 --- /dev/null +++ b/.changeset/tiny-apes-scream.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-common': patch +--- + +Define optional `backstage:permissions` property to parameters and steps used to authorize part of the template using the permission framework diff --git a/plugins/scaffolder-common/src/Template.v1beta3.schema.json b/plugins/scaffolder-common/src/Template.v1beta3.schema.json index e45bfb788d..3dc1990401 100644 --- a/plugins/scaffolder-common/src/Template.v1beta3.schema.json +++ b/plugins/scaffolder-common/src/Template.v1beta3.schema.json @@ -16,6 +16,9 @@ "owner": "artist-relations-team", "parameters": { "required": ["name", "description", "repoUrl"], + "backstage:permissions": { + "tags": ["one", "two"] + }, "properties": { "name": { "title": "Name", @@ -41,6 +44,9 @@ "action": "fetch:plain", "parameters": { "url": "./template" + }, + "backstage:permissions": { + "tags": ["one", "two"] } }, { From c42a1159ad0099096d5e498b7e30161a989da610 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 7 Mar 2023 23:06:59 +0100 Subject: [PATCH 42/55] scaffolder: api report Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/api-report.md | 14 +++++++------- plugins/scaffolder-common/api-report.md | 4 ---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 0f22365c27..2cb969fa41 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -224,7 +224,7 @@ export function createGithubRepoCreateAction(options: { requiredStatusCheckContexts?: string[] | undefined; requireBranchesToBeUpToDate?: boolean | undefined; requiredConversationResolution?: boolean | undefined; - repoVisibility?: 'public' | 'private' | 'internal' | undefined; + repoVisibility?: 'internal' | 'private' | 'public' | undefined; collaborators?: | ( | { @@ -327,7 +327,7 @@ export function createPublishBitbucketAction(options: { repoUrl: string; description?: string | undefined; defaultBranch?: string | undefined; - repoVisibility?: 'public' | 'private' | undefined; + repoVisibility?: 'private' | 'public' | undefined; sourcePath?: string | undefined; enableLFS?: boolean | undefined; token?: string | undefined; @@ -344,7 +344,7 @@ export function createPublishBitbucketCloudAction(options: { repoUrl: string; description?: string | undefined; defaultBranch?: string | undefined; - repoVisibility?: 'public' | 'private' | undefined; + repoVisibility?: 'private' | 'public' | undefined; sourcePath?: string | undefined; token?: string | undefined; }>; @@ -357,7 +357,7 @@ export function createPublishBitbucketServerAction(options: { repoUrl: string; description?: string | undefined; defaultBranch?: string | undefined; - repoVisibility?: 'public' | 'private' | undefined; + repoVisibility?: 'private' | 'public' | undefined; sourcePath?: string | undefined; enableLFS?: boolean | undefined; token?: string | undefined; @@ -441,7 +441,7 @@ export function createPublishGithubAction(options: { requiredStatusCheckContexts?: string[] | undefined; requireBranchesToBeUpToDate?: boolean | undefined; requiredConversationResolution?: boolean | undefined; - repoVisibility?: 'public' | 'private' | 'internal' | undefined; + repoVisibility?: 'internal' | 'private' | 'public' | undefined; collaborators?: | ( | { @@ -489,7 +489,7 @@ export function createPublishGitlabAction(options: { }): TemplateAction_2<{ repoUrl: string; defaultBranch?: string | undefined; - repoVisibility?: 'public' | 'private' | 'internal' | undefined; + repoVisibility?: 'internal' | 'private' | 'public' | undefined; sourcePath?: string | undefined; token?: string | undefined; gitCommitMessage?: string | undefined; @@ -510,7 +510,7 @@ export const createPublishGitlabMergeRequestAction: (options: { sourcePath?: string | undefined; targetPath?: string | undefined; token?: string | undefined; - commitAction?: 'create' | 'update' | 'delete' | undefined; + commitAction?: 'update' | 'delete' | 'create' | undefined; projectid?: string | undefined; removeSourceBranch?: boolean | undefined; assignee?: string | undefined; diff --git a/plugins/scaffolder-common/api-report.md b/plugins/scaffolder-common/api-report.md index 5417504eb8..02e52cc01d 100644 --- a/plugins/scaffolder-common/api-report.md +++ b/plugins/scaffolder-common/api-report.md @@ -89,10 +89,6 @@ export type TemplateInfo = { export interface TemplateParameter extends JsonObject { // (undocumented) 'backstage:permissions'?: TemplateSpecValuesMetadata; - // (undocumented) - properties?: { - [name: string]: JsonObject; - }; } // @public From 67edf386c69debadb716a43c95687e4a2530cfc8 Mon Sep 17 00:00:00 2001 From: Ainhoa Larumbe Date: Wed, 8 Mar 2023 17:16:34 +0000 Subject: [PATCH 43/55] scaffolder-backend: add metadata endpoint to expose template permissions Signed-off-by: Ainhoa Larumbe --- plugins/scaffolder-backend/src/service/router.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index d74bfca5b1..f6415fae24 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -34,6 +34,8 @@ import { templateEntityV1beta3Validator, } from '@backstage/plugin-scaffolder-common'; import { + RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + scaffolderPermissions, templateParameterReadPermission, templateStepReadPermission, } from '@backstage/plugin-scaffolder-common/alpha'; @@ -59,7 +61,10 @@ import { } from '@backstage/plugin-auth-node'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; -import { createIsAuthorized } from '@backstage/plugin-permission-node'; +import { + createIsAuthorized, + createPermissionIntegrationRouter, +} from '@backstage/plugin-permission-node'; import { scaffolderTemplateRules } from './rules'; const isAuthorized = createIsAuthorized(Object.values(scaffolderTemplateRules)); @@ -261,6 +266,14 @@ export async function createRouter( additionalTemplateGlobals, }); + const permissionIntegrationRouter = createPermissionIntegrationRouter({ + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + permissions: scaffolderPermissions, + rules: Object.values(scaffolderTemplateRules), + }); + + router.use(permissionIntegrationRouter); + router .get( '/v2/templates/:namespace/:kind/:name/parameter-schema', From 51182944212adf817cb79fb5374e7d7338bff92c Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 9 Mar 2023 12:27:58 +0100 Subject: [PATCH 44/55] scaffolder: validate backstage:permissions in templates Signed-off-by: Vincenzo Scamporlino --- .../src/Template.v1beta3.schema.json | 44 ++++++++++++++++++- .../src/TemplateEntityV1beta3.test.ts | 20 ++++++++- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-common/src/Template.v1beta3.schema.json b/plugins/scaffolder-common/src/Template.v1beta3.schema.json index 3dc1990401..beb0ca5e31 100644 --- a/plugins/scaffolder-common/src/Template.v1beta3.schema.json +++ b/plugins/scaffolder-common/src/Template.v1beta3.schema.json @@ -98,14 +98,42 @@ "oneOf": [ { "type": "object", - "description": "The JSONSchema describing the inputs for the template." + "description": "The JSONSchema describing the inputs for the template.", + "properties": { + "backstage:permissions": { + "type": "object", + "description": "Object used for authorizing the parameter", + "properties": { + "tags": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } }, { "type": "array", "description": "A list of separate forms to collect parameters.", "items": { "type": "object", - "description": "The JSONSchema describing the inputs for the template." + "description": "The JSONSchema describing the inputs for the template.", + "properties": { + "backstage:permissions": { + "type": "object", + "description": "Object used for authorizing the parameter", + "properties": { + "tags": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } } } ] @@ -137,6 +165,18 @@ "if": { "type": ["string", "boolean"], "description": "A templated condition that skips the step when evaluated to false. If the condition is true or not defined, the step is executed. The condition is true, if the input is not `false`, `undefined`, `null`, `\"\"`, `0`, or `[]`." + }, + "backstage:permissions": { + "type": "object", + "description": "Object used for authorizing the step", + "properties": { + "tags": { + "type": "array", + "items": { + "type": "string" + } + } + } } } } diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts index ab5900ab32..a23d72f290 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts @@ -15,7 +15,10 @@ */ import { entityKindSchemaValidator } from '@backstage/catalog-model'; -import type { TemplateEntityV1beta3 } from './TemplateEntityV1beta3'; +import type { + TemplateEntityV1beta3, + TemplateParameter, +} from './TemplateEntityV1beta3'; import schema from './Template.v1beta3.schema.json'; const validator = entityKindSchemaValidator(schema); @@ -35,6 +38,7 @@ describe('templateEntityV1beta3Validator', () => { owner: 'team-b', parameters: { required: ['owner'], + 'backstage:permissions': { tags: ['one', 'two'] }, properties: { owner: { type: 'string', @@ -52,6 +56,9 @@ describe('templateEntityV1beta3Validator', () => { url: './template', }, if: '${{ parameters.owner }}', + 'backstage:permissions': { + tags: ['one', 'two'], + }, }, ], output: { @@ -154,4 +161,15 @@ describe('templateEntityV1beta3Validator', () => { (entity as any).spec.steps[0].if = 5; expect(() => validator(entity)).toThrow(/if/); }); + + it('rejects parameters with wrong backstage:permissions', async () => { + (entity.spec.parameters as TemplateParameter)['backstage:permissions'] = + true as {}; + expect(() => validator(entity)).toThrow(/must be object/); + }); + + it('rejects steps with wrong backstage:permissions', async () => { + entity.spec.steps[0]['backstage:permissions'] = true as {}; + expect(() => validator(entity)).toThrow(/must be object/); + }); }); From 6e5e1b49c04b97ca8d7ac181cbefce1cc63d92ce Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 9 Mar 2023 12:32:19 +0100 Subject: [PATCH 45/55] permission-node rename createIsAuthorized to createConditionAuthorizer Signed-off-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.test.ts | 10 +++++----- .../integration/createPermissionIntegrationRouter.ts | 2 +- plugins/scaffolder-backend/src/service/router.ts | 6 ++++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index e7091e7936..6e28c5ddc4 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -25,7 +25,7 @@ import { z } from 'zod'; import { createPermissionIntegrationRouter, CreatePermissionIntegrationRouterResourceOptions, - createIsAuthorized, + createConditionAuthorizer, } from './createPermissionIntegrationRouter'; import { createPermissionRule } from './createPermissionRule'; @@ -633,13 +633,13 @@ describe('createPermissionIntegrationRouter', () => { }); }); -describe('createIsAuthorized', () => { +describe('createConditionAuthorizer', () => { afterEach(() => { jest.clearAllMocks(); }); it('should return true in case of allowed decision', () => { - const isAuthorized = createIsAuthorized([testRule1, testRule2]); + const isAuthorized = createConditionAuthorizer([testRule1, testRule2]); expect( isAuthorized( { @@ -653,7 +653,7 @@ describe('createIsAuthorized', () => { expect(mockTestRule2Apply).not.toHaveBeenCalled(); }); it('should return false in case of denied decision', () => { - const isAuthorized = createIsAuthorized([testRule1, testRule2]); + const isAuthorized = createConditionAuthorizer([testRule1, testRule2]); expect( isAuthorized( { @@ -668,7 +668,7 @@ describe('createIsAuthorized', () => { }); it('should apply conditions to a resource in case of conditional decision', () => { - const isAuthorized = createIsAuthorized([testRule1, testRule2]); + const isAuthorized = createConditionAuthorizer([testRule1, testRule2]); expect( isAuthorized( { diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 5b0c2c25f1..850123d55e 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -168,7 +168,7 @@ const applyConditions = ( * * @public */ -export const createIsAuthorized = ( +export const createConditionAuthorizer = ( rules: PermissionRule[], ) => { const getRule = createGetRule(rules); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index f6415fae24..c1f9a15831 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -62,12 +62,14 @@ import { import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { - createIsAuthorized, + createConditionAuthorizer, createPermissionIntegrationRouter, } from '@backstage/plugin-permission-node'; import { scaffolderTemplateRules } from './rules'; -const isAuthorized = createIsAuthorized(Object.values(scaffolderTemplateRules)); +const isAuthorized = createConditionAuthorizer( + Object.values(scaffolderTemplateRules), +); /** * RouterOptions From bb0addebd5c4960e4b8a1db85a2358db39b581d4 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 9 Mar 2023 12:37:29 +0100 Subject: [PATCH 46/55] scaffolder: tests for validating tags Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-common/src/TemplateEntityV1beta3.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts index a23d72f290..9358e08df5 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts @@ -163,12 +163,20 @@ describe('templateEntityV1beta3Validator', () => { }); it('rejects parameters with wrong backstage:permissions', async () => { + (entity.spec.parameters as TemplateParameter)[ + 'backstage:permissions' + ]!.tags = true as unknown as []; + expect(() => validator(entity)).toThrow(/must be array/); + (entity.spec.parameters as TemplateParameter)['backstage:permissions'] = true as {}; expect(() => validator(entity)).toThrow(/must be object/); }); it('rejects steps with wrong backstage:permissions', async () => { + entity.spec.steps[0]['backstage:permissions']!.tags = true as unknown as []; + expect(() => validator(entity)).toThrow(/must be array/); + entity.spec.steps[0]['backstage:permissions'] = true as {}; expect(() => validator(entity)).toThrow(/must be object/); }); From a89a930e8a1137a21572b36fc8cf72108e26ebb1 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 13 Mar 2023 15:03:57 +0100 Subject: [PATCH 47/55] scaffolder: mark new models as v1beta3 Signed-off-by: Vincenzo Scamporlino --- plugins/permission-node/api-report.md | 10 +++++----- plugins/scaffolder-backend/alpha-api-report.md | 4 ++-- plugins/scaffolder-backend/src/service/rules.ts | 4 ++-- plugins/scaffolder-common/api-report.md | 10 +++++----- .../src/TemplateEntityV1beta3.test.ts | 9 +++++---- plugins/scaffolder-common/src/TemplateEntityV1beta3.ts | 10 +++++----- plugins/scaffolder-common/src/index.ts | 4 ++-- 7 files changed, 26 insertions(+), 25 deletions(-) diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 6af78fcebe..b3d06efdc7 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -73,6 +73,11 @@ export type ConditionTransformer = ( conditions: PermissionCriteria, ) => PermissionCriteria; +// @public +export const createConditionAuthorizer: ( + rules: PermissionRule[], +) => (decision: PolicyDecision, resource: TResource | undefined) => boolean; + // @public export const createConditionExports: < TResourceType extends string, @@ -111,11 +116,6 @@ export const createConditionTransformer: < permissionRules: [...TRules], ) => ConditionTransformer; -// @public -export const createIsAuthorized: ( - rules: PermissionRule[], -) => (decision: PolicyDecision, resource: TResource | undefined) => boolean; - // @public export function createPermissionIntegrationRouter< TResourceType extends string, diff --git a/plugins/scaffolder-backend/alpha-api-report.md b/plugins/scaffolder-backend/alpha-api-report.md index 208714a266..7246d768e3 100644 --- a/plugins/scaffolder-backend/alpha-api-report.md +++ b/plugins/scaffolder-backend/alpha-api-report.md @@ -16,7 +16,7 @@ import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateFilter } from '@backstage/plugin-scaffolder-backend'; import { TemplateGlobal } from '@backstage/plugin-scaffolder-backend'; -import { TemplateParameter } from '@backstage/plugin-scaffolder-common'; +import { TemplateParameterV1beta3 } from '@backstage/plugin-scaffolder-common'; // @alpha export const catalogModuleTemplateKind: () => BackendFeature; @@ -32,7 +32,7 @@ export const createScaffolderConditionalDecision: ( // @alpha export const scaffolderConditions: Conditions<{ hasTag: PermissionRule< - TemplateParameter | TemplateEntityStepV1beta3, + TemplateParameterV1beta3 | TemplateEntityStepV1beta3, {}, 'scaffolder-template', { diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 8c9dde50e5..db7ad993e9 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -17,14 +17,14 @@ import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { TemplateEntityStepV1beta3, - TemplateParameter, + TemplateParameterV1beta3, } from '@backstage/plugin-scaffolder-common'; import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; import { z } from 'zod'; export const createScaffolderPermissionRule = makeCreatePermissionRule< - TemplateEntityStepV1beta3 | TemplateParameter, + TemplateEntityStepV1beta3 | TemplateParameterV1beta3, {}, typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); diff --git a/plugins/scaffolder-common/api-report.md b/plugins/scaffolder-common/api-report.md index 02e52cc01d..61d6d51a78 100644 --- a/plugins/scaffolder-common/api-report.md +++ b/plugins/scaffolder-common/api-report.md @@ -45,7 +45,7 @@ export interface TaskStep { // @public export interface TemplateEntityStepV1beta3 extends JsonObject { // (undocumented) - 'backstage:permissions'?: TemplateSpecValuesMetadata; + 'backstage:permissions'?: TemplatePermissionsV1beta3; // (undocumented) action: string; // (undocumented) @@ -64,7 +64,7 @@ export interface TemplateEntityV1beta3 extends Entity { kind: 'Template'; spec: { type: string; - parameters?: TemplateParameter | TemplateParameter[]; + parameters?: TemplateParameterV1beta3 | TemplateParameterV1beta3[]; steps: Array; output?: { [name: string]: string; @@ -86,13 +86,13 @@ export type TemplateInfo = { }; // @public -export interface TemplateParameter extends JsonObject { +export interface TemplateParameterV1beta3 extends JsonObject { // (undocumented) - 'backstage:permissions'?: TemplateSpecValuesMetadata; + 'backstage:permissions'?: TemplatePermissionsV1beta3; } // @public -export interface TemplateSpecValuesMetadata extends JsonObject { +export interface TemplatePermissionsV1beta3 extends JsonObject { // (undocumented) tags?: string[]; } diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts index 9358e08df5..4609a7802a 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts @@ -17,7 +17,7 @@ import { entityKindSchemaValidator } from '@backstage/catalog-model'; import type { TemplateEntityV1beta3, - TemplateParameter, + TemplateParameterV1beta3, } from './TemplateEntityV1beta3'; import schema from './Template.v1beta3.schema.json'; @@ -163,13 +163,14 @@ describe('templateEntityV1beta3Validator', () => { }); it('rejects parameters with wrong backstage:permissions', async () => { - (entity.spec.parameters as TemplateParameter)[ + (entity.spec.parameters as TemplateParameterV1beta3)[ 'backstage:permissions' ]!.tags = true as unknown as []; expect(() => validator(entity)).toThrow(/must be array/); - (entity.spec.parameters as TemplateParameter)['backstage:permissions'] = - true as {}; + (entity.spec.parameters as TemplateParameterV1beta3)[ + 'backstage:permissions' + ] = true as {}; expect(() => validator(entity)).toThrow(/must be object/); }); diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index 03563dc3b5..5d002ddfe4 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -50,7 +50,7 @@ export interface TemplateEntityV1beta3 extends Entity { * to collect user input and validate it against that schema. This can then be used in the `steps` part below to template * variables passed from the user into each action in the template. */ - parameters?: TemplateParameter | TemplateParameter[]; + parameters?: TemplateParameterV1beta3 | TemplateParameterV1beta3[]; /** * A list of steps to be executed in sequence which are defined by the template. These steps are a list of the underlying * javascript action and some optional input parameters that may or may not have been collected from the end user. @@ -78,7 +78,7 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { action: string; input?: JsonObject; if?: string | boolean; - 'backstage:permissions'?: TemplatePermissions; + 'backstage:permissions'?: TemplatePermissionsV1beta3; } /** @@ -86,8 +86,8 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { * * @public */ -export interface TemplateParameter extends JsonObject { - 'backstage:permissions'?: TemplatePermissions; +export interface TemplateParameterV1beta3 extends JsonObject { + 'backstage:permissions'?: TemplatePermissionsV1beta3; } /** @@ -95,7 +95,7 @@ export interface TemplateParameter extends JsonObject { * * @public */ -export interface TemplatePermissions extends JsonObject { +export interface TemplatePermissionsV1beta3 extends JsonObject { tags?: string[]; } diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index a0133632db..701cad1f71 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -29,6 +29,6 @@ export { export type { TemplateEntityV1beta3, TemplateEntityStepV1beta3, - TemplateParameter, - TemplatePermissions as TemplateSpecValuesMetadata, + TemplateParameterV1beta3, + TemplatePermissionsV1beta3, } from './TemplateEntityV1beta3'; From b3ae19452648a214e6283f172b0e8726057f19d5 Mon Sep 17 00:00:00 2001 From: Claire Casey Date: Tue, 14 Mar 2023 13:51:11 -0400 Subject: [PATCH 48/55] add optional custom rules to scaffolder router Signed-off-by: Claire Casey --- .../scaffolder-backend/src/service/router.ts | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index c1f9a15831..acfec5dc90 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -32,6 +32,8 @@ import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, + TemplateParameterV1beta3, + TemplateEntityStepV1beta3, } from '@backstage/plugin-scaffolder-common'; import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, @@ -60,16 +62,30 @@ import { IdentityApiGetIdentityRequest, } from '@backstage/plugin-auth-node'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; -import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { + PermissionEvaluator, + PermissionRuleParams, +} from '@backstage/plugin-permission-common'; import { createConditionAuthorizer, createPermissionIntegrationRouter, + PermissionRule, } from '@backstage/plugin-permission-node'; import { scaffolderTemplateRules } from './rules'; -const isAuthorized = createConditionAuthorizer( - Object.values(scaffolderTemplateRules), -); +/** + * ScaffolderPermissionRuleInput + * + * @public + */ +export type ScaffolderPermissionRuleInput< + TParams extends PermissionRuleParams = PermissionRuleParams, +> = PermissionRule< + TemplateEntityStepV1beta3 | TemplateParameterV1beta3, + {}, + typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + TParams +>; /** * RouterOptions @@ -98,6 +114,7 @@ export interface RouterOptions { additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; permissionApi?: PermissionEvaluator; + customPermissionRules?: ScaffolderPermissionRuleInput[]; identity?: IdentityApi; } @@ -193,6 +210,7 @@ export async function createRouter( additionalTemplateFilters, additionalTemplateGlobals, permissionApi, + customPermissionRules, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); @@ -268,10 +286,21 @@ export async function createRouter( additionalTemplateGlobals, }); + const permissionRules: ScaffolderPermissionRuleInput[] = Object.values( + scaffolderTemplateRules, + ); + if (customPermissionRules) { + permissionRules.push(...customPermissionRules); + } + + const isAuthorized = createConditionAuthorizer( + Object.values(permissionRules), + ); + const permissionIntegrationRouter = createPermissionIntegrationRouter({ resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, permissions: scaffolderPermissions, - rules: Object.values(scaffolderTemplateRules), + rules: permissionRules, }); router.use(permissionIntegrationRouter); From 4722fd4c207fb4d4d2b0a8239bfa0e6fafe28c04 Mon Sep 17 00:00:00 2001 From: Claire Casey Date: Wed, 15 Mar 2023 11:48:25 -0400 Subject: [PATCH 49/55] add tests for checking conditional permissions and update api-reports Signed-off-by: Claire Casey --- plugins/scaffolder-backend/api-report.md | 17 ++ .../src/service/router.test.ts | 258 +++++++++++++++--- 2 files changed, 244 insertions(+), 31 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 2cb969fa41..c9f92d6172 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -25,8 +25,11 @@ import { Logger } from 'winston'; import { Observable } from '@backstage/types'; import { Octokit } from 'octokit'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { PermissionRule } from '@backstage/plugin-permission-node'; +import { PermissionRuleParams } from '@backstage/plugin-permission-common'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; +import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; import { Schema } from 'jsonschema'; import { ScmIntegrationRegistry } from '@backstage/integration'; import { ScmIntegrations } from '@backstage/integration'; @@ -36,6 +39,8 @@ import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { TaskSpecV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateAction as TemplateAction_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateActionOptions } from '@backstage/plugin-scaffolder-node'; +import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; +import { TemplateParameterV1beta3 } from '@backstage/plugin-scaffolder-common'; import { UrlReader } from '@backstage/backend-common'; import { Writable } from 'stream'; import { ZodType } from 'zod'; @@ -651,6 +656,8 @@ export interface RouterOptions { // (undocumented) config: Config; // (undocumented) + customPermissionRules?: ScaffolderPermissionRuleInput[]; + // (undocumented) database: PluginDatabaseManager; // (undocumented) identity?: IdentityApi; @@ -690,6 +697,16 @@ export class ScaffolderEntitiesProcessor implements CatalogProcessor { validateEntityKind(entity: Entity): Promise; } +// @public +export type ScaffolderPermissionRuleInput< + TParams extends PermissionRuleParams = PermissionRuleParams, +> = PermissionRule< + TemplateEntityStepV1beta3 | TemplateParameterV1beta3, + {}, + typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + TParams +>; + // @public export type SerializedTask = { id: string; diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 2bf9203056..4f5ed0fe22 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -109,17 +109,52 @@ describe('createRouter', () => { spec: { owner: 'web@example.com', type: 'website', - steps: [], - parameters: { - type: 'object', - required: ['required'], - properties: { - required: { - type: 'string', - description: 'Required parameter', + steps: [ + { + id: 'step-one', + name: 'First log', + action: 'debug:log', + input: { + message: 'hello', }, }, - }, + { + id: 'step-two', + name: 'Second log', + action: 'debug:log', + input: { + message: 'world', + }, + 'backstage:permissions': { + tags: ['steps-tag'], + }, + }, + ], + parameters: [ + { + type: 'object', + required: ['requiredParameter1'], + properties: { + requiredParameter1: { + type: 'string', + description: 'Required parameter 1', + }, + }, + }, + { + type: 'object', + required: ['requiredParameter2'], + 'backstage:permissions': { + tags: ['parameters-tag'], + }, + properties: { + requiredParameter2: { + type: 'string', + description: 'Required parameter 2', + }, + }, + }, + ], }, }); @@ -237,12 +272,13 @@ describe('createRouter', () => { name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); - expect(response.body.id).toBe('a-random-id'); expect(response.status).toEqual(201); + expect(response.body.id).toBe('a-random-id'); }); it('should call the broker with a correct spec', async () => { @@ -261,7 +297,8 @@ describe('createRouter', () => { name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); expect(broker).toHaveBeenCalledWith( @@ -280,7 +317,8 @@ describe('createRouter', () => { })), output: mockTemplate.spec.output ?? {}, parameters: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, user: { entity: mockUser, @@ -317,7 +355,8 @@ describe('createRouter', () => { name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); expect(broker).toHaveBeenCalledWith( @@ -336,7 +375,8 @@ describe('createRouter', () => { })), output: mockTemplate.spec.output ?? {}, parameters: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, user: { entity: undefined, @@ -370,7 +410,8 @@ describe('createRouter', () => { name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); @@ -393,7 +434,8 @@ describe('createRouter', () => { name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); @@ -416,7 +458,8 @@ describe('createRouter', () => { name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); @@ -836,16 +879,32 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ { title: 'Please enter the following information', schema: { - required: ['required'], + required: ['requiredParameter1'], type: 'object', properties: { - required: { - description: 'Required parameter', + requiredParameter1: { + description: 'Required parameter 1', type: 'string', }, }, }, }, + { + title: 'Please enter the following information', + schema: { + type: 'object', + required: ['requiredParameter2'], + 'backstage:permissions': { + tags: ['parameters-tag'], + }, + properties: { + requiredParameter2: { + type: 'string', + description: 'Required parameter 2', + }, + }, + }, + }, ], }); }); @@ -866,7 +925,6 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ '/v2/templates/default/Template/create-react-app-template/parameter-schema', ) .send(); - expect(response.status).toEqual(200); expect(response.body).toEqual({ title: 'Create React App Template', @@ -874,6 +932,54 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ steps: [], }); }); + + it('filters parameters that the user is not authorized to see in case of conditional decision', async () => { + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + conditions: { + resourceType: 'scaffolder-template', + rule: 'HAS_TAG', + params: { tag: 'parameters-tag' }, + }, + pluginId: 'scaffolder', + resourceType: 'scaffolder-template', + result: AuthorizeResult.CONDITIONAL, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]); + const response = await request(app) + .get( + '/v2/templates/default/Template/create-react-app-template/parameter-schema', + ) + .send(); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + title: 'Create React App Template', + description: 'Create a new CRA website project', + steps: [ + { + title: 'Please enter the following information', + schema: { + type: 'object', + required: ['requiredParameter2'], + 'backstage:permissions': { + tags: ['parameters-tag'], + }, + properties: { + requiredParameter2: { + type: 'string', + description: 'Required parameter 2', + }, + }, + }, + }, + ], + }); + }); }); describe('POST /v2/tasks', () => { @@ -917,7 +1023,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); expect(broker).toHaveBeenCalledWith( @@ -932,7 +1039,89 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ steps: [], output: mockTemplate.spec.output ?? {}, parameters: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', + }, + user: { + entity: mockUser, + ref: 'user:default/guest', + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind: 'Template', + namespace: 'Default', + name: mockTemplate.metadata?.name, + }), + baseUrl: 'https://dev.azure.com', + entity: { + metadata: mockTemplate.metadata, + }, + }, + }, + }), + ); + }); + + it('filters steps that the user is not authorized to see in case of conditional decision', async () => { + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + { + conditions: { + resourceType: 'scaffolder-template', + rule: 'HAS_TAG', + params: { tag: 'steps-tag' }, + }, + pluginId: 'scaffolder', + resourceType: 'scaffolder-template', + result: AuthorizeResult.CONDITIONAL, + }, + ]); + + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + const mockTemplate = getMockTemplate(); + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', + }, + }); + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: 'user:default/guest', + secrets: { + backstageToken: 'token', + }, + + spec: { + apiVersion: mockTemplate.apiVersion, + steps: [ + { + id: 'step-two', + name: 'Second log', + action: 'debug:log', + input: { + message: 'world', + }, + 'backstage:permissions': { + tags: ['steps-tag'], + }, + }, + ], + output: mockTemplate.spec.output ?? {}, + parameters: { + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, user: { entity: mockUser, @@ -969,7 +1158,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); @@ -990,7 +1180,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); expect(broker).toHaveBeenCalledWith( @@ -1009,7 +1200,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ })), output: mockTemplate.spec.output ?? {}, parameters: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, user: { entity: mockUser, @@ -1052,7 +1244,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); expect(response.status).not.toEqual(201); @@ -1084,7 +1277,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); @@ -1107,7 +1301,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); @@ -1127,7 +1322,8 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ name: 'create-react-app-template', }), values: { - required: 'required-value', + requiredParameter1: 'required-value-1', + requiredParameter2: 'required-value-2', }, }); From 9deb0f66eccf0baf19d32fb891e20e47d55b6465 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 16 Mar 2023 23:57:11 +0100 Subject: [PATCH 50/55] scaffolder: TemplateParameterV1beta3 to TemplateParametersV1beta3 Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/alpha-api-report.md | 4 ++-- plugins/scaffolder-backend/api-report.md | 4 ++-- plugins/scaffolder-backend/src/service/router.ts | 4 ++-- plugins/scaffolder-backend/src/service/rules.ts | 4 ++-- plugins/scaffolder-common/api-report.md | 4 ++-- plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts | 6 +++--- plugins/scaffolder-common/src/TemplateEntityV1beta3.ts | 4 ++-- plugins/scaffolder-common/src/index.ts | 2 +- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/plugins/scaffolder-backend/alpha-api-report.md b/plugins/scaffolder-backend/alpha-api-report.md index 7246d768e3..2ecd2cc800 100644 --- a/plugins/scaffolder-backend/alpha-api-report.md +++ b/plugins/scaffolder-backend/alpha-api-report.md @@ -16,7 +16,7 @@ import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateFilter } from '@backstage/plugin-scaffolder-backend'; import { TemplateGlobal } from '@backstage/plugin-scaffolder-backend'; -import { TemplateParameterV1beta3 } from '@backstage/plugin-scaffolder-common'; +import { TemplateParametersV1beta3 } from '@backstage/plugin-scaffolder-common'; // @alpha export const catalogModuleTemplateKind: () => BackendFeature; @@ -32,7 +32,7 @@ export const createScaffolderConditionalDecision: ( // @alpha export const scaffolderConditions: Conditions<{ hasTag: PermissionRule< - TemplateParameterV1beta3 | TemplateEntityStepV1beta3, + TemplateParametersV1beta3 | TemplateEntityStepV1beta3, {}, 'scaffolder-template', { diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index c9f92d6172..087b795c62 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -40,7 +40,7 @@ import { TaskSpecV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateAction as TemplateAction_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateActionOptions } from '@backstage/plugin-scaffolder-node'; import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; -import { TemplateParameterV1beta3 } from '@backstage/plugin-scaffolder-common'; +import { TemplateParametersV1beta3 } from '@backstage/plugin-scaffolder-common'; import { UrlReader } from '@backstage/backend-common'; import { Writable } from 'stream'; import { ZodType } from 'zod'; @@ -701,7 +701,7 @@ export class ScaffolderEntitiesProcessor implements CatalogProcessor { export type ScaffolderPermissionRuleInput< TParams extends PermissionRuleParams = PermissionRuleParams, > = PermissionRule< - TemplateEntityStepV1beta3 | TemplateParameterV1beta3, + TemplateEntityStepV1beta3 | TemplateParametersV1beta3, {}, typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, TParams diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index acfec5dc90..f50c83ed3f 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -32,7 +32,7 @@ import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, - TemplateParameterV1beta3, + TemplateParametersV1beta3, TemplateEntityStepV1beta3, } from '@backstage/plugin-scaffolder-common'; import { @@ -81,7 +81,7 @@ import { scaffolderTemplateRules } from './rules'; export type ScaffolderPermissionRuleInput< TParams extends PermissionRuleParams = PermissionRuleParams, > = PermissionRule< - TemplateEntityStepV1beta3 | TemplateParameterV1beta3, + TemplateEntityStepV1beta3 | TemplateParametersV1beta3, {}, typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, TParams diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index db7ad993e9..d94f7dd4f4 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -17,14 +17,14 @@ import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { TemplateEntityStepV1beta3, - TemplateParameterV1beta3, + TemplateParametersV1beta3, } from '@backstage/plugin-scaffolder-common'; import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; import { z } from 'zod'; export const createScaffolderPermissionRule = makeCreatePermissionRule< - TemplateEntityStepV1beta3 | TemplateParameterV1beta3, + TemplateEntityStepV1beta3 | TemplateParametersV1beta3, {}, typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); diff --git a/plugins/scaffolder-common/api-report.md b/plugins/scaffolder-common/api-report.md index 61d6d51a78..7c40780b57 100644 --- a/plugins/scaffolder-common/api-report.md +++ b/plugins/scaffolder-common/api-report.md @@ -64,7 +64,7 @@ export interface TemplateEntityV1beta3 extends Entity { kind: 'Template'; spec: { type: string; - parameters?: TemplateParameterV1beta3 | TemplateParameterV1beta3[]; + parameters?: TemplateParametersV1beta3 | TemplateParametersV1beta3[]; steps: Array; output?: { [name: string]: string; @@ -86,7 +86,7 @@ export type TemplateInfo = { }; // @public -export interface TemplateParameterV1beta3 extends JsonObject { +export interface TemplateParametersV1beta3 extends JsonObject { // (undocumented) 'backstage:permissions'?: TemplatePermissionsV1beta3; } diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts index 4609a7802a..432292548e 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.test.ts @@ -17,7 +17,7 @@ import { entityKindSchemaValidator } from '@backstage/catalog-model'; import type { TemplateEntityV1beta3, - TemplateParameterV1beta3, + TemplateParametersV1beta3, } from './TemplateEntityV1beta3'; import schema from './Template.v1beta3.schema.json'; @@ -163,12 +163,12 @@ describe('templateEntityV1beta3Validator', () => { }); it('rejects parameters with wrong backstage:permissions', async () => { - (entity.spec.parameters as TemplateParameterV1beta3)[ + (entity.spec.parameters as TemplateParametersV1beta3)[ 'backstage:permissions' ]!.tags = true as unknown as []; expect(() => validator(entity)).toThrow(/must be array/); - (entity.spec.parameters as TemplateParameterV1beta3)[ + (entity.spec.parameters as TemplateParametersV1beta3)[ 'backstage:permissions' ] = true as {}; expect(() => validator(entity)).toThrow(/must be object/); diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index 5d002ddfe4..ebabb5cad4 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -50,7 +50,7 @@ export interface TemplateEntityV1beta3 extends Entity { * to collect user input and validate it against that schema. This can then be used in the `steps` part below to template * variables passed from the user into each action in the template. */ - parameters?: TemplateParameterV1beta3 | TemplateParameterV1beta3[]; + parameters?: TemplateParametersV1beta3 | TemplateParametersV1beta3[]; /** * A list of steps to be executed in sequence which are defined by the template. These steps are a list of the underlying * javascript action and some optional input parameters that may or may not have been collected from the end user. @@ -86,7 +86,7 @@ export interface TemplateEntityStepV1beta3 extends JsonObject { * * @public */ -export interface TemplateParameterV1beta3 extends JsonObject { +export interface TemplateParametersV1beta3 extends JsonObject { 'backstage:permissions'?: TemplatePermissionsV1beta3; } diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index 701cad1f71..fd34fe6f41 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -29,6 +29,6 @@ export { export type { TemplateEntityV1beta3, TemplateEntityStepV1beta3, - TemplateParameterV1beta3, + TemplateParametersV1beta3, TemplatePermissionsV1beta3, } from './TemplateEntityV1beta3'; From 75e111f87918eba056cac7fbb8ae1033b1bd4496 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 17 Mar 2023 00:04:45 +0100 Subject: [PATCH 51/55] scaffolder: rename customPermissionRules to rules Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/api-report.md | 24 +++++++++---------- .../scaffolder-backend/src/service/router.ts | 20 +++++++--------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 087b795c62..d116fe60ff 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -656,8 +656,6 @@ export interface RouterOptions { // (undocumented) config: Config; // (undocumented) - customPermissionRules?: ScaffolderPermissionRuleInput[]; - // (undocumented) database: PluginDatabaseManager; // (undocumented) identity?: IdentityApi; @@ -668,6 +666,8 @@ export interface RouterOptions { // (undocumented) reader: UrlReader; // (undocumented) + rules?: TemplatePermissionRuleInput[]; + // (undocumented) scheduler?: PluginTaskScheduler; // (undocumented) taskBroker?: TaskBroker; @@ -697,16 +697,6 @@ export class ScaffolderEntitiesProcessor implements CatalogProcessor { validateEntityKind(entity: Entity): Promise; } -// @public -export type ScaffolderPermissionRuleInput< - TParams extends PermissionRuleParams = PermissionRuleParams, -> = PermissionRule< - TemplateEntityStepV1beta3 | TemplateParametersV1beta3, - {}, - typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - TParams ->; - // @public export type SerializedTask = { id: string; @@ -931,4 +921,14 @@ export type TemplateFilter = (...args: JsonValue[]) => JsonValue | undefined; export type TemplateGlobal = | ((...args: JsonValue[]) => JsonValue | undefined) | JsonValue; + +// @public (undocumented) +export type TemplatePermissionRuleInput< + TParams extends PermissionRuleParams = PermissionRuleParams, +> = PermissionRule< + TemplateEntityStepV1beta3 | TemplateParametersV1beta3, + {}, + typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + TParams +>; ``` diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index f50c83ed3f..e8a39444a7 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -74,11 +74,10 @@ import { import { scaffolderTemplateRules } from './rules'; /** - * ScaffolderPermissionRuleInput * * @public */ -export type ScaffolderPermissionRuleInput< +export type TemplatePermissionRuleInput< TParams extends PermissionRuleParams = PermissionRuleParams, > = PermissionRule< TemplateEntityStepV1beta3 | TemplateParametersV1beta3, @@ -114,7 +113,7 @@ export interface RouterOptions { additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; permissionApi?: PermissionEvaluator; - customPermissionRules?: ScaffolderPermissionRuleInput[]; + rules?: TemplatePermissionRuleInput[]; identity?: IdentityApi; } @@ -210,7 +209,7 @@ export async function createRouter( additionalTemplateFilters, additionalTemplateGlobals, permissionApi, - customPermissionRules, + rules, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); @@ -286,21 +285,20 @@ export async function createRouter( additionalTemplateGlobals, }); - const permissionRules: ScaffolderPermissionRuleInput[] = Object.values( + const templateRules: TemplatePermissionRuleInput[] = Object.values( scaffolderTemplateRules, ); - if (customPermissionRules) { - permissionRules.push(...customPermissionRules); + + if (rules) { + templateRules.push(...rules); } - const isAuthorized = createConditionAuthorizer( - Object.values(permissionRules), - ); + const isAuthorized = createConditionAuthorizer(Object.values(templateRules)); const permissionIntegrationRouter = createPermissionIntegrationRouter({ resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, permissions: scaffolderPermissions, - rules: permissionRules, + rules: templateRules, }); router.use(permissionIntegrationRouter); From acc6672cf699510c8a332d30c9b0d8c50d203719 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 20 Mar 2023 11:41:03 +0100 Subject: [PATCH 52/55] scaffolder: add docs on the changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/olive-months-talk.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/olive-months-talk.md b/.changeset/olive-months-talk.md index e37832736e..b3a89d1f9e 100644 --- a/.changeset/olive-months-talk.md +++ b/.changeset/olive-months-talk.md @@ -5,3 +5,4 @@ Added the possibility to authorize parameters and steps of a template The scaffolder plugin is now integrated with the permission framework. +It is possible to toggle parameters or actions within templates by marking each section with specific `tags`, inside a `backstage:permissions` property under each parameter or action. Each parameter or action can then be permissioned by using a conditional decision containing the `scaffolderTemplateRules.hasTag` rule. From a4f3c20430bb0e8f57724c51ae05fb39b004a535 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 20 Mar 2023 14:02:20 +0100 Subject: [PATCH 53/55] scaffolder: rename rules to permissionRules Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/api-report.md | 4 ++-- plugins/scaffolder-backend/src/service/router.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index d116fe60ff..41207f05be 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -664,9 +664,9 @@ export interface RouterOptions { // (undocumented) permissionApi?: PermissionEvaluator; // (undocumented) - reader: UrlReader; + permissionRules?: TemplatePermissionRuleInput[]; // (undocumented) - rules?: TemplatePermissionRuleInput[]; + reader: UrlReader; // (undocumented) scheduler?: PluginTaskScheduler; // (undocumented) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index e8a39444a7..1f2d390f9f 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -113,7 +113,7 @@ export interface RouterOptions { additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; permissionApi?: PermissionEvaluator; - rules?: TemplatePermissionRuleInput[]; + permissionRules?: TemplatePermissionRuleInput[]; identity?: IdentityApi; } @@ -209,7 +209,7 @@ export async function createRouter( additionalTemplateFilters, additionalTemplateGlobals, permissionApi, - rules, + permissionRules, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); @@ -289,8 +289,8 @@ export async function createRouter( scaffolderTemplateRules, ); - if (rules) { - templateRules.push(...rules); + if (permissionRules) { + templateRules.push(...permissionRules); } const isAuthorized = createConditionAuthorizer(Object.values(templateRules)); From 71fd0966d1094553e288524b3ab2d506ecad927f Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 21 Mar 2023 11:03:58 +0100 Subject: [PATCH 54/55] missing changesets Signed-off-by: Vincenzo Scamporlino --- .changeset/funny-rivers-grin.md | 5 +++++ .changeset/odd-grapes-double.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/funny-rivers-grin.md create mode 100644 .changeset/odd-grapes-double.md diff --git a/.changeset/funny-rivers-grin.md b/.changeset/funny-rivers-grin.md new file mode 100644 index 0000000000..55463e40a7 --- /dev/null +++ b/.changeset/funny-rivers-grin.md @@ -0,0 +1,5 @@ +--- +'@backstage/create-app': patch +--- + +Add `permissionApi` as dependency of the scaffolder-backend plugin diff --git a/.changeset/odd-grapes-double.md b/.changeset/odd-grapes-double.md new file mode 100644 index 0000000000..7ef7486b60 --- /dev/null +++ b/.changeset/odd-grapes-double.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-node': patch +--- + +Added createConditionAuthorizer utility function, which takes some permission conditions and returns a function that returns a definitive authorization result given a decision and a resource. From e66093a14d63e141d6edee55e64b566f47997b99 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 21 Mar 2023 14:36:21 +0100 Subject: [PATCH 55/55] scaffolder-backend: refactor alpha exports Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/alpha.ts | 2 +- plugins/scaffolder-backend/src/service/index.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 plugins/scaffolder-backend/src/service/index.ts diff --git a/plugins/scaffolder-backend/src/alpha.ts b/plugins/scaffolder-backend/src/alpha.ts index 6d6be0c156..713f2569aa 100644 --- a/plugins/scaffolder-backend/src/alpha.ts +++ b/plugins/scaffolder-backend/src/alpha.ts @@ -15,6 +15,6 @@ */ export * from './modules'; -export * from './service/conditionExports'; +export * from './service'; export { scaffolderPlugin } from './ScaffolderPlugin'; export type { ScaffolderPluginOptions } from './ScaffolderPlugin'; diff --git a/plugins/scaffolder-backend/src/service/index.ts b/plugins/scaffolder-backend/src/service/index.ts new file mode 100644 index 0000000000..dc5508d47f --- /dev/null +++ b/plugins/scaffolder-backend/src/service/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 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. + */ + +export * from './conditionExports';