From e2571d17616fe5dcdbba2f7fded86109341e25ba Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Tue, 7 Feb 2023 13:29:33 +0000 Subject: [PATCH 01/31] POC Signed-off-by: Harry Hogg --- .../tasks/NunjucksWorkflowRunner.ts | 1 + .../scaffolder-backend/src/service/router.ts | 56 +++++++++-------- .../scaffolder-backend/src/service/rules.ts | 60 ++++++++++++++++++- plugins/scaffolder-common/src/permissions.ts | 20 +++++++ 4 files changed, 109 insertions(+), 28 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 9358484642..b31def1739 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -66,6 +66,7 @@ type TemplateContext = { entity?: UserEntity; ref?: string; }; + token?: string; }; const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta3 => { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 1f2d390f9f..0927e5f062 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -378,31 +378,7 @@ export async function createRouter( const baseUrl = getEntityBaseUrl(template); - const taskSpec: TaskSpec = { - apiVersion: template.apiVersion, - steps: template.spec.steps.map((step, index) => ({ - ...step, - id: step.id ?? `step-${index + 1}`, - name: step.name ?? step.action, - })), - output: template.spec.output ?? {}, - parameters: values, - user: { - entity: userEntity as UserEntity, - ref: userEntityRef, - }, - templateInfo: { - entityRef: stringifyEntityRef({ - kind, - namespace, - name: template.metadata?.name, - }), - baseUrl, - entity: { - metadata: template.metadata, - }, - }, - }; + const taskSpec = await authorizeTaskSpec(template); const result = await taskBroker.dispatch({ spec: taskSpec, @@ -651,5 +627,35 @@ export async function createRouter( return template; } + async function authorizeTaskSpec(template: TemplateEntityV1beta3): TaskSpec { + const taskSpec: TaskSpec = { + apiVersion: template.apiVersion, + steps: template.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: template.spec.output ?? {}, + parameters: values, + user: { + entity: userEntity as UserEntity, + ref: userEntityRef, + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind, + namespace, + name: template.metadata?.name, + }), + baseUrl, + entity: { + metadata: template.metadata, + }, + }, + }; + + return taskSpec; + } + return app; } diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index d94f7dd4f4..129d61e591 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -15,21 +15,75 @@ */ import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; +import { + RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + RESOURCE_TYPE_SCAFFOLDER_ACTION, +} from '@backstage/plugin-scaffolder-common/alpha'; + import { TemplateEntityStepV1beta3, TemplateParametersV1beta3, } from '@backstage/plugin-scaffolder-common'; -import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; import { z } from 'zod'; +import { JsonObject } from '@backstage/types'; -export const createScaffolderPermissionRule = makeCreatePermissionRule< +export const createTemplatePermissionRule = makeCreatePermissionRule< TemplateEntityStepV1beta3 | TemplateParametersV1beta3, {}, typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); -export const hasTag = createScaffolderPermissionRule({ +export const createActionPermissionRule = makeCreatePermissionRule< + { + actionId: string; + input: JsonObject; + template: TemplateEntityStepV1beta3 | TemplateParametersV1beta3; + }, + {}, + typeof RESOURCE_TYPE_SCAFFOLDER_ACTION +>(); + +export const hasActionId = createActionPermissionRule({ + name: 'HAS_ACTION_ID', + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + description: `Match actions with the given actionId`, + paramsSchema: z.object({ + actionId: z.string().describe('Name of the actionId to match on'), + }), + apply: (resource, { actionId }) => { + return resource.actionId === actionId; + }, + toQuery: () => ({}), +}); + +export const matchesInput = createActionPermissionRule({ + name: 'MATCHED_INPUT', + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + description: `Matches actionId and the input given`, + paramsSchema: z.object({ + actionId: z.string().describe('Name of the actionId to match on'), + + // Pass in a json schema to validate the input against + input: z.jsonSchema({}).describe('Input to match on'), + }), + apply: (resource, { actionId, input }) => { + if (resource.actionId !== actionId) { + return false; + } + + for (const [key, value] of Object.entries(input)) { + if (resource.input[key] !== value) { + return false; + } + } + + return true; + }, + toQuery: () => ({}), +}); + +export const hasTag = createTemplatePermissionRule({ name: 'HAS_TAG', resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, description: `Match parameters or steps with the given tag`, diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 858430eaf3..f06d7984f2 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -23,6 +23,25 @@ import { createPermission } from '@backstage/plugin-permission-common'; */ export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; +/** + * Permission resource type which corresponds to a scaffolder action. + * + * @alpha + */ +export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action'; + +/** + * This permission is used to authorize actions that involve executing + * an action from a template. + * + * @alpha + */ +export const actionExecutePermission = createPermission({ + name: 'scaffolder.action.execute', + attributes: {}, + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, +}); + /** * This permission is used to authorize actions that involve reading * one or more parameters from a template. @@ -64,6 +83,7 @@ export const templateStepReadPermission = createPermission({ * @alpha */ export const scaffolderPermissions = [ + actionExecutePermission, templateParameterReadPermission, templateStepReadPermission, ]; From ddb5d1c72b1bd4caaad8bfca07173981c135a11a Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 8 Feb 2023 22:47:33 +0100 Subject: [PATCH 02/31] scaffolder: add validation on action input Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/permission.ts | 22 +++++++ plugins/scaffolder-backend/package.json | 1 + .../src/service/conditionExports.ts | 20 +++++- .../scaffolder-backend/src/service/router.ts | 63 +++++++++++++++---- .../scaffolder-backend/src/service/rules.ts | 32 +++++----- template.yaml | 30 +++++++++ yarn.lock | 1 + 7 files changed, 138 insertions(+), 31 deletions(-) create mode 100644 template.yaml diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index 7192a1ddec..a87e673309 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -18,6 +18,7 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { createRouter } from '@backstage/plugin-permission-backend'; import { AuthorizeResult, + isPermission, PolicyDecision, } from '@backstage/plugin-permission-common'; import { @@ -28,8 +29,19 @@ import { DefaultPlaylistPermissionPolicy, isPlaylistPermission, } from '@backstage/plugin-playlist-backend'; +import { + createScaffolderActionConditionalDecision, + scaffolderActionConditions, +} from '@backstage/plugin-scaffolder-backend'; +import { actionExecutePermission } from '@backstage/plugin-scaffolder-common'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; +import { z } from 'zod'; +import zodToJsonSchema from 'zod-to-json-schema'; + +const inputSchema = zodToJsonSchema( + z.object({ message: z.enum(['hello']) }).strict(), +); class ExamplePermissionPolicy implements PermissionPolicy { private playlistPermissionPolicy = new DefaultPlaylistPermissionPolicy(); @@ -42,6 +54,16 @@ class ExamplePermissionPolicy implements PermissionPolicy { return this.playlistPermissionPolicy.handle(request, user); } + if (isPermission(request.permission, actionExecutePermission)) { + return createScaffolderActionConditionalDecision( + request.permission, + scaffolderActionConditions.matchesInput({ + action: 'debug:log', + schema: inputSchema, + }), + ); + } + return { result: AuthorizeResult.ALLOW, }; diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 97d4577d23..f238ab3c4a 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -68,6 +68,7 @@ "@octokit/webhooks": "^10.0.0", "@types/express": "^4.17.6", "@types/luxon": "^3.0.0", + "ajv": "^8.12.0", "azure-devops-node-api": "^11.0.1", "command-exists": "^1.2.9", "compression": "^1.7.4", diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 0c502c6075..79c03c1852 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -14,9 +14,12 @@ * limitations under the License. */ -import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; +import { + RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + RESOURCE_TYPE_SCAFFOLDER_ACTION, +} from '@backstage/plugin-scaffolder-common/alpha'; import { createConditionExports } from '@backstage/plugin-permission-node'; -import { scaffolderTemplateRules } from './rules'; +import { scaffolderTemplateRules, scaffolderActionRules } from './rules'; const { conditions, createConditionalDecision } = createConditionExports({ pluginId: 'scaffolder', @@ -24,6 +27,15 @@ const { conditions, createConditionalDecision } = createConditionExports({ rules: scaffolderTemplateRules, }); +const { + conditions: scaffolderActionConditions, + createConditionalDecision: createScaffolderActionConditionalDecision, +} = createConditionExports({ + pluginId: 'scaffolder', + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + rules: scaffolderActionRules, +}); + /** * These conditions are used when creating conditional decisions for scaffolder * templates that are returned by authorization policies. @@ -65,3 +77,7 @@ export const scaffolderConditions = conditions; * @alpha */ export const createScaffolderConditionalDecision = createConditionalDecision; +export { + createScaffolderActionConditionalDecision, + scaffolderActionConditions, +}; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0927e5f062..19207a7b75 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -40,6 +40,7 @@ import { scaffolderPermissions, templateParameterReadPermission, templateStepReadPermission, + actionExecutePermission, } from '@backstage/plugin-scaffolder-common/alpha'; import express from 'express'; import Router from 'express-promise-router'; @@ -71,7 +72,7 @@ import { createPermissionIntegrationRouter, PermissionRule, } from '@backstage/plugin-permission-node'; -import { scaffolderTemplateRules } from './rules'; +import { scaffolderTemplateRules, scaffolderActionRules } from './rules'; /** * @@ -86,6 +87,10 @@ export type TemplatePermissionRuleInput< TParams >; +const isActionAuthorized = createConditionAuthorizer( + Object.values(scaffolderActionRules), +); + /** * RouterOptions * @@ -376,9 +381,16 @@ export async function createRouter( } } - const baseUrl = getEntityBaseUrl(template); - - const taskSpec = await authorizeTaskSpec(template); + const taskSpec = await authorizeActions( + { + parameters: values, + template, + templateRef: { kind, name, namespace }, + user: userEntity as UserEntity, + userEntityRef, + }, + { token }, + ); const result = await taskBroker.dispatch({ spec: taskSpec, @@ -627,7 +639,24 @@ export async function createRouter( return template; } - async function authorizeTaskSpec(template: TemplateEntityV1beta3): TaskSpec { + async function authorizeActions( + { + parameters, + template, + user, + userEntityRef, + templateRef, + }: { + template: TemplateEntityV1beta3; + parameters: JsonObject; + user: UserEntity; + userEntityRef: string | undefined; + templateRef: CompoundEntityRef; + }, + { token }: { token: string | undefined }, + ): Promise { + const baseUrl = getEntityBaseUrl(template); + const taskSpec: TaskSpec = { apiVersion: template.apiVersion, steps: template.spec.steps.map((step, index) => ({ @@ -636,17 +665,13 @@ export async function createRouter( name: step.name ?? step.action, })), output: template.spec.output ?? {}, - parameters: values, + parameters, user: { - entity: userEntity as UserEntity, + entity: user, ref: userEntityRef, }, templateInfo: { - entityRef: stringifyEntityRef({ - kind, - namespace, - name: template.metadata?.name, - }), + entityRef: stringifyEntityRef(templateRef), baseUrl, entity: { metadata: template.metadata, @@ -654,6 +679,20 @@ export async function createRouter( }, }; + if (permissionApi) { + const [decision] = await permissionApi.authorizeConditional( + [{ permission: actionExecutePermission }], + { token }, + ); + + for (const step of taskSpec.steps) { + const { action, input } = step; + if (!isActionAuthorized(decision, { action, input })) { + throw new InputError(`Unauthorized action: ${action}, ${input}`); + } + } + } + return taskSpec; } diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 129d61e591..7113ec86e3 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import Ajv from 'ajv'; import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, @@ -34,11 +35,11 @@ export const createTemplatePermissionRule = makeCreatePermissionRule< typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); +const ajv = new Ajv({ allErrors: true }); export const createActionPermissionRule = makeCreatePermissionRule< { - actionId: string; - input: JsonObject; - template: TemplateEntityStepV1beta3 | TemplateParametersV1beta3; + action: string; + input: JsonObject | undefined; }, {}, typeof RESOURCE_TYPE_SCAFFOLDER_ACTION @@ -52,7 +53,7 @@ export const hasActionId = createActionPermissionRule({ actionId: z.string().describe('Name of the actionId to match on'), }), apply: (resource, { actionId }) => { - return resource.actionId === actionId; + return resource.action === actionId; }, toQuery: () => ({}), }); @@ -62,23 +63,19 @@ export const matchesInput = createActionPermissionRule({ resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, description: `Matches actionId and the input given`, paramsSchema: z.object({ - actionId: z.string().describe('Name of the actionId to match on'), - + action: z.string().describe('Name of the actionId to match on'), // Pass in a json schema to validate the input against - input: z.jsonSchema({}).describe('Input to match on'), + schema: z.record(z.any()), }), - apply: (resource, { actionId, input }) => { - if (resource.actionId !== actionId) { - return false; + apply: (resource, { action, schema }) => { + if (resource.action !== action) { + return true; + } + if (!schema) { + return true; } - for (const [key, value] of Object.entries(input)) { - if (resource.input[key] !== value) { - return false; - } - } - - return true; + return ajv.validate(schema, resource.input); }, toQuery: () => ({}), }); @@ -97,3 +94,4 @@ export const hasTag = createTemplatePermissionRule({ }); export const scaffolderTemplateRules = { hasTag }; +export const scaffolderActionRules = { matchesInput }; diff --git a/template.yaml b/template.yaml new file mode 100644 index 0000000000..7773a04315 --- /dev/null +++ b/template.yaml @@ -0,0 +1,30 @@ +apiVersion: scaffolder.backstage.io/v1beta3 +kind: Template +metadata: + name: my-custom-template + parameters: + - title: Provide some simple information + metadata: + tags: + - bang + properties: + description: + title: Description + type: string + metadata: + tags: + - bar + steps: + - id: step-one + name: First log + action: debug:log + input: + message: hello + metadata: + tags: + - foo + - id: step-two + name: Second log + action: debug:log + input: + message: world diff --git a/yarn.lock b/yarn.lock index 40d006519f..16b336dcb4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8020,6 +8020,7 @@ __metadata: "@types/nunjucks": ^3.1.4 "@types/supertest": ^2.0.8 "@types/zen-observable": ^0.8.0 + ajv: ^8.12.0 azure-devops-node-api: ^11.0.1 command-exists: ^1.2.9 compression: ^1.7.4 From 85ed7d94efb031b9c00c49c1feeb6052fe8617b9 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Fri, 10 Feb 2023 09:47:54 +0000 Subject: [PATCH 03/31] Fixed typings for JSON schema as paramsSchema input Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- packages/backend/package.json | 5 +++- packages/backend/src/plugins/permission.ts | 2 +- plugins/scaffolder-backend/package.json | 4 ++-- .../scaffolder-backend/src/service/rules.ts | 23 ++++++++++++++----- yarn.lock | 12 ++++++++++ 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index fd8ec82d91..57bb988f64 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:^", @@ -83,7 +84,9 @@ "pg": "^8.3.0", "pg-connection-string": "^2.3.0", "prom-client": "^14.0.1", - "winston": "^3.2.1" + "winston": "^3.2.1", + "zod": "~3.18.0", + "zod-to-json-schema": "~3.18.0" }, "devDependencies": { "@backstage/cli": "workspace:^", diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index a87e673309..c5f92c17d0 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -35,9 +35,9 @@ import { } from '@backstage/plugin-scaffolder-backend'; import { actionExecutePermission } from '@backstage/plugin-scaffolder-common'; import { Router } from 'express'; -import { PluginEnvironment } from '../types'; import { z } from 'zod'; import zodToJsonSchema from 'zod-to-json-schema'; +import { PluginEnvironment } from '../types'; const inputSchema = zodToJsonSchema( z.object({ message: z.enum(['hello']) }).strict(), diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index f238ab3c4a..9b28ac8199 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -66,8 +66,6 @@ "@gitbeaker/core": "^35.6.0", "@gitbeaker/node": "^35.1.0", "@octokit/webhooks": "^10.0.0", - "@types/express": "^4.17.6", - "@types/luxon": "^3.0.0", "ajv": "^8.12.0", "azure-devops-node-api": "^11.0.1", "command-exists": "^1.2.9", @@ -103,8 +101,10 @@ "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@types/command-exists": "^1.2.0", + "@types/express": "^4.17.6", "@types/fs-extra": "^9.0.1", "@types/git-url-parse": "^9.0.0", + "@types/luxon": "^3.0.0", "@types/mock-fs": "^4.13.0", "@types/nunjucks": "^3.1.4", "@types/supertest": "^2.0.8", diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 7113ec86e3..7f71693478 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -59,23 +59,34 @@ export const hasActionId = createActionPermissionRule({ }); export const matchesInput = createActionPermissionRule({ - name: 'MATCHED_INPUT', + name: 'MATCHES_INPUT', resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, description: `Matches actionId and the input given`, paramsSchema: z.object({ action: z.string().describe('Name of the actionId to match on'), - // Pass in a json schema to validate the input against - schema: z.record(z.any()), + input: z + .any() + .optional() + .describe('JSON schema to validate input against') + .superRefine((value, ctx) => { + if (value && ajv.validateSchema(value)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Invalid JSON schema', + }); + } + }), }), - apply: (resource, { action, schema }) => { + apply: (resource, { action, input }) => { if (resource.action !== action) { return true; } - if (!schema) { + + if (!input) { return true; } - return ajv.validate(schema, resource.input); + return ajv.validate(input, resource.input); }, toQuery: () => ({}), }); diff --git a/yarn.lock b/yarn.lock index 16b336dcb4..1fe70bea95 100644 --- a/yarn.lock +++ b/yarn.lock @@ -23494,6 +23494,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:^" @@ -23522,6 +23523,8 @@ __metadata: pg-connection-string: ^2.3.0 prom-client: ^14.0.1 winston: ^3.2.1 + zod: ~3.18.0 + zod-to-json-schema: ~3.18.0 languageName: unknown linkType: soft @@ -40643,6 +40646,15 @@ __metadata: languageName: node linkType: hard +"zod-to-json-schema@npm:~3.18.0": + version: 3.18.2 + resolution: "zod-to-json-schema@npm:3.18.2" + peerDependencies: + zod: ^3.18.0 + checksum: 10e84864a3ccc2e04500b9bc81369ed7c5b3ec068ea582a13369175ed40e785091a63ae7fa86ebb4ddf1bccd55c3dab32f4d3070bbcc3296e68305ff787c9df3 + languageName: node + linkType: hard + "zod@npm:^3.21.4": version: 3.21.4 resolution: "zod@npm:3.21.4" From faeb730c9ffdc6d4205cee53059ca0a40be8cd86 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 17 Feb 2023 10:18:45 +0100 Subject: [PATCH 04/31] scaffolder: perform actions authorization in the runners Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/permission.ts | 9 +++-- .../src/scaffolder/dryrun/createDryRunner.ts | 2 ++ .../tasks/NunjucksWorkflowRunner.ts | 27 +++++++++++++++ .../src/scaffolder/tasks/TaskWorker.ts | 7 +++- .../scaffolder-backend/src/service/router.ts | 31 ++++++++++++----- .../scaffolder-backend/src/service/rules.ts | 33 ++++++++++++++++++- template.yaml | 28 +++++++++------- 7 files changed, 112 insertions(+), 25 deletions(-) diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index c5f92c17d0..e59a53549b 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -57,10 +57,15 @@ class ExamplePermissionPolicy implements PermissionPolicy { if (isPermission(request.permission, actionExecutePermission)) { return createScaffolderActionConditionalDecision( request.permission, - scaffolderActionConditions.matchesInput({ + scaffolderActionConditions.hasInputProperty({ action: 'debug:log', - schema: inputSchema, + key: 'message', + value: 'Test', }), + // scaffolderActionConditions.matchesInput({ + // action: 'debug:log', + // schema: inputSchema, + // }), ); } diff --git a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts index d9302f4d74..d28d16f4cf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts @@ -35,6 +35,7 @@ import { createTemplateAction, TaskSecrets, } from '@backstage/plugin-scaffolder-node'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; interface DryRunInput { spec: TaskSpec; @@ -53,6 +54,7 @@ export type TemplateTesterCreateOptions = { logger: Logger; integrations: ScmIntegrations; actionRegistry: TemplateActionRegistry; + permissionApi: PermissionEvaluator; workingDirectory: string; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index b31def1739..203c3cd32d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -42,10 +42,15 @@ import { TaskSpecV1beta3, TaskStep, } from '@backstage/plugin-scaffolder-common'; + import { TemplateAction } from '@backstage/plugin-scaffolder-node'; +import { createConditionAuthorizer } from '@backstage/plugin-permission-node'; import { UserEntity } from '@backstage/catalog-model'; import { createCounterMetric, createHistogramMetric } from '../../util/metrics'; import { createDefaultFilters } from '../../lib/templating/filters'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { scaffolderActionRules } from '../../service/rules'; +import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha'; type NunjucksWorkflowRunnerOptions = { workingDirectory: string; @@ -54,6 +59,7 @@ type NunjucksWorkflowRunnerOptions = { logger: winston.Logger; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; + permissionApi: PermissionEvaluator; }; type TemplateContext = { @@ -103,6 +109,10 @@ const createStepLogger = ({ return { taskLogger, streamLogger }; }; +const isActionAuthorized = createConditionAuthorizer( + Object.values(scaffolderActionRules), +); + export class NunjucksWorkflowRunner implements WorkflowRunner { private readonly defaultTemplateFilters: Record; constructor(private readonly options: NunjucksWorkflowRunnerOptions) { @@ -282,6 +292,23 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { } } + const [decision] = await this.options.permissionApi.authorizeConditional( + [{ permission: actionExecutePermission }], + { token: task.secrets?.backstageToken }, + ); + + if (!isActionAuthorized(decision, { action: action.id, input })) { + throw new InputError( + `Unauthorized action: ${ + action.id + }. The input is not allowed. Input: ${JSON.stringify( + input, + null, + 2, + )}`, + ); + } + const tmpDirs = new Array(); const stepOutput: { [outputName: string]: JsonValue } = {}; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 5723e7136f..5c7a35303c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -22,7 +22,7 @@ import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; import { assertError } from '@backstage/errors'; import { TemplateFilter, TemplateGlobal } from '../../lib'; - +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; /** * TaskWorkerOptions * @@ -30,6 +30,7 @@ import { TemplateFilter, TemplateGlobal } from '../../lib'; */ export type TaskWorkerOptions = { taskBroker: TaskBroker; + permissionApi: PermissionEvaluator; runners: { workflowRunner: WorkflowRunner; }; @@ -43,6 +44,7 @@ export type TaskWorkerOptions = { */ export type CreateWorkerOptions = { taskBroker: TaskBroker; + permissionApi: PermissionEvaluator; actionRegistry: TemplateActionRegistry; integrations: ScmIntegrations; workingDirectory: string; @@ -86,6 +88,7 @@ export class TaskWorker { additionalTemplateFilters, concurrentTasksLimit = 10, // from 1 to Infinity additionalTemplateGlobals, + permissionApi, } = options; const workflowRunner = new NunjucksWorkflowRunner({ @@ -95,12 +98,14 @@ export class TaskWorker { workingDirectory, additionalTemplateFilters, additionalTemplateGlobals, + permissionApi, }); return new TaskWorker({ taskBroker: taskBroker, runners: { workflowRunner }, concurrentTasksLimit, + permissionApi, }); } diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 19207a7b75..41d7a67c89 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -263,6 +263,7 @@ export async function createRouter( additionalTemplateFilters, additionalTemplateGlobals, concurrentTasksLimit, + permissionApi, }); workers.push(worker); } @@ -288,6 +289,7 @@ export async function createRouter( workingDirectory, additionalTemplateFilters, additionalTemplateGlobals, + permissionApi, }); const templateRules: TemplatePermissionRuleInput[] = Object.values( @@ -381,16 +383,27 @@ export async function createRouter( } } - const taskSpec = await authorizeActions( - { - parameters: values, - template, - templateRef: { kind, name, namespace }, - user: userEntity as UserEntity, - userEntityRef, + const taskSpec: TaskSpec = { + apiVersion: template.apiVersion, + steps: template.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: template.spec.output ?? {}, + parameters: values, + user: { + entity: userEntity as UserEntity, + ref: userEntityRef, }, - { token }, - ); + templateInfo: { + entityRef: stringifyEntityRef({ kind, name, namespace }), + baseUrl: getEntityBaseUrl(template), + entity: { + metadata: template.metadata, + }, + }, + }; const result = await taskBroker.dispatch({ spec: taskSpec, diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 7f71693478..379f95db10 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -28,6 +28,7 @@ import { import { z } from 'zod'; import { JsonObject } from '@backstage/types'; +import { get } from 'lodash'; export const createTemplatePermissionRule = makeCreatePermissionRule< TemplateEntityStepV1beta3 | TemplateParametersV1beta3, @@ -91,6 +92,36 @@ export const matchesInput = createActionPermissionRule({ toQuery: () => ({}), }); +export const hasInputProperty = createActionPermissionRule({ + name: 'HAS_INPUT', + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + description: `Matches the key and value of the input of an action`, + paramsSchema: z.object({ + action: z.string().describe('Name of the actionId to match on'), + key: z.string().describe('Name of the property to match on'), + value: z.string().describe('Value of the property to match on').optional(), + }), + apply: (resource, { action, key, value }) => { + if (resource.action !== action) { + return true; + } + + const foundValue = get(resource.input, key); + + if (Array.isArray(foundValue)) { + if (value !== undefined) { + return foundValue.includes(value); + } + return foundValue.length > 0; + } + if (value !== undefined) { + return value === foundValue; + } + return !!foundValue; + }, + toQuery: () => ({}), +}); + export const hasTag = createTemplatePermissionRule({ name: 'HAS_TAG', resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, @@ -105,4 +136,4 @@ export const hasTag = createTemplatePermissionRule({ }); export const scaffolderTemplateRules = { hasTag }; -export const scaffolderActionRules = { matchesInput }; +export const scaffolderActionRules = { matchesInput, hasInputProperty }; diff --git a/template.yaml b/template.yaml index 7773a04315..fef90dbcd8 100644 --- a/template.yaml +++ b/template.yaml @@ -2,29 +2,33 @@ apiVersion: scaffolder.backstage.io/v1beta3 kind: Template metadata: name: my-custom-template +spec: + type: service parameters: - - title: Provide some simple information - metadata: - tags: - - bang + - title: Basic information properties: description: title: Description type: string - metadata: - tags: - - bar + - title: Extra information + backstage:accessControl: + tags: + - foo + properties: + name: + title: Your name + type: string steps: - id: step-one name: First log action: debug:log input: - message: hello - metadata: - tags: - - foo + message: Test - id: step-two name: Second log action: debug:log input: - message: world + message: Hello ${{ parameters.name }} + backstage:accessControl: + tags: + - foo From ec13659446e192fd35961beb9d49b969649c6774 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 24 Mar 2023 12:02:28 +0100 Subject: [PATCH 05/31] scaffolder-backend: remove matchesinput rule Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/package.json | 1 - .../scaffolder-backend/src/service/rules.ts | 63 +++++-------------- yarn.lock | 1 - 3 files changed, 14 insertions(+), 51 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 9b28ac8199..dbcf0f624a 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -66,7 +66,6 @@ "@gitbeaker/core": "^35.6.0", "@gitbeaker/node": "^35.1.0", "@octokit/webhooks": "^10.0.0", - "ajv": "^8.12.0", "azure-devops-node-api": "^11.0.1", "command-exists": "^1.2.9", "compression": "^1.7.4", diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 379f95db10..30aebd1583 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import Ajv from 'ajv'; import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, @@ -36,7 +35,19 @@ export const createTemplatePermissionRule = makeCreatePermissionRule< typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE >(); -const ajv = new Ajv({ allErrors: true }); +export const hasTag = createTemplatePermissionRule({ + name: 'HAS_TAG', + resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + description: `Match parameters or steps with the given tag`, + paramsSchema: z.object({ + tag: z.string().describe('Name of the tag to match on'), + }), + apply: (resource, { tag }) => { + return resource['backstage:permissions']?.tags?.includes(tag) ?? false; + }, + toQuery: () => ({}), +}); + export const createActionPermissionRule = makeCreatePermissionRule< { action: string; @@ -59,39 +70,6 @@ export const hasActionId = createActionPermissionRule({ toQuery: () => ({}), }); -export const matchesInput = createActionPermissionRule({ - name: 'MATCHES_INPUT', - resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, - description: `Matches actionId and the input given`, - paramsSchema: z.object({ - action: z.string().describe('Name of the actionId to match on'), - input: z - .any() - .optional() - .describe('JSON schema to validate input against') - .superRefine((value, ctx) => { - if (value && ajv.validateSchema(value)) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: 'Invalid JSON schema', - }); - } - }), - }), - apply: (resource, { action, input }) => { - if (resource.action !== action) { - return true; - } - - if (!input) { - return true; - } - - return ajv.validate(input, resource.input); - }, - toQuery: () => ({}), -}); - export const hasInputProperty = createActionPermissionRule({ name: 'HAS_INPUT', resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, @@ -122,18 +100,5 @@ export const hasInputProperty = createActionPermissionRule({ toQuery: () => ({}), }); -export const hasTag = createTemplatePermissionRule({ - name: 'HAS_TAG', - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - description: `Match parameters or steps with the given tag`, - paramsSchema: z.object({ - tag: z.string().describe('Name of the tag to match on'), - }), - apply: (resource, { tag }) => { - return resource['backstage:permissions']?.tags?.includes(tag) ?? false; - }, - toQuery: () => ({}), -}); - export const scaffolderTemplateRules = { hasTag }; -export const scaffolderActionRules = { matchesInput, hasInputProperty }; +export const scaffolderActionRules = { hasActionId, hasInputProperty }; diff --git a/yarn.lock b/yarn.lock index 1fe70bea95..b566ae3bdd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8020,7 +8020,6 @@ __metadata: "@types/nunjucks": ^3.1.4 "@types/supertest": ^2.0.8 "@types/zen-observable": ^0.8.0 - ajv: ^8.12.0 azure-devops-node-api: ^11.0.1 command-exists: ^1.2.9 compression: ^1.7.4 From 20a66731829b0667b3fe2fa2b466c4de3eacacf2 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 3 Apr 2023 18:05:31 +0200 Subject: [PATCH 06/31] scaffolder: remove dependency Signed-off-by: Vincenzo Scamporlino --- packages/backend/package.json | 3 +-- yarn.lock | 10 ---------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index 57bb988f64..9fc07e6e4f 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -85,8 +85,7 @@ "pg-connection-string": "^2.3.0", "prom-client": "^14.0.1", "winston": "^3.2.1", - "zod": "~3.18.0", - "zod-to-json-schema": "~3.18.0" + "zod": "~3.18.0" }, "devDependencies": { "@backstage/cli": "workspace:^", diff --git a/yarn.lock b/yarn.lock index b566ae3bdd..093320debe 100644 --- a/yarn.lock +++ b/yarn.lock @@ -23523,7 +23523,6 @@ __metadata: prom-client: ^14.0.1 winston: ^3.2.1 zod: ~3.18.0 - zod-to-json-schema: ~3.18.0 languageName: unknown linkType: soft @@ -40645,15 +40644,6 @@ __metadata: languageName: node linkType: hard -"zod-to-json-schema@npm:~3.18.0": - version: 3.18.2 - resolution: "zod-to-json-schema@npm:3.18.2" - peerDependencies: - zod: ^3.18.0 - checksum: 10e84864a3ccc2e04500b9bc81369ed7c5b3ec068ea582a13369175ed40e785091a63ae7fa86ebb4ddf1bccd55c3dab32f4d3070bbcc3296e68305ff787c9df3 - languageName: node - linkType: hard - "zod@npm:^3.21.4": version: 3.21.4 resolution: "zod@npm:3.21.4" From d03426c5ad6796b0aa3a59d6f6b4b2bff48c7db8 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 3 Apr 2023 18:08:12 +0200 Subject: [PATCH 07/31] remove actionId parameter Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/src/service/rules.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 30aebd1583..6611fcefbf 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -75,15 +75,10 @@ export const hasInputProperty = createActionPermissionRule({ resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, description: `Matches the key and value of the input of an action`, paramsSchema: z.object({ - action: z.string().describe('Name of the actionId to match on'), key: z.string().describe('Name of the property to match on'), value: z.string().describe('Value of the property to match on').optional(), }), - apply: (resource, { action, key, value }) => { - if (resource.action !== action) { - return true; - } - + apply: (resource, { key, value }) => { const foundValue = get(resource.input, key); if (Array.isArray(foundValue)) { From d059aeb0395ea9338bd53f6d904a7842f74d1765 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 3 Apr 2023 18:09:15 +0200 Subject: [PATCH 08/31] scaffolder: clean up permission policy Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/permission.ts | 34 +++++++++------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index e59a53549b..f0b977743c 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -32,17 +32,11 @@ import { import { createScaffolderActionConditionalDecision, scaffolderActionConditions, -} from '@backstage/plugin-scaffolder-backend'; -import { actionExecutePermission } from '@backstage/plugin-scaffolder-common'; +} from '@backstage/plugin-scaffolder-backend/alpha'; +import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha'; import { Router } from 'express'; -import { z } from 'zod'; -import zodToJsonSchema from 'zod-to-json-schema'; import { PluginEnvironment } from '../types'; -const inputSchema = zodToJsonSchema( - z.object({ message: z.enum(['hello']) }).strict(), -); - class ExamplePermissionPolicy implements PermissionPolicy { private playlistPermissionPolicy = new DefaultPlaylistPermissionPolicy(); @@ -55,18 +49,18 @@ class ExamplePermissionPolicy implements PermissionPolicy { } if (isPermission(request.permission, actionExecutePermission)) { - return createScaffolderActionConditionalDecision( - request.permission, - scaffolderActionConditions.hasInputProperty({ - action: 'debug:log', - key: 'message', - value: 'Test', - }), - // scaffolderActionConditions.matchesInput({ - // action: 'debug:log', - // schema: inputSchema, - // }), - ); + return createScaffolderActionConditionalDecision(request.permission, { + allOf: [ + scaffolderActionConditions.hasInputProperty({ + key: 'message', + value: 'Test', + }), + scaffolderActionConditions.hasInputProperty({ + key: 'message', + value: 'Hello ddd', + }), + ], + }); } return { From 6ffbf079fb935122186526b0e01280ddca4b6c48 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 3 Apr 2023 18:09:52 +0200 Subject: [PATCH 09/31] scaffolder: refactor condition exports Signed-off-by: Vincenzo Scamporlino --- .../src/service/conditionExports.ts | 90 ++++++++++--------- 1 file changed, 48 insertions(+), 42 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 79c03c1852..8175ec9a00 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -21,7 +21,10 @@ import { import { createConditionExports } from '@backstage/plugin-permission-node'; import { scaffolderTemplateRules, scaffolderActionRules } from './rules'; -const { conditions, createConditionalDecision } = createConditionExports({ +const { + conditions: scaffolderTemplateConditions, + createConditionalDecision: createScaffolderTemplateConditionalDecision, +} = createConditionExports({ pluginId: 'scaffolder', resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, rules: scaffolderTemplateRules, @@ -36,48 +39,51 @@ const { rules: scaffolderActionRules, }); -/** - * 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; export { createScaffolderActionConditionalDecision, scaffolderActionConditions, }; + +export { + /** + * `createScaffolderTemplateConditionalDecision` 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 + */ + createScaffolderTemplateConditionalDecision, + + /** + * These conditions are used when creating conditional decisions for scaffolder + * templates that are returned by authorization policies. + * + * @alpha + */ + scaffolderTemplateConditions, +}; From 942f06b9c9dcb1b8f63fa392a92ad461a053e524 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Mon, 3 Apr 2023 18:52:48 +0200 Subject: [PATCH 10/31] scaffolder: make permissionApi optional Signed-off-by: Vincenzo Scamporlino --- .../src/scaffolder/dryrun/createDryRunner.ts | 2 +- .../scaffolder/tasks/NunjucksWorkflowRunner.ts | 16 ++++++++++------ .../src/scaffolder/tasks/TaskWorker.ts | 4 ++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts index d28d16f4cf..03db2147a7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts @@ -54,10 +54,10 @@ export type TemplateTesterCreateOptions = { logger: Logger; integrations: ScmIntegrations; actionRegistry: TemplateActionRegistry; - permissionApi: PermissionEvaluator; workingDirectory: string; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; + permissionApi?: PermissionEvaluator; }; /** diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 203c3cd32d..7be7b95c78 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -48,7 +48,10 @@ import { createConditionAuthorizer } from '@backstage/plugin-permission-node'; import { UserEntity } from '@backstage/catalog-model'; import { createCounterMetric, createHistogramMetric } from '../../util/metrics'; import { createDefaultFilters } from '../../lib/templating/filters'; -import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { + AuthorizeResult, + PermissionEvaluator, +} from '@backstage/plugin-permission-common'; import { scaffolderActionRules } from '../../service/rules'; import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha'; @@ -59,7 +62,7 @@ type NunjucksWorkflowRunnerOptions = { logger: winston.Logger; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; - permissionApi: PermissionEvaluator; + permissionApi?: PermissionEvaluator; }; type TemplateContext = { @@ -292,10 +295,11 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { } } - const [decision] = await this.options.permissionApi.authorizeConditional( - [{ permission: actionExecutePermission }], - { token: task.secrets?.backstageToken }, - ); + const [decision] = + (await this.options.permissionApi?.authorizeConditional( + [{ permission: actionExecutePermission }], + { token: task.secrets?.backstageToken }, + )) || [{ result: AuthorizeResult.ALLOW }]; if (!isActionAuthorized(decision, { action: action.id, input })) { throw new InputError( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 5c7a35303c..960c89511a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -30,11 +30,11 @@ import { PermissionEvaluator } from '@backstage/plugin-permission-common'; */ export type TaskWorkerOptions = { taskBroker: TaskBroker; - permissionApi: PermissionEvaluator; runners: { workflowRunner: WorkflowRunner; }; concurrentTasksLimit: number; + permissionApi?: PermissionEvaluator; }; /** @@ -44,7 +44,6 @@ export type TaskWorkerOptions = { */ export type CreateWorkerOptions = { taskBroker: TaskBroker; - permissionApi: PermissionEvaluator; actionRegistry: TemplateActionRegistry; integrations: ScmIntegrations; workingDirectory: string; @@ -64,6 +63,7 @@ export type CreateWorkerOptions = { */ concurrentTasksLimit?: number; additionalTemplateGlobals?: Record; + permissionApi?: PermissionEvaluator; }; /** From 3bf5c639166639f729db5e114977046c8aa4f9af Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 11:53:17 +0200 Subject: [PATCH 11/31] scaffolder: remove authorizeAction from router Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-backend/src/service/router.ts | 68 ++----------------- 1 file changed, 4 insertions(+), 64 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 41d7a67c89..d4d55bcb51 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -40,7 +40,6 @@ import { scaffolderPermissions, templateParameterReadPermission, templateStepReadPermission, - actionExecutePermission, } from '@backstage/plugin-scaffolder-common/alpha'; import express from 'express'; import Router from 'express-promise-router'; @@ -72,7 +71,7 @@ import { createPermissionIntegrationRouter, PermissionRule, } from '@backstage/plugin-permission-node'; -import { scaffolderTemplateRules, scaffolderActionRules } from './rules'; +import { scaffolderTemplateRules } from './rules'; /** * @@ -87,10 +86,6 @@ export type TemplatePermissionRuleInput< TParams >; -const isActionAuthorized = createConditionAuthorizer( - Object.values(scaffolderActionRules), -); - /** * RouterOptions * @@ -383,6 +378,8 @@ export async function createRouter( } } + const baseUrl = getEntityBaseUrl(template); + const taskSpec: TaskSpec = { apiVersion: template.apiVersion, steps: template.spec.steps.map((step, index) => ({ @@ -398,7 +395,7 @@ export async function createRouter( }, templateInfo: { entityRef: stringifyEntityRef({ kind, name, namespace }), - baseUrl: getEntityBaseUrl(template), + baseUrl, entity: { metadata: template.metadata, }, @@ -652,62 +649,5 @@ export async function createRouter( return template; } - async function authorizeActions( - { - parameters, - template, - user, - userEntityRef, - templateRef, - }: { - template: TemplateEntityV1beta3; - parameters: JsonObject; - user: UserEntity; - userEntityRef: string | undefined; - templateRef: CompoundEntityRef; - }, - { token }: { token: string | undefined }, - ): Promise { - const baseUrl = getEntityBaseUrl(template); - - const taskSpec: TaskSpec = { - apiVersion: template.apiVersion, - steps: template.spec.steps.map((step, index) => ({ - ...step, - id: step.id ?? `step-${index + 1}`, - name: step.name ?? step.action, - })), - output: template.spec.output ?? {}, - parameters, - user: { - entity: user, - ref: userEntityRef, - }, - templateInfo: { - entityRef: stringifyEntityRef(templateRef), - baseUrl, - entity: { - metadata: template.metadata, - }, - }, - }; - - if (permissionApi) { - const [decision] = await permissionApi.authorizeConditional( - [{ permission: actionExecutePermission }], - { token }, - ); - - for (const step of taskSpec.steps) { - const { action, input } = step; - if (!isActionAuthorized(decision, { action, input })) { - throw new InputError(`Unauthorized action: ${action}, ${input}`); - } - } - } - - return taskSpec; - } - return app; } From 64712633ebd487662a1b09dd1e9138a8d2cafdde Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 11:54:51 +0200 Subject: [PATCH 12/31] scaffolder: refactor action rules Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/permission.ts | 4 +- .../scaffolder-backend/src/service/rules.ts | 47 ++++++++++++++++--- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index f0b977743c..a14a113b36 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -51,11 +51,11 @@ class ExamplePermissionPolicy implements PermissionPolicy { if (isPermission(request.permission, actionExecutePermission)) { return createScaffolderActionConditionalDecision(request.permission, { allOf: [ - scaffolderActionConditions.hasInputProperty({ + scaffolderActionConditions.hasStringProperty({ key: 'message', value: 'Test', }), - scaffolderActionConditions.hasInputProperty({ + scaffolderActionConditions.hasStringProperty({ key: 'message', value: 'Hello ddd', }), diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 6611fcefbf..d03f8daa2e 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -70,13 +70,15 @@ export const hasActionId = createActionPermissionRule({ toQuery: () => ({}), }); -export const hasInputProperty = createActionPermissionRule({ - name: 'HAS_INPUT', +export const hasNumberProperty = createActionPermissionRule({ + name: `HAS_NUMBER_PROPERTY`, + description: `Allow actions with the specified property`, resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, - description: `Matches the key and value of the input of an action`, paramsSchema: z.object({ - key: z.string().describe('Name of the property to match on'), - value: z.string().describe('Value of the property to match on').optional(), + key: z + .string() + .describe(`Property within the action parameters to match on`), + value: z.number().describe(`Value of the given property to match on`), }), apply: (resource, { key, value }) => { const foundValue = get(resource.input, key); @@ -87,7 +89,34 @@ export const hasInputProperty = createActionPermissionRule({ } return foundValue.length > 0; } - if (value !== undefined) { + if (value !== undefined && z.number().safeParse(value).success) { + return value === foundValue; + } + return !!foundValue; + }, + toQuery: () => ({}), +}); + +export const hasStringProperty = createActionPermissionRule({ + name: `HAS_STRING_PROPERTY`, + description: `Allow actions with the specified property`, + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + paramsSchema: z.object({ + key: z + .string() + .describe(`Property within the action parameters to match on`), + value: z.string().describe(`Value of the given property to match on`), + }), + apply: (resource, { key, value }) => { + const foundValue = get(resource.input, key); + + if (Array.isArray(foundValue)) { + if (value !== undefined) { + return foundValue.includes(value); + } + return foundValue.length > 0; + } + if (value !== undefined && z.string().safeParse(value).success) { return value === foundValue; } return !!foundValue; @@ -96,4 +125,8 @@ export const hasInputProperty = createActionPermissionRule({ }); export const scaffolderTemplateRules = { hasTag }; -export const scaffolderActionRules = { hasActionId, hasInputProperty }; +export const scaffolderActionRules = { + hasActionId, + hasNumberProperty, + hasStringProperty, +}; From 507aa63c343e29e2d1d1c07b8a38738a614d8309 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 11:56:22 +0200 Subject: [PATCH 13/31] scaffolder: authorize actions only once Signed-off-by: Vincenzo Scamporlino --- .../tasks/NunjucksWorkflowRunner.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 7be7b95c78..e93085b4e2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -51,6 +51,7 @@ import { createDefaultFilters } from '../../lib/templating/filters'; import { AuthorizeResult, PermissionEvaluator, + PolicyDecision, } from '@backstage/plugin-permission-common'; import { scaffolderActionRules } from '../../service/rules'; import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha'; @@ -212,6 +213,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { renderTemplate: (template: string, values: unknown) => string, taskTrack: TaskTrackType, workspacePath: string, + decision: PolicyDecision, ) { const stepTrack = await this.tracker.stepStart(task, step); @@ -295,17 +297,11 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { } } - const [decision] = - (await this.options.permissionApi?.authorizeConditional( - [{ permission: actionExecutePermission }], - { token: task.secrets?.backstageToken }, - )) || [{ result: AuthorizeResult.ALLOW }]; - if (!isActionAuthorized(decision, { action: action.id, input })) { throw new InputError( `Unauthorized action: ${ action.id - }. The input is not allowed. Input: ${JSON.stringify( + }. The action is not allowed. Input: ${JSON.stringify( input, null, 2, @@ -387,6 +383,14 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { user: task.spec.user, }; + const [decision]: PolicyDecision[] = + this.options.permissionApi && task.spec.steps.length + ? await this.options.permissionApi.authorizeConditional( + [{ permission: actionExecutePermission }], + { token: task.secrets?.backstageToken }, + ) + : [{ result: AuthorizeResult.ALLOW }]; + for (const step of task.spec.steps) { await this.executeStep( task, @@ -395,6 +399,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { renderTemplate, taskTrack, workspacePath, + decision, ); } From 8d271343e7e758c916ab08e68d5749e2968b4da9 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 11:56:57 +0200 Subject: [PATCH 14/31] scaffolder: add tests for permission actions Signed-off-by: Vincenzo Scamporlino --- .../tasks/NunjucksWorkflowRunner.test.ts | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index b8c9edaae7..614a10339f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -31,6 +31,11 @@ import { } from '@backstage/plugin-scaffolder-node'; import { UserEntity } from '@backstage/catalog-model'; import { z } from 'zod'; +import { + AuthorizeResult, + PermissionEvaluator, +} from '@backstage/plugin-permission-common'; +import { RESOURCE_TYPE_SCAFFOLDER_ACTION } from '@backstage/plugin-scaffolder-common/alpha'; // The Stream module is lazy loaded, so make sure it's in the module cache before mocking fs void winston.transports.Stream; @@ -51,6 +56,10 @@ describe('DefaultWorkflowRunner', () => { let runner: NunjucksWorkflowRunner; let fakeActionHandler: jest.Mock; + const mockedPermissionApi: jest.Mocked = { + authorizeConditional: jest.fn(), + } as unknown as jest.Mocked; + const integrations = ScmIntegrations.fromConfig( new ConfigReader({ integrations: { @@ -132,11 +141,16 @@ describe('DefaultWorkflowRunner', () => { }, }); + mockedPermissionApi.authorizeConditional.mockResolvedValue([ + { result: AuthorizeResult.ALLOW }, + ]); + runner = new NunjucksWorkflowRunner({ actionRegistry, integrations, workingDirectory: '/tmp', logger, + permissionApi: mockedPermissionApi, }); }); @@ -809,4 +823,83 @@ describe('DefaultWorkflowRunner', () => { expect(fakeActionHandler.mock.calls[0][0].isDryRun).toEqual(true); }); }); + + describe('permissions', () => { + it('should throw an error if an actions is not authorized', async () => { + mockedPermissionApi.authorizeConditional.mockResolvedValueOnce([ + { result: AuthorizeResult.DENY }, + ]); + + const task = createMockTaskWithSpec({ + apiVersion: 'scaffolder.backstage.io/v1beta3', + parameters: {}, + output: {}, + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-validated-action', + input: { foo: 1 }, + }, + ], + }); + + await expect(runner.execute(task)).rejects.toThrow( + /Unauthorized action: jest-validated-action. The action is not allowed/, + ); + expect(fakeActionHandler).not.toHaveBeenCalled(); + }); + + it(`shouldn't execute actions who aren't authorized`, async () => { + mockedPermissionApi.authorizeConditional.mockResolvedValueOnce([ + { + result: AuthorizeResult.CONDITIONAL, + pluginId: 'scaffolder', + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + conditions: { + anyOf: [ + { + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + rule: 'HAS_NUMBER_PROPERTY', + params: { + key: 'foo', + value: 1, + }, + }, + ], + }, + }, + ]); + + const task = createMockTaskWithSpec({ + apiVersion: 'scaffolder.backstage.io/v1beta3', + parameters: {}, + output: {}, + steps: [ + { + id: 'test1', + name: 'valid action', + action: 'jest-validated-action', + input: { foo: 1 }, + }, + { + id: 'test2', + name: 'invalid action', + action: 'jest-validated-action', + input: { foo: 2 }, + }, + ], + }); + + await expect(runner.execute(task)).rejects.toThrow( + `Unauthorized action: jest-validated-action. The action is not allowed. Input: ${JSON.stringify( + { foo: 2 }, + null, + 2, + )}`, + ); + expect(fakeActionHandler).toHaveBeenCalled(); + expect(mockedPermissionApi.authorizeConditional).toHaveBeenCalledTimes(1); + }); + }); }); From 7a3db152953dae968f88386c12e869877bc379dd Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 12:41:21 +0200 Subject: [PATCH 15/31] scaffolder: add actions condition exports Signed-off-by: Vincenzo Scamporlino --- .../src/service/conditionExports.ts | 111 +++++++++--------- 1 file changed, 57 insertions(+), 54 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index 8175ec9a00..7d22c9d262 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -21,69 +21,72 @@ import { import { createConditionExports } from '@backstage/plugin-permission-node'; import { scaffolderTemplateRules, scaffolderActionRules } from './rules'; -const { - conditions: scaffolderTemplateConditions, - createConditionalDecision: createScaffolderTemplateConditionalDecision, -} = createConditionExports({ +const templateConditionExports = createConditionExports({ pluginId: 'scaffolder', resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, rules: scaffolderTemplateRules, }); -const { - conditions: scaffolderActionConditions, - createConditionalDecision: createScaffolderActionConditionalDecision, -} = createConditionExports({ +const actionsConditionExports = createConditionExports({ pluginId: 'scaffolder', resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, rules: scaffolderActionRules, }); -export { - createScaffolderActionConditionalDecision, - scaffolderActionConditions, -}; +/** + * `createScaffolderTemplateConditionalDecision` 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 createScaffolderTemplateConditionalDecision = + templateConditionExports.createConditionalDecision; -export { - /** - * `createScaffolderTemplateConditionalDecision` 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 - */ - createScaffolderTemplateConditionalDecision, +/** + * These conditions are used when creating conditional decisions for scaffolder + * templates that are returned by authorization policies. + * + * @alpha + */ +export const scaffolderTemplateConditions = templateConditionExports.conditions; - /** - * These conditions are used when creating conditional decisions for scaffolder - * templates that are returned by authorization policies. - * - * @alpha - */ - scaffolderTemplateConditions, -}; +/** + * @alpha + */ +export const createScaffolderActionConditionalDecision = + actionsConditionExports.createConditionalDecision; + +/** + * + * These conditions are used when creating conditional decisions for scaffolder + * actions that are returned by authorization policies. + * + * @alpha + */ +export const scaffolderActionConditions = actionsConditionExports.conditions; From e9486a4701b06983b040ef912badd00e63370b4e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 12:42:21 +0200 Subject: [PATCH 16/31] scaffolder: actions api report Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-backend/alpha-api-report.md | 60 ++++++++++++++++--- plugins/scaffolder-backend/api-report.md | 15 ++--- plugins/scaffolder-common/alpha-api-report.md | 11 +++- 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/plugins/scaffolder-backend/alpha-api-report.md b/plugins/scaffolder-backend/alpha-api-report.md index e762c2be4f..1ea6c02cbe 100644 --- a/plugins/scaffolder-backend/alpha-api-report.md +++ b/plugins/scaffolder-backend/alpha-api-report.md @@ -6,6 +6,7 @@ import { BackendFeature } from '@backstage/backend-plugin-api'; import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; import { Conditions } from '@backstage/plugin-permission-node'; +import { JsonObject } from '@backstage/types'; import { PermissionCondition } from '@backstage/plugin-permission-common'; import { PermissionCriteria } from '@backstage/plugin-permission-common'; import { PermissionRule } from '@backstage/plugin-permission-node'; @@ -20,20 +21,53 @@ import { TemplateParametersV1beta3 } from '@backstage/plugin-scaffolder-common'; // @alpha export const catalogModuleTemplateKind: () => BackendFeature; +// @alpha (undocumented) +export const createScaffolderActionConditionalDecision: ( + permission: ResourcePermission<'scaffolder-action'>, + conditions: PermissionCriteria>, +) => ConditionalPolicyDecision; + // @alpha -export const createScaffolderConditionalDecision: ( +export const createScaffolderTemplateConditionalDecision: ( permission: ResourcePermission<'scaffolder-template'>, conditions: PermissionCriteria>, ) => ConditionalPolicyDecision; // @alpha -export const scaffolderConditions: Conditions<{ - hasTag: PermissionRule< - TemplateParametersV1beta3 | TemplateEntityStepV1beta3, - {}, - 'scaffolder-template', +export const scaffolderActionConditions: Conditions<{ + hasActionId: PermissionRule< { - tag: string; + action: string; + input: JsonObject | undefined; + }, + {}, + 'scaffolder-action', + { + actionId: string; + } + >; + hasNumberProperty: PermissionRule< + { + action: string; + input: JsonObject | undefined; + }, + {}, + 'scaffolder-action', + { + value: number; + key: string; + } + >; + hasStringProperty: PermissionRule< + { + action: string; + input: JsonObject | undefined; + }, + {}, + 'scaffolder-action', + { + value: string; + key: string; } >; }>; @@ -52,5 +86,17 @@ export type ScaffolderPluginOptions = { additionalTemplateGlobals?: Record; }; +// @alpha +export const scaffolderTemplateConditions: Conditions<{ + hasTag: PermissionRule< + TemplateEntityStepV1beta3 | TemplateParametersV1beta3, + {}, + 'scaffolder-template', + { + tag: string; + } + >; +}>; + // (No @packageDocumentation comment for this package) ``` diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 2faaeca7cc..7e6050fe1d 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -273,7 +273,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?: | ( | { @@ -388,7 +388,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; @@ -408,7 +408,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; }, @@ -424,7 +424,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; @@ -517,7 +517,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?: | ( | { @@ -571,7 +571,7 @@ export function createPublishGitlabAction(options: { { repoUrl: string; defaultBranch?: string | undefined; - repoVisibility?: 'internal' | 'private' | 'public' | undefined; + repoVisibility?: 'public' | 'private' | 'internal' | undefined; sourcePath?: string | undefined; token?: string | undefined; gitCommitMessage?: string | undefined; @@ -595,7 +595,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; @@ -650,6 +650,7 @@ export type CreateWorkerOptions = { additionalTemplateFilters?: Record; concurrentTasksLimit?: number; additionalTemplateGlobals?: Record; + permissionApi?: PermissionEvaluator; }; // @public diff --git a/plugins/scaffolder-common/alpha-api-report.md b/plugins/scaffolder-common/alpha-api-report.md index e89999ad85..d6e746026a 100644 --- a/plugins/scaffolder-common/alpha-api-report.md +++ b/plugins/scaffolder-common/alpha-api-report.md @@ -5,11 +5,20 @@ ```ts import { ResourcePermission } from '@backstage/plugin-permission-common'; +// @alpha +export const actionExecutePermission: ResourcePermission<'scaffolder-action'>; + +// @alpha +export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action'; + // @alpha export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; // @alpha -export const scaffolderPermissions: ResourcePermission<'scaffolder-template'>[]; +export const scaffolderPermissions: ( + | ResourcePermission<'scaffolder-action'> + | ResourcePermission<'scaffolder-template'> +)[]; // @alpha export const templateParameterReadPermission: ResourcePermission<'scaffolder-template'>; From 220247b833082ec1f30a1b46ce5b0fd626e54383 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 14:25:42 +0200 Subject: [PATCH 17/31] backend: remove zod Signed-off-by: Vincenzo Scamporlino --- packages/backend/package.json | 3 +-- yarn.lock | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index 9fc07e6e4f..304ea34877 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -84,8 +84,7 @@ "pg": "^8.3.0", "pg-connection-string": "^2.3.0", "prom-client": "^14.0.1", - "winston": "^3.2.1", - "zod": "~3.18.0" + "winston": "^3.2.1" }, "devDependencies": { "@backstage/cli": "workspace:^", diff --git a/yarn.lock b/yarn.lock index 093320debe..e3f29991af 100644 --- a/yarn.lock +++ b/yarn.lock @@ -23522,7 +23522,6 @@ __metadata: pg-connection-string: ^2.3.0 prom-client: ^14.0.1 winston: ^3.2.1 - zod: ~3.18.0 languageName: unknown linkType: soft From 05cbe289dd22af6e1d00726f5bec206538f3d257 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 14:32:48 +0200 Subject: [PATCH 18/31] scaffolder: remove token from TemplateContext Signed-off-by: Vincenzo Scamporlino --- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index e93085b4e2..d3ed2d8e03 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -76,7 +76,6 @@ type TemplateContext = { entity?: UserEntity; ref?: string; }; - token?: string; }; const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta3 => { From d19aa072c8bb92467abdce5916631bfd0f8f2267 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 4 Apr 2023 15:12:14 +0200 Subject: [PATCH 19/31] scaffolder: sync api report Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/alpha-api-report.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/alpha-api-report.md b/plugins/scaffolder-backend/alpha-api-report.md index 1ea6c02cbe..c59432a4ec 100644 --- a/plugins/scaffolder-backend/alpha-api-report.md +++ b/plugins/scaffolder-backend/alpha-api-report.md @@ -89,7 +89,7 @@ export type ScaffolderPluginOptions = { // @alpha export const scaffolderTemplateConditions: Conditions<{ hasTag: PermissionRule< - TemplateEntityStepV1beta3 | TemplateParametersV1beta3, + TemplateParametersV1beta3 | TemplateEntityStepV1beta3, {}, 'scaffolder-template', { From f65a32b342959c3ad7092b4b58287f1ef9667c9c Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 5 Apr 2023 12:26:31 +0200 Subject: [PATCH 20/31] scaffolder: make action rules generic Co-Authored-By: Ainhoa Larumbe Co-Authored-By: Mike Lewis Signed-off-by: Vincenzo Scamporlino --- .../scaffolder-backend/src/service/rules.ts | 88 ++++++++----------- 1 file changed, 36 insertions(+), 52 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index d03f8daa2e..c93681a0fb 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -26,8 +26,8 @@ import { } from '@backstage/plugin-scaffolder-common'; import { z } from 'zod'; -import { JsonObject } from '@backstage/types'; -import { get } from 'lodash'; +import { JsonObject, JsonPrimitive, JsonValue } from '@backstage/types'; +import { String, get } from 'lodash'; export const createTemplatePermissionRule = makeCreatePermissionRule< TemplateEntityStepV1beta3 | TemplateParametersV1beta3, @@ -70,63 +70,47 @@ export const hasActionId = createActionPermissionRule({ toQuery: () => ({}), }); -export const hasNumberProperty = createActionPermissionRule({ - name: `HAS_NUMBER_PROPERTY`, - description: `Allow actions with the specified property`, - resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, - paramsSchema: z.object({ - key: z - .string() - .describe(`Property within the action parameters to match on`), - value: z.number().describe(`Value of the given property to match on`), - }), - apply: (resource, { key, value }) => { - const foundValue = get(resource.input, key); +export const hasBooleanProperty = buildHasProperty(z.boolean()); +export const hasNullProperty = buildHasProperty(z.null()); +export const hasNumberProperty = buildHasProperty(z.number()); +export const hasStringProperty = buildHasProperty(z.string()); - if (Array.isArray(foundValue)) { - if (value !== undefined) { - return foundValue.includes(value); +function buildHasProperty>( + valueSchema: Schema, +) { + return createActionPermissionRule({ + name: `HAS_STRING_PROPERTY`, + description: `Allow actions with the specified property`, + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, + paramsSchema: z.object({ + key: z + .string() + .describe(`Property within the action parameters to match on`), + value: valueSchema.describe(`Value of the given property to match on`), + }) as unknown as z.ZodType<{ key: string; value: z.infer }>, + apply: (resource, { key, value }) => { + const foundValue = get(resource.input, key); + + if (Array.isArray(foundValue)) { + if (value !== undefined) { + return foundValue.includes(value); + } + return foundValue.length > 0; } - return foundValue.length > 0; - } - if (value !== undefined && z.number().safeParse(value).success) { - return value === foundValue; - } - return !!foundValue; - }, - toQuery: () => ({}), -}); - -export const hasStringProperty = createActionPermissionRule({ - name: `HAS_STRING_PROPERTY`, - description: `Allow actions with the specified property`, - resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, - paramsSchema: z.object({ - key: z - .string() - .describe(`Property within the action parameters to match on`), - value: z.string().describe(`Value of the given property to match on`), - }), - apply: (resource, { key, value }) => { - const foundValue = get(resource.input, key); - - if (Array.isArray(foundValue)) { - if (value !== undefined) { - return foundValue.includes(value); + if (value !== undefined && z.string().safeParse(value).success) { + return value === foundValue; } - return foundValue.length > 0; - } - if (value !== undefined && z.string().safeParse(value).success) { - return value === foundValue; - } - return !!foundValue; - }, - toQuery: () => ({}), -}); + return !!foundValue; + }, + toQuery: () => ({}), + }); +} export const scaffolderTemplateRules = { hasTag }; export const scaffolderActionRules = { hasActionId, + hasBooleanProperty, + hasNullProperty, hasNumberProperty, hasStringProperty, }; From 4324322ab4c93db1df7973c7f3e3efd856164e5b Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 18:23:12 +0200 Subject: [PATCH 21/31] scaffolder-backend: add tests for action rules Signed-off-by: Vincenzo Scamporlino --- .../src/service/rules.test.ts | 365 +++++++++++++++++- .../scaffolder-backend/src/service/rules.ts | 60 ++- 2 files changed, 404 insertions(+), 21 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/rules.test.ts b/plugins/scaffolder-backend/src/service/rules.test.ts index 7d8e0eb445..e6032dde15 100644 --- a/plugins/scaffolder-backend/src/service/rules.test.ts +++ b/plugins/scaffolder-backend/src/service/rules.test.ts @@ -14,7 +14,15 @@ * limitations under the License. */ -import { hasTag } from './rules'; +import { JsonObject, JsonPrimitive } from '@backstage/types'; +import { + hasActionId, + hasBooleanProperty, + hasNumberProperty, + hasProperty, + hasStringProperty, + hasTag, +} from './rules'; describe('hasTag', () => { describe('apply', () => { @@ -79,3 +87,358 @@ describe('hasTag', () => { }); }); }); + +describe('hasActionId', () => { + describe('apply', () => { + it('returns false when actionId is not matched', () => { + expect( + hasActionId.apply( + { + action: 'action', + input: {}, + }, + { + actionId: 'not-matched', + }, + ), + ).toEqual(false); + }); + + it('returns true when actionId is matched', () => { + expect( + hasActionId.apply( + { + action: 'action', + input: {}, + }, + { + actionId: 'action', + }, + ), + ).toEqual(true); + }); + }); +}); + +const input: JsonObject = { + propwithstring: '1', + propwithnumber: 2, + propwithobject: {}, + propwithnull: null, + propwithfalse: false, + propwithtrue: true, + propwitharray: ['item', 0, true, false], + nested: { propwithstring: '1', nested: { propwithnumber: 1 } }, +}; + +describe('hasProperty', () => { + describe('apply', () => { + it.each([ + 'foo', + 'bar', + 'prop.prop', + 'nested.nonexisting', + '', + 'propwitharray.100', + ])(`returns false when a property doesn't exist in the input`, key => { + expect(hasProperty.apply({ action: 'action', input }, { key })).toEqual( + false, + ); + }); + + it.each([ + 'propwithstring', + 'propwithnumber', + 'propwithobject', + 'propwithnull', + 'propwithfalse', + 'propwithtrue', + 'propwitharray', + 'propwitharray.1', + 'nested.propwithstring', + 'nested.nested', + 'nested.nested.propwithnumber', + ])(`returns true when a property exists, property=%s`, key => { + expect(hasProperty.apply({ action: 'action', input }, { key })).toEqual( + true, + ); + }); + + it.each([ + ['propwithstring', 1], + ['propwithnumber', '2'], + ['propwithnumber', true], + ['propwithobject', [{}]], + ['propwithnull', false], + ['propwithfalse', true], + ['propwithtrue', null], + ['propwitharray', 'nonexistingitem'], + ['propwitharray.0', 'nonmatchingitem'], + ['nested.propwithstring', 'x'], + ['nested.nested', '1'], + ['nested.nested.propwithnumber', 'ops'], + ])( + `returns false when a property exists but the value doesn't match, key=%s value=%o`, + (key, value) => { + expect( + hasProperty.apply( + { action: 'action', input }, + { key, value: value as JsonPrimitive }, + ), + ).toEqual(false); + }, + ); + + it.each([ + ['propwithstring', '1'], + ['propwithnumber', 2], + ['propwithnull', null], + ['propwithfalse', false], + ['propwithtrue', true], + ['propwitharray.0', 'item'], + ['nested.propwithstring', '1'], + ['nested.nested.propwithnumber', 1], + ])( + `returns true when a property exists and the value matches, key=%s value=%o`, + (key, value) => { + expect( + hasProperty.apply({ action: 'action', input }, { key, value }), + ).toEqual(true); + }, + ); + }); +}); + +describe('hasBooleanProperty', () => { + describe('apply', () => { + it.each(['foo', 'bar', 'prop.prop', 'nested.nonexisting', ''])( + `returns false when a property doesn't exist in the input`, + key => { + expect( + hasBooleanProperty.apply({ action: 'action', input }, { key }), + ).toEqual(false); + }, + ); + + it.each([ + 'propwithstring', + 'propwithnumber', + 'propwithobject', + 'propwithnull', + 'propwitharray', + 'propwitharray.0', + 'nested.propwithstring', + 'nested.nested', + 'nested.nested.propwithnumber', + ])( + `returns false when a property exists and is not a boolean, property=%s`, + key => { + expect( + hasBooleanProperty.apply({ action: 'action', input }, { key }), + ).toEqual(false); + }, + ); + + it.each([ + 'propwithfalse', + 'propwithtrue', + 'propwitharray.2', + 'propwitharray.3', + ])(`returns true when a property exists, property=%s`, key => { + expect( + hasBooleanProperty.apply({ action: 'action', input }, { key }), + ).toEqual(true); + }); + + it.each([ + ['propwithstring', true], + ['propwithnumber', true], + ['propwithnumber', true], + ['propwithobject', true], + ['propwithnull', true], + ['propwithfalse', true], + ['propwithtrue', false], + ['propwitharray', true], + ['propwitharray.2', false], + ['propwitharray.3', true], + ['nested.propwithstring', true], + ['nested.nested', true], + ['nested.nested.propwithnumber', true], + ])( + `returns false when a property exists but the value doesn't match, key=%s value=%o`, + (key, value) => { + expect( + hasBooleanProperty.apply({ action: 'action', input }, { key, value }), + ).toEqual(false); + }, + ); + + it.each([ + ['propwithfalse', false], + ['propwithtrue', true], + ['propwitharray.2', true], + ['propwitharray.3', false], + ])( + `returns true when a property exists and the value matches, key=%s value=%o`, + (key, value) => { + expect( + hasBooleanProperty.apply({ action: 'action', input }, { key, value }), + ).toEqual(true); + }, + ); + }); +}); + +describe('hasNumberProperty', () => { + describe('apply', () => { + it.each(['foo', 'bar', 'prop.prop', 'nested.nonexisting', ''])( + `returns false when a property doesn't exist in the input`, + key => { + expect( + hasNumberProperty.apply({ action: 'action', input }, { key }), + ).toEqual(false); + }, + ); + + it.each([ + 'propwithstring', + 'propwithobject', + 'propwithnull', + 'propwithfalse', + 'propwithtrue', + 'propwitharray', + 'propwitharray.0', + 'nested.propwithstring', + 'nested.nested', + ])( + `returns false when a property exists and is not a number, property=%s`, + key => { + expect( + hasNumberProperty.apply({ action: 'action', input }, { key }), + ).toEqual(false); + }, + ); + + it.each([ + 'propwithnumber', + 'nested.nested.propwithnumber', + 'propwitharray.1', + ])(`returns true when a property exists, property=%s`, key => { + expect( + hasNumberProperty.apply({ action: 'action', input }, { key }), + ).toEqual(true); + }); + + it.each([ + ['propwithstring', 1], + ['propwithnumber', 1000], + ['propwithnumber', 101], + ['propwithobject', 1], + ['propwithnull', 1], + ['propwithfalse', 1], + ['propwithtrue', 1], + ['propwitharray', 1], + ['propwitharray.2', 1], + ['propwitharray.3', 1], + ['nested.propwithstring', 1], + ['nested.nested', 1], + ['nested.nested.propwithnumber', 100], + ])( + `returns false when a property exists but the value doesn't match, key=%s value=%o`, + (key, value) => { + expect( + hasNumberProperty.apply({ action: 'action', input }, { key, value }), + ).toEqual(false); + }, + ); + + it.each([ + ['propwithnumber', 2], + ['nested.nested.propwithnumber', 1], + ['propwitharray.1', 0], + ])( + `returns true when a property exists and the value matches, key=%s value=%o`, + (key, value) => { + expect( + hasNumberProperty.apply({ action: 'action', input }, { key, value }), + ).toEqual(true); + }, + ); + }); +}); + +describe('hasStringProperty', () => { + describe('apply', () => { + it.each(['foo', 'bar', 'prop.prop', 'nested.nonexisting', ''])( + `returns false when a property doesn't exist in the input`, + key => { + expect( + hasStringProperty.apply({ action: 'action', input }, { key }), + ).toEqual(false); + }, + ); + + it.each([ + 'propwithnumber', + 'propwithobject', + 'propwithnull', + 'propwithfalse', + 'propwithtrue', + 'propwitharray', + 'propwitharray.1', + 'nested.nested.propwithnumber', + 'nested.nested', + ])( + `returns false when a property exists and is not a string, property=%s`, + key => { + expect( + hasStringProperty.apply({ action: 'action', input }, { key }), + ).toEqual(false); + }, + ); + + it.each(['propwithstring', 'nested.propwithstring', 'propwitharray.0'])( + `returns true when a property exists, property=%s`, + key => { + expect( + hasStringProperty.apply({ action: 'action', input }, { key }), + ).toEqual(true); + }, + ); + + it.each([ + ['propwithstring', 'nonmatchingstring'], + ['propwithnumber', 's'], + ['propwithnumber', 's'], + ['propwithobject', 's'], + ['propwithnull', 's'], + ['propwithfalse', 's'], + ['propwithtrue', 's'], + ['propwitharray', 's'], + ['propwitharray.2', 's'], + ['propwitharray.3', 's'], + ['nested.nested', 's'], + ['nested.nested.propwithnumber', 's'], + ])( + `returns false when a property exists but the value doesn't match, key=%s value=%o`, + (key, value) => { + expect( + hasStringProperty.apply({ action: 'action', input }, { key, value }), + ).toEqual(false); + }, + ); + + it.each([ + ['propwithstring', '1'], + ['nested.propwithstring', '1'], + ['propwitharray.0', 'item'], + ])( + `returns true when a property exists and the value matches, key=%s value=%o`, + (key, value) => { + expect( + hasStringProperty.apply({ action: 'action', input }, { key, value }), + ).toEqual(true); + }, + ); + }); +}); diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index c93681a0fb..19b34b9f7c 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -26,8 +26,8 @@ import { } from '@backstage/plugin-scaffolder-common'; import { z } from 'zod'; -import { JsonObject, JsonPrimitive, JsonValue } from '@backstage/types'; -import { String, get } from 'lodash'; +import { JsonObject, JsonPrimitive } from '@backstage/types'; +import { get } from 'lodash'; export const createTemplatePermissionRule = makeCreatePermissionRule< TemplateEntityStepV1beta3 | TemplateParametersV1beta3, @@ -70,16 +70,36 @@ export const hasActionId = createActionPermissionRule({ toQuery: () => ({}), }); -export const hasBooleanProperty = buildHasProperty(z.boolean()); -export const hasNullProperty = buildHasProperty(z.null()); -export const hasNumberProperty = buildHasProperty(z.number()); -export const hasStringProperty = buildHasProperty(z.string()); +export const hasProperty = buildHasProperty({ + name: 'HAS_PROPERTY', + valueSchema: z.union([z.string(), z.number(), z.boolean(), z.null()]), + validateProperty: false, +}); -function buildHasProperty>( - valueSchema: Schema, -) { +export const hasBooleanProperty = buildHasProperty({ + name: 'HAS_BOOLEAN_PROPERTY', + valueSchema: z.boolean(), +}); +export const hasNumberProperty = buildHasProperty({ + name: 'HAS_NUMBER_PROPERTY', + valueSchema: z.number(), +}); +export const hasStringProperty = buildHasProperty({ + name: 'HAS_STRING_PROPERTY', + valueSchema: z.string(), +}); + +function buildHasProperty>({ + name, + valueSchema, + validateProperty = true, +}: { + name: string; + valueSchema: Schema; + validateProperty?: boolean; +}) { return createActionPermissionRule({ - name: `HAS_STRING_PROPERTY`, + name, description: `Allow actions with the specified property`, resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, paramsSchema: z.object({ @@ -87,20 +107,21 @@ function buildHasProperty>( .string() .describe(`Property within the action parameters to match on`), value: valueSchema.describe(`Value of the given property to match on`), - }) as unknown as z.ZodType<{ key: string; value: z.infer }>, + }) as unknown as z.ZodType<{ key: string; value?: z.infer }>, apply: (resource, { key, value }) => { const foundValue = get(resource.input, key); - if (Array.isArray(foundValue)) { - if (value !== undefined) { - return foundValue.includes(value); + if (validateProperty && !valueSchema.safeParse(foundValue).success) { + return false; + } + if (value !== undefined) { + if (valueSchema.safeParse(value).success) { + return value === foundValue; } - return foundValue.length > 0; + return false; } - if (value !== undefined && z.string().safeParse(value).success) { - return value === foundValue; - } - return !!foundValue; + + return foundValue !== undefined; }, toQuery: () => ({}), }); @@ -110,7 +131,6 @@ export const scaffolderTemplateRules = { hasTag }; export const scaffolderActionRules = { hasActionId, hasBooleanProperty, - hasNullProperty, hasNumberProperty, hasStringProperty, }; From 3e293a9a2cdfbfe4fbbcaf993b4a368b278416b8 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 18:30:03 +0200 Subject: [PATCH 22/31] scaffolder-backend: api report action rules Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/alpha-api-report.md | 16 ++++++++++++++-- plugins/scaffolder-backend/api-report.md | 14 +++++++------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/plugins/scaffolder-backend/alpha-api-report.md b/plugins/scaffolder-backend/alpha-api-report.md index c59432a4ec..fe7ab73420 100644 --- a/plugins/scaffolder-backend/alpha-api-report.md +++ b/plugins/scaffolder-backend/alpha-api-report.md @@ -46,6 +46,18 @@ export const scaffolderActionConditions: Conditions<{ actionId: string; } >; + hasBooleanProperty: PermissionRule< + { + action: string; + input: JsonObject | undefined; + }, + {}, + 'scaffolder-action', + { + key: string; + value?: boolean | undefined; + } + >; hasNumberProperty: PermissionRule< { action: string; @@ -54,8 +66,8 @@ export const scaffolderActionConditions: Conditions<{ {}, 'scaffolder-action', { - value: number; key: string; + value?: number | undefined; } >; hasStringProperty: PermissionRule< @@ -66,8 +78,8 @@ export const scaffolderActionConditions: Conditions<{ {}, 'scaffolder-action', { - value: string; key: string; + value?: string | undefined; } >; }>; diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 7e6050fe1d..5bcb38308b 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -273,7 +273,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?: | ( | { @@ -388,7 +388,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; @@ -408,7 +408,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; }, @@ -424,7 +424,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; @@ -517,7 +517,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?: | ( | { @@ -571,7 +571,7 @@ export function createPublishGitlabAction(options: { { repoUrl: string; defaultBranch?: string | undefined; - repoVisibility?: 'public' | 'private' | 'internal' | undefined; + repoVisibility?: 'internal' | 'private' | 'public' | undefined; sourcePath?: string | undefined; token?: string | undefined; gitCommitMessage?: string | undefined; @@ -595,7 +595,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; From d271f2ad686382361cb581d708076cfa19d5cd56 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 18:34:35 +0200 Subject: [PATCH 23/31] backend: clean up example policy and template Signed-off-by: Vincenzo Scamporlino --- packages/backend/package.json | 1 - packages/backend/src/plugins/permission.ts | 21 ------------- template.yaml | 34 ---------------------- yarn.lock | 1 - 4 files changed, 57 deletions(-) delete mode 100644 template.yaml diff --git a/packages/backend/package.json b/packages/backend/package.json index 304ea34877..fd8ec82d91 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/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index a14a113b36..7192a1ddec 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, - isPermission, PolicyDecision, } from '@backstage/plugin-permission-common'; import { @@ -29,11 +28,6 @@ import { DefaultPlaylistPermissionPolicy, isPlaylistPermission, } from '@backstage/plugin-playlist-backend'; -import { - createScaffolderActionConditionalDecision, - scaffolderActionConditions, -} from '@backstage/plugin-scaffolder-backend/alpha'; -import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; @@ -48,21 +42,6 @@ class ExamplePermissionPolicy implements PermissionPolicy { return this.playlistPermissionPolicy.handle(request, user); } - if (isPermission(request.permission, actionExecutePermission)) { - return createScaffolderActionConditionalDecision(request.permission, { - allOf: [ - scaffolderActionConditions.hasStringProperty({ - key: 'message', - value: 'Test', - }), - scaffolderActionConditions.hasStringProperty({ - key: 'message', - value: 'Hello ddd', - }), - ], - }); - } - return { result: AuthorizeResult.ALLOW, }; diff --git a/template.yaml b/template.yaml deleted file mode 100644 index fef90dbcd8..0000000000 --- a/template.yaml +++ /dev/null @@ -1,34 +0,0 @@ -apiVersion: scaffolder.backstage.io/v1beta3 -kind: Template -metadata: - name: my-custom-template -spec: - type: service - parameters: - - title: Basic information - properties: - description: - title: Description - type: string - - title: Extra information - backstage:accessControl: - tags: - - foo - properties: - name: - title: Your name - type: string - steps: - - id: step-one - name: First log - action: debug:log - input: - message: Test - - id: step-two - name: Second log - action: debug:log - input: - message: Hello ${{ parameters.name }} - backstage:accessControl: - tags: - - foo diff --git a/yarn.lock b/yarn.lock index e3f29991af..40d006519f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -23493,7 +23493,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 bcae5aaf25cd833d54e5093afd70450e863774ea Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 18:47:02 +0200 Subject: [PATCH 24/31] scaffolder: add changesets Signed-off-by: Vincenzo Scamporlino --- .changeset/silly-eels-admire.md | 7 +++++++ .changeset/ten-games-rush.md | 5 +++++ 2 files changed, 12 insertions(+) create mode 100644 .changeset/silly-eels-admire.md create mode 100644 .changeset/ten-games-rush.md diff --git a/.changeset/silly-eels-admire.md b/.changeset/silly-eels-admire.md new file mode 100644 index 0000000000..ddedf41929 --- /dev/null +++ b/.changeset/silly-eels-admire.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Added the possibility to authorize actions + +It is now possible to decide who should be able to execute certain actions or who should be able to pass specific input to specified actions. diff --git a/.changeset/ten-games-rush.md b/.changeset/ten-games-rush.md new file mode 100644 index 0000000000..ceb8604080 --- /dev/null +++ b/.changeset/ten-games-rush.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-common': patch +--- + +Added permissions for authorizing actions From 6e5a809ea19b6f22968c449183340fd1824116c6 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 19:08:06 +0200 Subject: [PATCH 25/31] rollback dependencies don't ask me why Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index dbcf0f624a..97d4577d23 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -66,6 +66,8 @@ "@gitbeaker/core": "^35.6.0", "@gitbeaker/node": "^35.1.0", "@octokit/webhooks": "^10.0.0", + "@types/express": "^4.17.6", + "@types/luxon": "^3.0.0", "azure-devops-node-api": "^11.0.1", "command-exists": "^1.2.9", "compression": "^1.7.4", @@ -100,10 +102,8 @@ "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@types/command-exists": "^1.2.0", - "@types/express": "^4.17.6", "@types/fs-extra": "^9.0.1", "@types/git-url-parse": "^9.0.0", - "@types/luxon": "^3.0.0", "@types/mock-fs": "^4.13.0", "@types/nunjucks": "^3.1.4", "@types/supertest": "^2.0.8", From ddf7b5ed912a15a829ae884b0b9809b39091c071 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 14 Apr 2023 11:53:38 +0200 Subject: [PATCH 26/31] scaffolder-backend: NotAllowedError instead of InputError Signed-off-by: Vincenzo Scamporlino --- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index d3ed2d8e03..a82ad1ae55 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -26,7 +26,7 @@ import fs from 'fs-extra'; import path from 'path'; import nunjucks from 'nunjucks'; import { JsonObject, JsonValue } from '@backstage/types'; -import { InputError } from '@backstage/errors'; +import { InputError, NotAllowedError } from '@backstage/errors'; import { PassThrough } from 'stream'; import { generateExampleOutput, isTruthy } from './helper'; import { validate as validateJsonSchema } from 'jsonschema'; @@ -297,7 +297,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { } if (!isActionAuthorized(decision, { action: action.id, input })) { - throw new InputError( + throw new NotAllowedError( `Unauthorized action: ${ action.id }. The action is not allowed. Input: ${JSON.stringify( From 067370e1f0e5196decc5318c222d2bb63854be31 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 14 Apr 2023 13:36:04 +0200 Subject: [PATCH 27/31] scaffolder-backend: improve changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/silly-eels-admire.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.changeset/silly-eels-admire.md b/.changeset/silly-eels-admire.md index ddedf41929..0d39027518 100644 --- a/.changeset/silly-eels-admire.md +++ b/.changeset/silly-eels-admire.md @@ -5,3 +5,8 @@ Added the possibility to authorize actions It is now possible to decide who should be able to execute certain actions or who should be able to pass specific input to specified actions. + +Some of the existing utility functions for creating conditional decisions have been renamed: + +- `createScaffolderConditionalDecision` has been renamed to `createScaffolderActionConditionalDecision` +- `scaffolderConditions` has been renamed to `scaffolderTemplateConditions` From 3b68b09fc2d87fbed1f6aa2714207113c5ad4476 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 14 Apr 2023 16:13:16 +0200 Subject: [PATCH 28/31] scaffolder: rename permissions dependency Signed-off-by: Vincenzo Scamporlino --- .changeset/seven-oranges-act.md | 5 +++++ plugins/scaffolder-backend/src/ScaffolderPlugin.ts | 2 +- .../src/scaffolder/dryrun/createDryRunner.ts | 2 +- .../scaffolder/tasks/NunjucksWorkflowRunner.test.ts | 2 +- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 6 +++--- .../src/scaffolder/tasks/TaskWorker.ts | 10 +++++----- .../scaffolder-backend/src/service/router.test.ts | 4 ++-- plugins/scaffolder-backend/src/service/router.ts | 12 ++++++------ 8 files changed, 24 insertions(+), 19 deletions(-) create mode 100644 .changeset/seven-oranges-act.md diff --git a/.changeset/seven-oranges-act.md b/.changeset/seven-oranges-act.md new file mode 100644 index 0000000000..c1b3bc61c0 --- /dev/null +++ b/.changeset/seven-oranges-act.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Renamed permissionApi router option to permissions diff --git a/plugins/scaffolder-backend/src/ScaffolderPlugin.ts b/plugins/scaffolder-backend/src/ScaffolderPlugin.ts index 97abf41828..6b1ca1d164 100644 --- a/plugins/scaffolder-backend/src/ScaffolderPlugin.ts +++ b/plugins/scaffolder-backend/src/ScaffolderPlugin.ts @@ -132,7 +132,7 @@ export const scaffolderPlugin = createBackendPlugin( taskWorkers, additionalTemplateFilters, additionalTemplateGlobals, - permissionApi: permissions, + permissions, }); httpRouter.use(router); }, diff --git a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts index 03db2147a7..655c485a6d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/dryrun/createDryRunner.ts @@ -57,7 +57,7 @@ export type TemplateTesterCreateOptions = { workingDirectory: string; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; - permissionApi?: PermissionEvaluator; + permissions?: PermissionEvaluator; }; /** diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 614a10339f..5d4f7b73cc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -150,7 +150,7 @@ describe('DefaultWorkflowRunner', () => { integrations, workingDirectory: '/tmp', logger, - permissionApi: mockedPermissionApi, + permissions: mockedPermissionApi, }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index a82ad1ae55..da690bcb82 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -63,7 +63,7 @@ type NunjucksWorkflowRunnerOptions = { logger: winston.Logger; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; - permissionApi?: PermissionEvaluator; + permissions?: PermissionEvaluator; }; type TemplateContext = { @@ -383,8 +383,8 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { }; const [decision]: PolicyDecision[] = - this.options.permissionApi && task.spec.steps.length - ? await this.options.permissionApi.authorizeConditional( + this.options.permissions && task.spec.steps.length + ? await this.options.permissions.authorizeConditional( [{ permission: actionExecutePermission }], { token: task.secrets?.backstageToken }, ) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 960c89511a..3b80c74832 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -34,7 +34,7 @@ export type TaskWorkerOptions = { workflowRunner: WorkflowRunner; }; concurrentTasksLimit: number; - permissionApi?: PermissionEvaluator; + permissions?: PermissionEvaluator; }; /** @@ -63,7 +63,7 @@ export type CreateWorkerOptions = { */ concurrentTasksLimit?: number; additionalTemplateGlobals?: Record; - permissionApi?: PermissionEvaluator; + permissions?: PermissionEvaluator; }; /** @@ -88,7 +88,7 @@ export class TaskWorker { additionalTemplateFilters, concurrentTasksLimit = 10, // from 1 to Infinity additionalTemplateGlobals, - permissionApi, + permissions, } = options; const workflowRunner = new NunjucksWorkflowRunner({ @@ -98,14 +98,14 @@ export class TaskWorker { workingDirectory, additionalTemplateFilters, additionalTemplateGlobals, - permissionApi, + permissions, }); return new TaskWorker({ taskBroker: taskBroker, runners: { workflowRunner }, concurrentTasksLimit, - permissionApi, + permissions, }); } diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 4f5ed0fe22..f5aa043ce0 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -195,7 +195,7 @@ describe('createRouter', () => { catalogClient, reader: mockUrlReader, taskBroker, - permissionApi, + permissions: permissionApi, }); app = express().use(router); @@ -819,7 +819,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ reader: mockUrlReader, taskBroker, identity: { getIdentity }, - permissionApi, + permissions: permissionApi, }); app = express().use(router); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index d4d55bcb51..0a45f2017d 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -112,7 +112,7 @@ export interface RouterOptions { taskBroker?: TaskBroker; additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; - permissionApi?: PermissionEvaluator; + permissions?: PermissionEvaluator; permissionRules?: TemplatePermissionRuleInput[]; identity?: IdentityApi; } @@ -208,7 +208,7 @@ export async function createRouter( scheduler, additionalTemplateFilters, additionalTemplateGlobals, - permissionApi, + permissions, permissionRules, } = options; @@ -258,7 +258,7 @@ export async function createRouter( additionalTemplateFilters, additionalTemplateGlobals, concurrentTasksLimit, - permissionApi, + permissions, }); workers.push(worker); } @@ -284,7 +284,7 @@ export async function createRouter( workingDirectory, additionalTemplateFilters, additionalTemplateGlobals, - permissionApi, + permissions, }); const templateRules: TemplatePermissionRuleInput[] = Object.values( @@ -616,12 +616,12 @@ export async function createRouter( ); } - if (!permissionApi) { + if (!permissions) { return template; } const [parameterDecision, stepDecision] = - await permissionApi.authorizeConditional( + await permissions.authorizeConditional( [ { permission: templateParameterReadPermission }, { permission: templateStepReadPermission }, From 10744537bf555d723c7bde85f9be54402c073a0c Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 14 Apr 2023 16:18:42 +0200 Subject: [PATCH 29/31] create-app: fix scaffolder dependency Signed-off-by: Vincenzo Scamporlino --- .changeset/pretty-yaks-carry.md | 5 +++++ .../default-app/packages/backend/src/plugins/scaffolder.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/pretty-yaks-carry.md diff --git a/.changeset/pretty-yaks-carry.md b/.changeset/pretty-yaks-carry.md new file mode 100644 index 0000000000..4c160156b5 --- /dev/null +++ b/.changeset/pretty-yaks-carry.md @@ -0,0 +1,5 @@ +--- +'@backstage/create-app': minor +--- + +Renamed `permissionApi` to `permissions` when passed as dependency of the scaffolder-backend plugin 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 fd424a3257..a12fee2295 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,6 +17,6 @@ export default async function createPlugin( reader: env.reader, catalogClient, identity: env.identity, - permissionApi: env.permissions, + permissions: env.permissions, }); } From 5fb2a2c281f075e70e648fb9bf3cc4836e6f6c4d Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 14 Apr 2023 16:19:25 +0200 Subject: [PATCH 30/31] backend: fix scaffolder backend dependencies Signed-off-by: Vincenzo Scamporlino --- packages/backend/src/plugins/scaffolder.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index 813092c9b9..821e5c1adf 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -34,6 +34,6 @@ export default async function createPlugin( reader: env.reader, identity: env.identity, scheduler: env.scheduler, - permissionApi: env.permissions, + permissions: env.permissions, }); } From 3c7b8fadf1ebc6e6f79872969d64b052ddb1cd9e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 14 Apr 2023 16:21:00 +0200 Subject: [PATCH 31/31] scaffolder-backend: api report Signed-off-by: Vincenzo Scamporlino --- plugins/scaffolder-backend/api-report.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 5bcb38308b..34e20be58e 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -650,7 +650,7 @@ export type CreateWorkerOptions = { additionalTemplateFilters?: Record; concurrentTasksLimit?: number; additionalTemplateGlobals?: Record; - permissionApi?: PermissionEvaluator; + permissions?: PermissionEvaluator; }; // @public @@ -762,10 +762,10 @@ export interface RouterOptions { // (undocumented) logger: Logger; // (undocumented) - permissionApi?: PermissionEvaluator; - // (undocumented) permissionRules?: TemplatePermissionRuleInput[]; // (undocumented) + permissions?: PermissionEvaluator; + // (undocumented) reader: UrlReader; // (undocumented) scheduler?: PluginTaskScheduler;