From 277847e064b2911902f64e586bebf11b0644929c Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Thu, 12 Jan 2023 12:11:18 +0000 Subject: [PATCH] Reworked permission rules and filtering to follow similar pattern to catalog Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- app-config.yaml | 3 + packages/backend/src/plugins/permission.ts | 24 ++-- .../createPermissionIntegrationRouter.ts | 2 +- .../permission-node/src/integration/index.ts | 7 +- .../src/service/conditionExports.ts | 62 +++++----- .../scaffolder-backend/src/service/router.ts | 55 +++------ .../scaffolder-backend/src/service/rules.ts | 106 ++++-------------- .../src/TemplateEntityV1beta3.ts | 22 ++-- plugins/scaffolder-common/src/index.ts | 5 +- plugins/scaffolder-common/src/permissions.ts | 40 ++++++- template.yaml | 5 +- 11 files changed, 153 insertions(+), 178 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 385c9c2e0e..d886d86896 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -223,6 +223,7 @@ catalog: - System - Domain - Location + - Template processors: ldapOrg: @@ -279,6 +280,8 @@ catalog: # Backstage end-to-end tests of TechDocs - type: file target: ../../cypress/e2e-fixture.catalog.info.yaml + - type: file + target: ../../template.yaml scaffolder: # Use to customize default commit author info used when new components are created # defaultAuthor: diff --git a/packages/backend/src/plugins/permission.ts b/packages/backend/src/plugins/permission.ts index 708f342265..84a102d6be 100644 --- a/packages/backend/src/plugins/permission.ts +++ b/packages/backend/src/plugins/permission.ts @@ -18,7 +18,7 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { createRouter } from '@backstage/plugin-permission-backend'; import { AuthorizeResult, - isPermission, + isResourcePermission, PolicyDecision, } from '@backstage/plugin-permission-common'; import { @@ -27,10 +27,10 @@ import { } from '@backstage/plugin-permission-node'; import { - createScaffolderConditionalDecision, - scaffolderConditions, + createScaffolderStepConditionalDecision, + scaffolderStepConditions, } from '@backstage/plugin-scaffolder-backend'; -import { templateSchemaExecutePermission } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_STEP } from '@backstage/plugin-scaffolder-common'; import { Router } from 'express'; import { PluginEnvironment } from '../types'; @@ -39,12 +39,16 @@ class ExamplePermissionPolicy implements PermissionPolicy { request: PolicyQuery, _user?: BackstageIdentityResponse, ): Promise { - if (isPermission(request.permission, templateSchemaExecutePermission)) { - return createScaffolderConditionalDecision(request.permission, [ - scaffolderConditions.allowCapabilities({ - capabilities: ['example'], - }), - ]); + /** + * This is an example of how to use the scaffolder step conditions. + */ + if ( + isResourcePermission(request.permission, RESOURCE_TYPE_SCAFFOLDER_STEP) + ) { + return createScaffolderStepConditionalDecision( + request.permission, + scaffolderStepConditions.hasTag({ tag: 'example' }), + ); } return { diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 087de23198..e3f4b33707 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -126,7 +126,7 @@ export type MetadataResponse = { rules: MetadataResponseSerializedRule[]; }; -const applyConditions = ( +export const applyConditions = ( criteria: PermissionCriteria>, resource: TResource | undefined, getRule: (name: string) => PermissionRule, diff --git a/plugins/permission-node/src/integration/index.ts b/plugins/permission-node/src/integration/index.ts index 7702fea95b..f3b0edad9b 100644 --- a/plugins/permission-node/src/integration/index.ts +++ b/plugins/permission-node/src/integration/index.ts @@ -19,4 +19,9 @@ export * from './createConditionExports'; export * from './createConditionTransformer'; export * from './createPermissionIntegrationRouter'; export * from './createPermissionRule'; -export { isAndCriteria, isOrCriteria, isNotCriteria } from './util'; +export { + createGetRule, + isAndCriteria, + isOrCriteria, + isNotCriteria, +} from './util'; diff --git a/plugins/scaffolder-backend/src/service/conditionExports.ts b/plugins/scaffolder-backend/src/service/conditionExports.ts index d627284e76..2a7c0d382f 100644 --- a/plugins/scaffolder-backend/src/service/conditionExports.ts +++ b/plugins/scaffolder-backend/src/service/conditionExports.ts @@ -14,34 +14,42 @@ * limitations under the License. */ -import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common'; +import { RESOURCE_TYPE_SCAFFOLDER_STEP } from '@backstage/plugin-scaffolder-common'; import { createConditionExports } from '@backstage/plugin-permission-node'; -import { scaffolderRules } from './rules'; -import { - AllOfCriteria, - ConditionalPolicyDecision, - PermissionCondition, - PermissionRuleParams, - ResourcePermission, -} from '@backstage/plugin-permission-common'; +import { scaffolderStepRules } from './rules'; -const { conditions: scaffolderConditions, createConditionalDecision } = - createConditionExports({ - pluginId: 'scaffolder', - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - rules: scaffolderRules, - }); +// const { +// conditions: scaffolderTemplateConditions, +// createConditionalDecision: createScaffolderTemplateConditionalDecision, +// } = createConditionExports({ +// pluginId: 'scaffolder', +// resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, +// rules: scaffolderRules, +// }); -export { scaffolderConditions }; +// const { +// conditions: scaffolderPropertyConditions, +// createConditionalDecision: createScaffolderPropertyConditionalDecision, +// } = createConditionExports({ +// pluginId: 'scaffolder', +// resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, +// rules: scaffolderRules, +// }); -export function createScaffolderConditionalDecision( - permission: ResourcePermission, - conditions: AllOfCriteria< - PermissionCondition< - typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - PermissionRuleParams - > - >['allOf'], -): ConditionalPolicyDecision { - return createConditionalDecision(permission, { allOf: conditions }); -} +const { + conditions: scaffolderStepConditions, + createConditionalDecision: createScaffolderStepConditionalDecision, +} = createConditionExports({ + pluginId: 'scaffolder', + resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, + rules: scaffolderStepRules, +}); + +export { + // scaffolderTemplateConditions, + // createScaffolderTemplateConditionalDecision, + // scaffolderPropertyConditions, + // createScaffolderPropertyConditionalDecision, + scaffolderStepConditions, + createScaffolderStepConditionalDecision, +}; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 9fe7a98a8a..d5a9e17601 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -25,19 +25,14 @@ import { UserEntity, } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { - InputError, - NotAllowedError, - NotFoundError, - stringifyError, -} from '@backstage/errors'; +import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { JsonObject, JsonValue } from '@backstage/types'; import { TaskSpec, TemplateEntityV1beta3, templateEntityV1beta3Validator, - templateSchemaExecutePermission, + templateStepReadPermission, } from '@backstage/plugin-scaffolder-common'; import express from 'express'; import Router from 'express-promise-router'; @@ -54,12 +49,7 @@ import { } from '../scaffolder'; import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; -import { - findTemplate, - getEntityBaseUrl, - getWorkingDirectory, - TemplateTransform, -} from './helpers'; +import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; import { IdentityApi, IdentityApiGetIdentityRequest, @@ -70,11 +60,10 @@ import { PermissionEvaluator, } from '@backstage/plugin-permission-common'; import { - ConditionTransformer, - createConditionTransformer, - isAndCriteria, + applyConditions, + createGetRule, } from '@backstage/plugin-permission-node'; -import { scaffolderRules } from './rules'; +import { scaffolderStepRules } from './rules'; /** * RouterOptions @@ -273,8 +262,7 @@ export async function createRouter( additionalTemplateGlobals, }); - const transformConditions: ConditionTransformer = - createConditionTransformer(Object.values(scaffolderRules)); + const getRule = createGetRule(Object.values(scaffolderStepRules)); router .get( @@ -576,11 +564,12 @@ export async function createRouter( entityRef: CompoundEntityRef, token: string | undefined, ) { - let template = await findTemplate({ + const template = await findTemplate({ catalogApi: catalogClient, entityRef, token, }); + if (!isSupportedTemplate(template)) { throw new InputError( `Unsupported apiVersion field in schema entity, ${ @@ -588,32 +577,24 @@ export async function createRouter( }`, ); } + const authorizeDecision = ( await permissionApi.authorizeConditional( - [{ permission: templateSchemaExecutePermission }], - { - token, - }, + [{ permission: templateStepReadPermission }], + { token }, ) )[0]; if (authorizeDecision.result === AuthorizeResult.DENY) { - throw new NotAllowedError( - `Not allowed to execute template ${entityRef.kind}:${entityRef.namespace}/${entityRef.name}`, + template.spec.steps = []; + } else if (authorizeDecision.result === AuthorizeResult.CONDITIONAL) { + template.spec.steps = template.spec.steps.filter(step => + applyConditions(authorizeDecision.conditions, step, getRule), ); } - if (authorizeDecision.result === AuthorizeResult.CONDITIONAL) { - const scaffolderFilter = transformConditions( - authorizeDecision.conditions, - ); - if (isAndCriteria(scaffolderFilter)) { - template = scaffolderFilter.allOf.reduce( - (acc, filter) => (filter as TemplateTransform)(acc), - template, - ); - } - } + return template; } + return app; } diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index 11c938cab4..5166c86723 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -14,100 +14,34 @@ * limitations under the License. */ +import { EntitiesSearchFilter } from '@backstage/plugin-catalog-backend'; import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; import { - RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - TemplateEntityV1beta3, + RESOURCE_TYPE_SCAFFOLDER_STEP, + TemplateEntityStepV1beta3, } from '@backstage/plugin-scaffolder-common'; import { z } from 'zod'; -import get from 'lodash/get'; -import unset from 'lodash/unset'; -import { TemplateTransform } from './helpers'; -import { - SecureTemplater, - SecureTemplateRenderer, -} from '../lib/templating/SecureTemplater'; - -export const createScaffolderPermissionRule = makeCreatePermissionRule< - TemplateEntityV1beta3, - TemplateTransform, - typeof RESOURCE_TYPE_SCAFFOLDER_TEMPLATE +export const createScaffolderStepPermissionRule = makeCreatePermissionRule< + TemplateEntityStepV1beta3, + EntitiesSearchFilter, + typeof RESOURCE_TYPE_SCAFFOLDER_STEP >(); -let render: SecureTemplateRenderer; - -export const allowCapabilities = createScaffolderPermissionRule({ - name: 'ALLOW_CAPABILITIES', - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - description: 'Allow capabilities based on permissions', +const hasTag = createScaffolderStepPermissionRule({ + name: 'HAS_TAG', + resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, + description: 'Match a scaffolder step with the given tag', paramsSchema: z.object({ - capabilities: z.array(z.string()), - templateRef: z.string().optional(), + tag: z.string(), + }), + apply: (resource, { tag }) => { + return resource.metadata?.tags?.includes(tag) ?? false; + }, + toQuery: ({ tag }) => ({ + key: 'metadata.tags', + values: [tag], }), - apply: () => true, - toQuery: - ({ capabilities }) => - template => { - const capabilitiesObj = capabilities.reduce((acc, c) => { - acc[c] = true; - return acc; - }, {} as Record); - - const parametersPath = ['spec', 'parameters']; - const capabilitiesKey = 'backstage:capabilities'; - - if (Array.isArray(template.spec.parameters)) { - template.spec.parameters.forEach((parameter, index) => { - const parameterPath = [...parametersPath, index.toString()]; - stripParams(parameterPath); - if (parameter.properties) { - Object.keys(parameter.properties).forEach(p => { - if (stripParams([...parameterPath, 'properties', p])) { - const requiredProperties = get(template, [ - ...parameterPath, - 'required', - ]); - if (Array.isArray(requiredProperties)) { - requiredProperties.splice(requiredProperties.indexOf(p), 1); - } - } - }); - } - }); - } else { - stripParams(parametersPath); - } - - template.spec.steps.forEach((_p, index) => - stripParams(['spec', 'steps', index.toString()]), - ); - - function stripParams(path: string[]) { - const jsonObject = get(template, path); - - if ( - typeof jsonObject[capabilitiesKey] === 'string' && - render(jsonObject[capabilitiesKey], { - capabilities: capabilitiesObj, - }) !== 'true' - ) { - const parent = get(template, [...path].splice(0, path.length - 1)); - if (Array.isArray(parent)) { - parent.splice(parent.indexOf(jsonObject), 1); - } else { - unset(template, path); - } - - return path[path.length - 1]; - } - return undefined; - } - - return template; - }, }); -SecureTemplater.loadRenderer().then(r => (render = r)); - -export const scaffolderRules = { allowCapabilities }; +export const scaffolderStepRules = { hasTag }; diff --git a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts index aaba35911e..d83a1e3be5 100644 --- a/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts +++ b/plugins/scaffolder-common/src/TemplateEntityV1beta3.ts @@ -55,13 +55,7 @@ export interface TemplateEntityV1beta3 extends Entity { * A list of steps to be executed in sequence which are defined by the template. These steps are a list of the underlying * javascript action and some optional input parameters that may or may not have been collected from the end user. */ - steps: Array<{ - id?: string; - name?: string; - action: string; - input?: JsonObject; - if?: string | boolean; - }>; + steps: Array; /** * The output is an object where template authors can pull out information from template actions and return them in a known standard way. */ @@ -73,6 +67,20 @@ export interface TemplateEntityV1beta3 extends Entity { }; } +/** + * TODO + */ +export interface TemplateEntityStepV1beta3 extends JsonObject { + id?: string; + name?: string; + action: string; + input?: JsonObject; + if?: string | boolean; + metadata: { + tags?: string[]; + }; +} + const validator = entityKindSchemaValidator(schema); /** diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index 4953a27f9e..e4207c96fd 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -26,4 +26,7 @@ export { isTemplateEntityV1beta3, } from './TemplateEntityV1beta3'; export * from './permissions'; -export type { TemplateEntityV1beta3 } from './TemplateEntityV1beta3'; +export type { + TemplateEntityV1beta3, + TemplateEntityStepV1beta3, +} from './TemplateEntityV1beta3'; diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 578ef09311..bbf465c525 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -16,14 +16,42 @@ import { createPermission } from '@backstage/plugin-permission-common'; -export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-field'; +export const RESOURCE_TYPE_SCAFFOLDER_PARAMETER = 'scaffolder-parameter'; +export const RESOURCE_TYPE_SCAFFOLDER_PROPERTY = 'scaffolder-property'; +export const RESOURCE_TYPE_SCAFFOLDER_STEP = 'scaffolder-step'; -export const templateSchemaExecutePermission = createPermission({ - name: 'scaffolder.template.schema.execute', +export const templateParameterReadPermission = createPermission({ + name: 'scaffolder.template.parameter.read', attributes: { - action: 'create', + action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PARAMETER, }); -export const scaffolderPermissions = [templateSchemaExecutePermission]; +export const templatePropertyReadPermission = createPermission({ + name: 'scaffolder.template.property.read', + attributes: { + action: 'read', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_PROPERTY, +}); + +export const templateStepReadPermission = createPermission({ + name: 'scaffolder.template.step.read', + attributes: { + action: 'read', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_STEP, +}); + +export const scaffolderParameterPermissions = [templateParameterReadPermission]; +export const scaffolderPropertyPermissions = [templatePropertyReadPermission]; +export const scaffolderStepPermissions = [templateStepReadPermission]; + +/** + * TODOs: + * 1. Implement for Parameters & Properties + * 2. What metadata should be included in the template? + * 3. Write tests + * 4. Write documentation + */ diff --git a/template.yaml b/template.yaml index 0bb67735a9..36932eb4a6 100644 --- a/template.yaml +++ b/template.yaml @@ -14,7 +14,6 @@ spec: - owner properties: component_id: - backstage:capabilities: ${{ capabilities.cap1 }} title: Name type: string description: Unique name of the component @@ -44,11 +43,13 @@ spec: - github.com steps: - id: one - backstage:capabilities: ${{ capabilities.cap1 }} name: First log action: debug:log input: message: hello + metadata: + tags: + - example - id: two name: Second log action: debug:log