From ddb5d1c72b1bd4caaad8bfca07173981c135a11a Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 8 Feb 2023 22:47:33 +0100 Subject: [PATCH] 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