From 9ee832c5a940c97c2f43b9fa0a0cd65efc6527df Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Fri, 19 Apr 2024 16:48:12 -0400 Subject: [PATCH 01/24] feat(scaffolder): add additional scaffolder task permissions Signed-off-by: Frank Kong --- plugins/scaffolder-common/src/permissions.ts | 61 ++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 5c4c367889..bd548df950 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -30,6 +30,13 @@ export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; */ export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action'; +/** + * Permission resource type which corresponds to a scaffolder task. + * + * @alpha + */ +export const RESOURCE_TYPE_SCAFFOLDER_TASK = 'scaffolder-task'; + /** * This permission is used to authorize actions that involve executing * an action from a template. @@ -78,6 +85,50 @@ export const templateStepReadPermission = createPermission({ resourceType: RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, }); +/** + * This permission is used to authorize actions that involve reading one or more tasks in the scaffolder, + * and reading logs of tasks + * + * Task cancellation would also require this permission. + * + * @alpha + */ +export const taskReadPermission = createPermission({ + name: 'scaffolder.task.read', + attributes: { + action: 'read', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, +}); + +/** + * This permission is used to authorize actions that involve the creation of tasks in the scaffolder. + * + * @alpha + */ +export const taskCreatePermission = createPermission({ + name: 'scaffolder.task.create', + attributes: { + action: 'create', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, +}); + +/** + * This permission us used to authorize actions that involve the cancellation of tasks in the scaffolder. + * + * This will require the `scaffolder.task.read` permission to be authorized. + * + * @alpha + */ +export const taskCancelPermission = createPermission({ + name: 'scaffolder.task.cancel', + attributes: { + action: 'update', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, +}); + /** * List of all the scaffolder permissions * @alpha @@ -102,3 +153,13 @@ export const scaffolderTemplatePermissions = [ * @alpha */ export const scaffolderActionPermissions = [actionExecutePermission]; + +/** + * List of the scaffolder permissions that are associated with scaffolder tasks. + * @alpha + */ +export const scaffolderTaskPermissions = [ + taskCancelPermission, + taskCreatePermission, + taskReadPermission, +]; From a7b500bd0829bb2a449dd488377385a76b73aeb4 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Mon, 22 Apr 2024 17:15:22 -0400 Subject: [PATCH 02/24] feat(scaffolder): added permissions to backend endpoints Signed-off-by: Frank Kong --- .../scaffolder-backend/src/service/router.ts | 177 +++++++++++++++++- plugins/scaffolder-common/src/permissions.ts | 21 ++- 2 files changed, 184 insertions(+), 14 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 43618ed5a7..7b39290cfc 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -30,7 +30,12 @@ import { UserEntity, } from '@backstage/catalog-model'; import { Config, readDurationFromConfig } from '@backstage/config'; -import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; +import { + InputError, + NotAllowedError, + NotFoundError, + stringifyError, +} from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { HumanDuration, JsonObject, JsonValue } from '@backstage/types'; import { @@ -43,10 +48,16 @@ import { import { RESOURCE_TYPE_SCAFFOLDER_ACTION, RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + RESOURCE_TYPE_SCAFFOLDER_TASK, scaffolderActionPermissions, scaffolderTemplatePermissions, + taskCancelPermission, + taskCreatePermission, + taskReadPermission, templateParameterReadPermission, templateStepReadPermission, + scaffolderTaskPermissions, + actionReadPermission, } from '@backstage/plugin-scaffolder-common/alpha'; import express from 'express'; import Router from 'express-promise-router'; @@ -68,7 +79,10 @@ import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; import { TemplateAction } from '@backstage/plugin-scaffolder-node'; -import { PermissionRuleParams } from '@backstage/plugin-permission-common'; +import { + AuthorizeResult, + PermissionRuleParams, +} from '@backstage/plugin-permission-common'; import { createConditionAuthorizer, createPermissionIntegrationRouter, @@ -90,6 +104,11 @@ import { } from '@backstage/plugin-auth-node'; import { InternalTaskSecrets } from '../scaffolder/tasks/types'; +type ScaffolderPermissionRuleInput = + | TemplatePermissionRuleInput + | ActionPermissionRuleInput + | TaskPermissionRuleInput; + /** * * @public @@ -103,7 +122,7 @@ export type TemplatePermissionRuleInput< TParams >; function isTemplatePermissionRuleInput( - permissionRule: TemplatePermissionRuleInput | ActionPermissionRuleInput, + permissionRule: ScaffolderPermissionRuleInput, ): permissionRule is TemplatePermissionRuleInput { return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_TEMPLATE; } @@ -121,11 +140,28 @@ export type ActionPermissionRuleInput< TParams >; function isActionPermissionRuleInput( - permissionRule: TemplatePermissionRuleInput | ActionPermissionRuleInput, + permissionRule: ScaffolderPermissionRuleInput, ): permissionRule is ActionPermissionRuleInput { return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_ACTION; } +/** + * + * @public + */ +export type TaskPermissionRuleInput< + TParams extends PermissionRuleParams = PermissionRuleParams, +> = PermissionRule< + TemplateEntityStepV1beta3 | TemplateParametersV1beta3, + {}, + typeof RESOURCE_TYPE_SCAFFOLDER_TASK, + TParams +>; +function isTaskPermissionRuleInput( + permissionRule: ScaffolderPermissionRuleInput, +): permissionRule is TaskPermissionRuleInput { + return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_TASK; +} /** * RouterOptions * @@ -154,9 +190,7 @@ export interface RouterOptions { additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; permissions?: PermissionsService; - permissionRules?: Array< - TemplatePermissionRuleInput | ActionPermissionRuleInput - >; + permissionRules?: Array; auth?: AuthService; httpAuth?: HttpAuthService; identity?: IdentityApi; @@ -384,12 +418,14 @@ export async function createRouter( const actionRules: ActionPermissionRuleInput[] = Object.values( scaffolderActionRules, ); + const taskRules: TaskPermissionRuleInput[] = []; if (permissionRules) { templateRules.push( ...permissionRules.filter(isTemplatePermissionRuleInput), ); actionRules.push(...permissionRules.filter(isActionPermissionRuleInput)); + taskRules.push(...permissionRules.filter(isTaskPermissionRuleInput)); } const isAuthorized = createConditionAuthorizer(Object.values(templateRules)); @@ -406,6 +442,11 @@ export async function createRouter( permissions: scaffolderActionPermissions, rules: actionRules, }, + { + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, + permissions: scaffolderTaskPermissions, + rules: taskRules, + }, ], }); @@ -445,7 +486,21 @@ export async function createRouter( }); }, ) - .get('/v2/actions', async (_req, res) => { + .get('/v2/actions', async (req, res) => { + const credentials = await httpAuth.credentials(req); + + if (permissions) { + const authorizationResponse = ( + await permissions?.authorizeConditional( + [{ permission: actionReadPermission }], + { credentials: credentials }, + ) + )[0]; + + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } const actionsList = actionRegistry.list().map(action => { return { id: action.id, @@ -463,6 +518,19 @@ export async function createRouter( }); const credentials = await httpAuth.credentials(req); + if (permissions) { + const authorizationResponse = ( + await permissions?.authorizeConditional( + [{ permission: taskCreatePermission }], + { credentials: credentials }, + ) + )[0]; + + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + const { token } = await auth.getPluginRequestToken({ onBehalfOf: credentials, targetPluginId: 'catalog', @@ -539,6 +607,21 @@ export async function createRouter( res.status(201).json({ id: result.taskId }); }) .get('/v2/tasks', async (req, res) => { + const credentials = await httpAuth.credentials(req); + + if (permissions) { + const authorizationResponse = ( + await permissions?.authorizeConditional( + [{ permission: taskReadPermission }], + { credentials: credentials }, + ) + )[0]; + + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + const [userEntityRef] = [req.query.createdBy].flat(); if ( @@ -561,6 +644,21 @@ export async function createRouter( res.status(200).json(tasks); }) .get('/v2/tasks/:taskId', async (req, res) => { + const credentials = await httpAuth.credentials(req); + + if (permissions) { + const authorizationResponse = ( + await permissions?.authorizeConditional( + [{ permission: taskReadPermission }], + { credentials: credentials }, + ) + )[0]; + + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + const { taskId } = req.params; const task = await taskBroker.get(taskId); if (!task) { @@ -571,11 +669,43 @@ export async function createRouter( res.status(200).json(task); }) .post('/v2/tasks/:taskId/cancel', async (req, res) => { + const credentials = await httpAuth.credentials(req); + + if (permissions) { + const authorizationResponses = await permissions?.authorizeConditional( + [ + { permission: taskCancelPermission }, + { permission: taskReadPermission }, + ], + { credentials: credentials }, + ); + // Requires both read and cancel permissions + for (const response of authorizationResponses) { + if (response.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + } + const { taskId } = req.params; await taskBroker.cancel?.(taskId); res.status(200).json({ status: 'cancelled' }); }) .get('/v2/tasks/:taskId/eventstream', async (req, res) => { + const credentials = await httpAuth.credentials(req); + + if (permissions) { + const authorizationResponse = ( + await permissions?.authorizeConditional( + [{ permission: taskReadPermission }], + { credentials: credentials }, + ) + )[0]; + + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } const { taskId } = req.params; const after = req.query.after !== undefined ? Number(req.query.after) : undefined; @@ -624,6 +754,20 @@ export async function createRouter( }); }) .get('/v2/tasks/:taskId/events', async (req, res) => { + const credentials = await httpAuth.credentials(req); + + if (permissions) { + const authorizationResponse = ( + await permissions?.authorizeConditional( + [{ permission: taskReadPermission }], + { credentials: credentials }, + ) + )[0]; + + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } const { taskId } = req.params; const after = Number(req.query.after) || undefined; @@ -654,6 +798,21 @@ export async function createRouter( }); }) .post('/v2/dry-run', async (req, res) => { + const credentials = await httpAuth.credentials(req); + + if (permissions) { + const authorizationResponse = ( + await permissions?.authorizeConditional( + [{ permission: taskCreatePermission }], + { credentials: credentials }, + ) + )[0]; + + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + const bodySchema = z.object({ template: z.unknown(), values: z.record(z.unknown()), @@ -671,8 +830,6 @@ export async function createRouter( throw new InputError('Input template is not a template'); } - const credentials = await httpAuth.credentials(req); - const { token } = await auth.getPluginRequestToken({ onBehalfOf: credentials, targetPluginId: 'catalog', diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index bd548df950..4a4137875a 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -49,6 +49,18 @@ export const actionExecutePermission = createPermission({ resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, }); +/** + * This permission is used to authorize actions that involve access the action registry + * + * @alpha + */ +export const actionReadPermission = createPermission({ + name: 'scaffolder.action.read', + attributes: { + action: 'read', + }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, +}); /** * This permission is used to authorize actions that involve reading * one or more parameters from a template. @@ -123,9 +135,7 @@ export const taskCreatePermission = createPermission({ */ export const taskCancelPermission = createPermission({ name: 'scaffolder.task.cancel', - attributes: { - action: 'update', - }, + attributes: {}, resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, }); @@ -152,7 +162,10 @@ export const scaffolderTemplatePermissions = [ * List of the scaffolder permissions that are associated with scaffolder actions. * @alpha */ -export const scaffolderActionPermissions = [actionExecutePermission]; +export const scaffolderActionPermissions = [ + actionExecutePermission, + actionReadPermission, +]; /** * List of the scaffolder permissions that are associated with scaffolder tasks. From b7fcaca26bbddf7d8b86214f4634526f33dac711 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 25 Apr 2024 16:42:23 -0400 Subject: [PATCH 03/24] feat(scaffolder): update scaffolder frontend to support additional backend permissions Signed-off-by: Frank Kong --- .../components/AboutCard/AboutCard.test.tsx | 61 ++++++++++++++- .../src/components/AboutCard/AboutCard.tsx | 11 ++- .../scaffolder-backend/src/service/router.ts | 18 ++--- plugins/scaffolder-react/package.json | 5 +- .../TemplateCard/TemplateCard.test.tsx | 57 ++++++++++++++ .../components/TemplateCard/TemplateCard.tsx | 25 ++++-- plugins/scaffolder/dev/index.tsx | 3 +- .../components/ActionsPage/ActionsPage.tsx | 47 ++++++----- .../ListTasksPage/ListTasksPage.tsx | 2 +- .../components/OngoingTask/ContextMenu.tsx | 40 +++++++++- .../OngoingTask/OngoingTask.test.tsx | 77 ++++++++++++------- .../components/OngoingTask/OngoingTask.tsx | 37 ++++++++- 12 files changed, 309 insertions(+), 74 deletions(-) diff --git a/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx b/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx index f90942dcd8..76d41399ba 100644 --- a/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx +++ b/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx @@ -35,6 +35,7 @@ import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { permissionApiRef } from '@backstage/plugin-permission-react'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { SWRConfig } from 'swr'; const mockAuthorize = jest.fn(); @@ -546,7 +547,7 @@ describe('', () => { ).not.toBeInTheDocument(); }); - it('renders techdocs lin when 3rdparty', async () => { + it('renders techdocs link when 3rdparty', async () => { const entity = { apiVersion: 'v1', kind: 'Component', @@ -774,7 +775,9 @@ describe('', () => { namespace: 'default', }, }; - + mockAuthorize.mockImplementation(async () => ({ + result: AuthorizeResult.ALLOW, + })); await renderInTestApp( ', () => { ), ], [catalogApiRef, catalogApi], - [permissionApiRef, {}], + [permissionApiRef, mockPermissionApi], ]} > @@ -816,6 +819,58 @@ describe('', () => { '/create/templates/default/create-react-app-template', ); }); + it('renders disabled launch template button if user has insufficient permissions', async () => { + const entity = { + apiVersion: 'scaffolder.backstage.io/v1beta3', + kind: 'Template', + metadata: { + name: 'create-react-app-template', + namespace: 'default', + }, + }; + mockAuthorize.mockImplementation(async () => ({ + result: AuthorizeResult.DENY, + })); + const rendered = await renderInTestApp( + new Map() }}> + + + + + + , + { + mountedRoutes: { + '/catalog/:namespace/:kind/:name': entityRouteRef, + '/create/templates/:namespace/:templateName': + createFromTemplateRouteRef, + }, + }, + ); + + expect(screen.getByText('Launch Template')).toBeVisible(); + expect(screen.getByText('Launch Template').closest('a')).toBeNull(); + }); it.each([ { diff --git a/plugins/catalog/src/components/AboutCard/AboutCard.tsx b/plugins/catalog/src/components/AboutCard/AboutCard.tsx index a24a132bc4..b63666f80c 100644 --- a/plugins/catalog/src/components/AboutCard/AboutCard.tsx +++ b/plugins/catalog/src/components/AboutCard/AboutCard.tsx @@ -19,6 +19,7 @@ import { CompoundEntityRef, DEFAULT_NAMESPACE, stringifyEntityRef, + parseEntityRef, } from '@backstage/catalog-model'; import Card from '@material-ui/core/Card'; import CardContent from '@material-ui/core/CardContent'; @@ -58,10 +59,11 @@ import CreateComponentIcon from '@material-ui/icons/AddCircleOutline'; import DocsIcon from '@material-ui/icons/Description'; import EditIcon from '@material-ui/icons/Edit'; import { isTemplateEntityV1beta3 } from '@backstage/plugin-scaffolder-common'; -import { parseEntityRef } from '@backstage/catalog-model'; import { useEntityPermission } from '@backstage/plugin-catalog-react/alpha'; import { catalogEntityRefreshPermission } from '@backstage/plugin-catalog-common/alpha'; import { useSourceTemplateCompoundEntityRef } from './hooks'; +import { taskCreatePermission } from '@backstage/plugin-scaffolder-common/alpha'; +import { usePermission } from '@backstage/plugin-permission-react'; const TECHDOCS_ANNOTATION = 'backstage.io/techdocs-ref'; @@ -114,6 +116,11 @@ export function AboutCard(props: AboutCardProps) { const { allowed: canRefresh } = useEntityPermission( catalogEntityRefreshPermission, ); + const { kind, name, namespace } = entity.metadata; + const { allowed: canCreateTemplateTask } = usePermission({ + permission: taskCreatePermission, + resourceRef: `${kind}:${namespace}/${name}`, + }); const entitySourceLocation = getEntitySourceLocation( entity, @@ -172,7 +179,7 @@ export function AboutCard(props: AboutCardProps) { const launchTemplate: IconLinkVerticalProps = { label: 'Launch Template', icon: , - disabled: !templateRoute, + disabled: !templateRoute || !canCreateTemplateTask, href: templateRoute && templateRoute({ diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 7b39290cfc..3fd1e14fa1 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -65,6 +65,7 @@ import { validate } from 'jsonschema'; import { Logger } from 'winston'; import { z } from 'zod'; import { + TemplateAction, TaskBroker, TemplateFilter, TemplateGlobal, @@ -78,7 +79,6 @@ import { import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; -import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { AuthorizeResult, PermissionRuleParams, @@ -491,7 +491,7 @@ export async function createRouter( if (permissions) { const authorizationResponse = ( - await permissions?.authorizeConditional( + await permissions.authorizeConditional( [{ permission: actionReadPermission }], { credentials: credentials }, ) @@ -520,7 +520,7 @@ export async function createRouter( const credentials = await httpAuth.credentials(req); if (permissions) { const authorizationResponse = ( - await permissions?.authorizeConditional( + await permissions.authorizeConditional( [{ permission: taskCreatePermission }], { credentials: credentials }, ) @@ -611,7 +611,7 @@ export async function createRouter( if (permissions) { const authorizationResponse = ( - await permissions?.authorizeConditional( + await permissions.authorizeConditional( [{ permission: taskReadPermission }], { credentials: credentials }, ) @@ -648,7 +648,7 @@ export async function createRouter( if (permissions) { const authorizationResponse = ( - await permissions?.authorizeConditional( + await permissions.authorizeConditional( [{ permission: taskReadPermission }], { credentials: credentials }, ) @@ -672,7 +672,7 @@ export async function createRouter( const credentials = await httpAuth.credentials(req); if (permissions) { - const authorizationResponses = await permissions?.authorizeConditional( + const authorizationResponses = await permissions.authorizeConditional( [ { permission: taskCancelPermission }, { permission: taskReadPermission }, @@ -696,7 +696,7 @@ export async function createRouter( if (permissions) { const authorizationResponse = ( - await permissions?.authorizeConditional( + await permissions.authorizeConditional( [{ permission: taskReadPermission }], { credentials: credentials }, ) @@ -758,7 +758,7 @@ export async function createRouter( if (permissions) { const authorizationResponse = ( - await permissions?.authorizeConditional( + await permissions.authorizeConditional( [{ permission: taskReadPermission }], { credentials: credentials }, ) @@ -802,7 +802,7 @@ export async function createRouter( if (permissions) { const authorizationResponse = ( - await permissions?.authorizeConditional( + await permissions.authorizeConditional( [{ permission: taskCreatePermission }], { credentials: credentials }, ) diff --git a/plugins/scaffolder-react/package.json b/plugins/scaffolder-react/package.json index ad12268baa..0b803f35e4 100644 --- a/plugins/scaffolder-react/package.json +++ b/plugins/scaffolder-react/package.json @@ -54,6 +54,7 @@ "@backstage/core-components": "workspace:^", "@backstage/core-plugin-api": "workspace:^", "@backstage/plugin-catalog-react": "workspace:^", + "@backstage/plugin-permission-react": "workspace:^", "@backstage/plugin-scaffolder-common": "workspace:^", "@backstage/theme": "workspace:^", "@backstage/types": "workspace:^", @@ -87,13 +88,15 @@ "@backstage/core-app-api": "workspace:^", "@backstage/plugin-catalog": "workspace:^", "@backstage/plugin-catalog-common": "workspace:^", + "@backstage/plugin-permission-common": "workspace:^", "@backstage/test-utils": "workspace:^", "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^6.0.0", "@testing-library/react": "^15.0.0", "@testing-library/user-event": "^14.0.0", "@types/humanize-duration": "^3.18.1", - "@types/luxon": "^3.0.0" + "@types/luxon": "^3.0.0", + "swr": "^2.0.0" }, "peerDependencies": { "react": "^16.13.1 || ^17.0.0 || ^18.0.0", diff --git a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx index ec4e4e1b0a..90ae1e097c 100644 --- a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx +++ b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.test.tsx @@ -19,6 +19,7 @@ import { starredEntitiesApiRef, } from '@backstage/plugin-catalog-react'; import { + MockPermissionApi, MockStorageApi, renderInTestApp, TestApiProvider, @@ -28,6 +29,12 @@ import React from 'react'; import { TemplateEntityV1beta3 } from '@backstage/plugin-scaffolder-common'; import { RELATION_OWNED_BY } from '@backstage/catalog-model'; import { fireEvent } from '@testing-library/react'; +import { + PermissionApi, + permissionApiRef, +} from '@backstage/plugin-permission-react'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { SWRConfig } from 'swr'; describe('TemplateCard', () => { it('should render the card title', async () => { @@ -50,6 +57,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -79,6 +87,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -110,6 +119,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -139,6 +149,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -174,6 +185,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -213,6 +225,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -257,6 +270,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -305,6 +319,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -347,6 +362,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -386,6 +402,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -403,4 +420,44 @@ describe('TemplateCard', () => { expect(mockOnSelected).toHaveBeenCalledWith(mockTemplate); }); + it('should not render the choose button when user has insufficient permissions', async () => { + const mockTemplate: TemplateEntityV1beta3 = { + apiVersion: 'scaffolder.backstage.io/v1beta3', + kind: 'Template', + metadata: { name: 'bob', tags: ['cpp', 'react'] }, + spec: { + steps: [], + type: 'service', + }, + }; + const mockOnSelected = jest.fn(); + const mockAuthorize = jest + .fn() + .mockImplementation(async () => ({ result: AuthorizeResult.DENY })); + // SWR used by the usePermission hook needs cache to be reset for each test + const { queryByText } = await renderInTestApp( + new Map() }}> + + + + , + { + mountedRoutes: { + '/catalog/:kind/:namespace/:name': entityRouteRef, + }, + }, + ); + + expect(queryByText('Choose')).toBeNull(); + }); }); diff --git a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx index 4fb2ca0e4a..23e2f13e78 100644 --- a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx +++ b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx @@ -35,6 +35,8 @@ import LanguageIcon from '@material-ui/icons/Language'; import React from 'react'; import { CardHeader } from './CardHeader'; import { CardLink } from './CardLink'; +import { usePermission } from '@backstage/plugin-permission-react'; +import { taskCreatePermission } from '@backstage/plugin-scaffolder-common/alpha'; const useStyles = makeStyles(theme => ({ box: { @@ -103,6 +105,11 @@ export const TemplateCard = (props: TemplateCardProps) => { !!props.additionalLinks?.length || !!template.metadata.links?.length; const displayDefaultDivider = !hasTags && !hasLinks; + const { allowed: canCreateTask } = usePermission({ + permission: taskCreatePermission, + resourceRef: 'task', + }); + return ( @@ -186,14 +193,16 @@ export const TemplateCard = (props: TemplateCardProps) => { )} - + {canCreateTask ? ( + + ) : null} diff --git a/plugins/scaffolder/dev/index.tsx b/plugins/scaffolder/dev/index.tsx index e4425b04bf..ebc0c3596f 100644 --- a/plugins/scaffolder/dev/index.tsx +++ b/plugins/scaffolder/dev/index.tsx @@ -23,7 +23,8 @@ import { MockStarredEntitiesApi, } from '@backstage/plugin-catalog-react'; import React from 'react'; -import { scaffolderApiRef, ScaffolderClient } from '../src'; +import { ScaffolderClient } from '../src'; +import { scaffolderApiRef } from '@backstage/plugin-scaffolder-react'; import { ScaffolderPage } from '../src/plugin'; import { discoveryApiRef, diff --git a/plugins/scaffolder/src/components/ActionsPage/ActionsPage.tsx b/plugins/scaffolder/src/components/ActionsPage/ActionsPage.tsx index d6eabb0e5c..7e859e272d 100644 --- a/plugins/scaffolder/src/components/ActionsPage/ActionsPage.tsx +++ b/plugins/scaffolder/src/components/ActionsPage/ActionsPage.tsx @@ -43,7 +43,8 @@ import { useApi, useRouteRef } from '@backstage/core-plugin-api'; import { CodeSnippet, Content, - ErrorPage, + EmptyState, + ErrorPanel, Header, MarkdownContent, Page, @@ -112,19 +113,9 @@ const ExamplesTable = (props: { examples: ActionExample[] }) => { ); }; -export const ActionsPage = () => { +const ActionPageContent = () => { const api = useApi(scaffolderApiRef); - const navigate = useNavigate(); - const editorLink = useRouteRef(editRouteRef); - const tasksLink = useRouteRef(scaffolderListTaskRouteRef); - const createLink = useRouteRef(rootRouteRef); - const scaffolderPageContextMenuProps = { - onEditorClicked: () => navigate(editorLink()), - onActionsClicked: undefined, - onTasksClicked: () => navigate(tasksLink()), - onCreateClicked: () => navigate(createLink()), - }; const classes = useStyles(); const { loading, value, error } = useAsync(async () => { return api.listActions(); @@ -137,11 +128,14 @@ export const ActionsPage = () => { if (error) { return ( - + <> + + + ); } @@ -282,7 +276,7 @@ export const ActionsPage = () => { ); }; - const items = value?.map(action => { + return value?.map(action => { if (action.id.startsWith('legacy:')) { return undefined; } @@ -336,6 +330,19 @@ export const ActionsPage = () => { ); }); +}; +export const ActionsPage = () => { + const navigate = useNavigate(); + const editorLink = useRouteRef(editRouteRef); + const tasksLink = useRouteRef(scaffolderListTaskRouteRef); + const createLink = useRouteRef(rootRouteRef); + + const scaffolderPageContextMenuProps = { + onEditorClicked: () => navigate(editorLink()), + onActionsClicked: undefined, + onTasksClicked: () => navigate(tasksLink()), + onCreateClicked: () => navigate(createLink()), + }; return ( @@ -346,7 +353,9 @@ export const ActionsPage = () => { > - {items} + + + ); }; diff --git a/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx b/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx index 4c2eb04c79..d37716a710 100644 --- a/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx +++ b/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx @@ -77,7 +77,7 @@ const ListTaskPageContent = (props: MyTaskPageProps) => { ); diff --git a/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx b/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx index 99d9235791..bac23da67d 100644 --- a/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx @@ -30,6 +30,12 @@ import MoreVert from '@material-ui/icons/MoreVert'; import React, { useState } from 'react'; import { useApi } from '@backstage/core-plugin-api'; import { scaffolderApiRef } from '@backstage/plugin-scaffolder-react'; +import { usePermission } from '@backstage/plugin-permission-react'; +import { + taskCancelPermission, + taskReadPermission, + taskCreatePermission, +} from '@backstage/plugin-scaffolder-common/alpha'; type ContextMenuProps = { cancelEnabled?: boolean; @@ -69,6 +75,28 @@ export const ContextMenu = (props: ContextMenuProps) => { } }); + // Used dummy string value for `resourceRef` since `allowed` field will always return `false` if `resourceRef` is `undefined` + const { allowed: canCancelTask } = usePermission({ + permission: taskCancelPermission, + resourceRef: 'task', + }); + + const { allowed: canReadTask } = usePermission({ + permission: taskReadPermission, + resourceRef: 'task', + }); + + const { allowed: canCreateTask } = usePermission({ + permission: taskCreatePermission, + resourceRef: 'task', + }); + + // Cancel endpoint requires user to have both read and cancel permissions + const cancelNotAllowed = !(canReadTask && canCancelTask); + + // Start Over endpoint requires user to have both read (to grab parameters) and create (to create new task) permissions + const canStartOver = canReadTask && canCreateTask; + return ( <> { primary={buttonBarVisible ? 'Hide Button Bar' : 'Show Button Bar'} /> - + @@ -113,7 +145,11 @@ export const ContextMenu = (props: ContextMenuProps) => { diff --git a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx index 9d3f29fe7b..70d2193470 100644 --- a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx @@ -16,10 +16,20 @@ import { OngoingTask } from './OngoingTask'; import React from 'react'; -import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; +import { + renderInTestApp, + TestApiProvider, + MockPermissionApi, +} from '@backstage/test-utils'; import { scaffolderApiRef } from '@backstage/plugin-scaffolder-react'; import { act, fireEvent, waitFor, within } from '@testing-library/react'; +import { + PermissionApi, + permissionApiRef, +} from '@backstage/plugin-permission-react'; import { rootRouteRef } from '../../routes'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { SWRConfig } from 'swr'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -49,18 +59,29 @@ describe('OngoingTask', () => { getTask: jest.fn().mockImplementation(async () => {}), }; - beforeEach(() => { + beforeEach(async () => { jest.clearAllMocks(); }); - it('should trigger cancel api on "Cancel" click in context menu', async () => { - const cancelOptionLabel = 'Cancel'; - const rendered = await renderInTestApp( - - - , + const render = (permissionApi?: PermissionApi) => { + // SWR used by the usePermission hook needs cache to be reset for each test + return renderInTestApp( + new Map() }}> + + + + , { mountedRoutes: { '/': rootRouteRef } }, ); + }; + it('should trigger cancel api on "Cancel" click in context menu', async () => { + const rendered = await render(); + const cancelOptionLabel = 'Cancel'; const { getByTestId } = rendered; await act(async () => { @@ -84,13 +105,9 @@ describe('OngoingTask', () => { }); it('should trigger cancel api on "Cancel" button click', async () => { + const rendered = await render(); const cancelOptionLabel = 'Cancel'; - const rendered = await renderInTestApp( - - - , - { mountedRoutes: { '/': rootRouteRef } }, - ); + const { getByTestId } = rendered; await act(async () => { @@ -114,22 +131,12 @@ describe('OngoingTask', () => { }); it('should initially do not display logs', async () => { - const rendered = await renderInTestApp( - - - , - { mountedRoutes: { '/': rootRouteRef } }, - ); + const rendered = await render(); await expect(rendered.findByText('Show Logs')).resolves.toBeInTheDocument(); }); it('should toggle logs visibility', async () => { - const rendered = await renderInTestApp( - - - , - { mountedRoutes: { '/': rootRouteRef } }, - ); + const rendered = await render(); await act(async () => { const element = await rendered.findByText('Show Logs'); fireEvent.click(element); @@ -137,4 +144,22 @@ describe('OngoingTask', () => { await expect(rendered.findByText('Hide Logs')).resolves.toBeInTheDocument(); }); + + it('should have cancel and start over buttons be disabled without the proper permissions', async () => { + const mockAuthorize = jest + .fn() + .mockImplementation(async () => ({ result: AuthorizeResult.DENY })); + const permissionApi: PermissionApi = { authorize: mockAuthorize }; + const rendered = await render(permissionApi); + + const { getByTestId } = rendered; + expect(getByTestId('cancel-button')).toHaveClass('Mui-disabled'); + expect(getByTestId('start-over-button')).toHaveClass('Mui-disabled'); + + await act(async () => { + fireEvent.click(getByTestId('menu-button')); + }); + expect(getByTestId('cancel-task')).toHaveClass('Mui-disabled'); + expect(getByTestId('start-over-task')).toHaveClass('Mui-disabled'); + }); }); diff --git a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx index 3e7a3b8f52..d5f6dcfb0d 100644 --- a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx @@ -35,6 +35,12 @@ import { TaskSteps, } from '@backstage/plugin-scaffolder-react/alpha'; import { useAsync } from '@react-hookz/web'; +import { usePermission } from '@backstage/plugin-permission-react'; +import { + taskCancelPermission, + taskReadPermission, + taskCreatePermission, +} from '@backstage/plugin-scaffolder-common/alpha'; const useStyles = makeStyles(theme => ({ contentWrapper: { @@ -81,6 +87,28 @@ export const OngoingTask = (props: { const [logsVisible, setLogVisibleState] = useState(false); const [buttonBarVisible, setButtonBarVisibleState] = useState(true); + // Used dummy string value for `resourceRef` since `allowed` field will always return `false` if `resourceRef` is `undefined` + const { allowed: canCancelTask } = usePermission({ + permission: taskCancelPermission, + resourceRef: 'task', + }); + + const { allowed: canReadTask } = usePermission({ + permission: taskReadPermission, + resourceRef: 'task', + }); + + const { allowed: canCreateTask } = usePermission({ + permission: taskCreatePermission, + resourceRef: 'task', + }); + + // Cancel endpoint requires user to have both read and cancel permissions + const cancelNotAllowed = !(canReadTask && canCancelTask); + + // Start Over endpoint requires user to have both read (to grab parameters) and create (to create new task) permissions + const canStartOver = canReadTask && canCreateTask; + useEffect(() => { if (taskStream.error) { setLogVisibleState(true); @@ -192,7 +220,11 @@ export const OngoingTask = (props: {
From fa26d03a6aab6fed41dc1b08cff88397e242b31e Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 25 Apr 2024 16:43:34 -0400 Subject: [PATCH 04/24] chore: update package.json Signed-off-by: Frank Kong --- plugins/catalog/package.json | 3 ++- plugins/scaffolder/package.json | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/catalog/package.json b/plugins/catalog/package.json index db2089a89e..bc4e125b97 100644 --- a/plugins/catalog/package.json +++ b/plugins/catalog/package.json @@ -87,7 +87,8 @@ "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^6.0.0", "@testing-library/react": "^15.0.0", - "@testing-library/user-event": "^14.0.0" + "@testing-library/user-event": "^14.0.0", + "swr": "^2.0.0" }, "peerDependencies": { "react": "^16.13.1 || ^17.0.0 || ^18.0.0", diff --git a/plugins/scaffolder/package.json b/plugins/scaffolder/package.json index 32dc701a1e..5c0773bd65 100644 --- a/plugins/scaffolder/package.json +++ b/plugins/scaffolder/package.json @@ -98,6 +98,7 @@ "@backstage/core-app-api": "workspace:^", "@backstage/dev-utils": "workspace:^", "@backstage/plugin-catalog": "workspace:^", + "@backstage/plugin-permission-common": "workspace:^", "@backstage/test-utils": "workspace:^", "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^6.0.0", @@ -105,7 +106,8 @@ "@testing-library/user-event": "^14.0.0", "@types/humanize-duration": "^3.18.1", "@types/json-schema": "^7.0.9", - "msw": "^1.0.0" + "msw": "^1.0.0", + "swr": "^2.0.0" }, "peerDependencies": { "react": "^16.13.1 || ^17.0.0 || ^18.0.0", From bcec60fb4a46137be4ab7ecc3d07170b69c5eb5f Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 25 Apr 2024 17:18:47 -0400 Subject: [PATCH 05/24] chore: add changeset Signed-off-by: Frank Kong --- .changeset/tender-seas-listen.md | 12 ++++++++++++ .changeset/weak-gifts-occur.md | 11 +++++++++++ 2 files changed, 23 insertions(+) create mode 100644 .changeset/tender-seas-listen.md create mode 100644 .changeset/weak-gifts-occur.md diff --git a/.changeset/tender-seas-listen.md b/.changeset/tender-seas-listen.md new file mode 100644 index 0000000000..cfcea07574 --- /dev/null +++ b/.changeset/tender-seas-listen.md @@ -0,0 +1,12 @@ +--- +'@backstage/plugin-scaffolder-react': patch +'@backstage/plugin-scaffolder': patch +'@backstage/plugin-catalog': patch +--- + +updated the ContextMenu, ActionsPage, OngoingTask and TemplateCard frontend components to support the new scaffolder permissions: + +- `scaffolder.task.create` +- `scaffolder.task.cancel` +- `scaffolder.task.read` +- `scaffolder.action.read` diff --git a/.changeset/weak-gifts-occur.md b/.changeset/weak-gifts-occur.md new file mode 100644 index 0000000000..c1a65e7dff --- /dev/null +++ b/.changeset/weak-gifts-occur.md @@ -0,0 +1,11 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +'@backstage/plugin-scaffolder-common': patch +--- + +added the following new permissions to the scaffolder backend endpoints: + +- `scaffolder.task.create` +- `scaffolder.task.cancel` +- `scaffolder.task.read` +- `scaffolder.action.read` From 959ed3afb27396b9cf532e708d22bfc7a7113a3a Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Fri, 26 Apr 2024 09:07:04 -0400 Subject: [PATCH 06/24] chore: fix tsc Signed-off-by: Frank Kong --- plugins/catalog/src/components/AboutCard/AboutCard.test.tsx | 2 +- .../src/next/components/TemplateCard/TemplateCard.test.tsx | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx b/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx index 76d41399ba..fcea659fc6 100644 --- a/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx +++ b/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx @@ -831,7 +831,7 @@ describe('', () => { mockAuthorize.mockImplementation(async () => ({ result: AuthorizeResult.DENY, })); - const rendered = await renderInTestApp( + await renderInTestApp( new Map() }}> Date: Fri, 26 Apr 2024 09:30:26 -0400 Subject: [PATCH 07/24] chore: update api-reports Signed-off-by: Frank Kong --- plugins/scaffolder-backend/api-report.md | 17 ++++++++++++++--- plugins/scaffolder-common/api-report-alpha.md | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 6af1ac04a6..dc7917e825 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -36,6 +36,7 @@ import { PermissionsService } from '@backstage/backend-plugin-api'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; import { RESOURCE_TYPE_SCAFFOLDER_ACTION } from '@backstage/plugin-scaffolder-common/alpha'; +import { RESOURCE_TYPE_SCAFFOLDER_TASK } from '@backstage/plugin-scaffolder-common/alpha'; import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; import { ScaffolderEntitiesProcessor as ScaffolderEntitiesProcessor_2 } from '@backstage/plugin-catalog-backend-module-scaffolder-entity-model'; import { Schema } from 'jsonschema'; @@ -478,10 +479,10 @@ export interface RouterOptions { lifecycle?: LifecycleService; // (undocumented) logger: Logger; + // Warning: (ae-forgotten-export) The symbol "ScaffolderPermissionRuleInput" needs to be exported by the entry point index.d.ts + // // (undocumented) - permissionRules?: Array< - TemplatePermissionRuleInput | ActionPermissionRuleInput - >; + permissionRules?: Array; // (undocumented) permissions?: PermissionsService; // (undocumented) @@ -575,6 +576,16 @@ export class TaskManager implements TaskContext_2 { ): Promise; } +// @public (undocumented) +export type TaskPermissionRuleInput< + TParams extends PermissionRuleParams = PermissionRuleParams, +> = PermissionRule< + TemplateEntityStepV1beta3 | TemplateParametersV1beta3, + {}, + typeof RESOURCE_TYPE_SCAFFOLDER_TASK, + TParams +>; + // @public @deprecated (undocumented) export type TaskSecrets = TaskSecrets_2; diff --git a/plugins/scaffolder-common/api-report-alpha.md b/plugins/scaffolder-common/api-report-alpha.md index de9dffd1b9..3762b0b773 100644 --- a/plugins/scaffolder-common/api-report-alpha.md +++ b/plugins/scaffolder-common/api-report-alpha.md @@ -8,9 +8,15 @@ import { ResourcePermission } from '@backstage/plugin-permission-common'; // @alpha export const actionExecutePermission: ResourcePermission<'scaffolder-action'>; +// @alpha +export const actionReadPermission: ResourcePermission<'scaffolder-action'>; + // @alpha export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action'; +// @alpha +export const RESOURCE_TYPE_SCAFFOLDER_TASK = 'scaffolder-task'; + // @alpha export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; @@ -23,9 +29,21 @@ export const scaffolderPermissions: ( | ResourcePermission<'scaffolder-template'> )[]; +// @alpha +export const scaffolderTaskPermissions: ResourcePermission<'scaffolder-task'>[]; + // @alpha export const scaffolderTemplatePermissions: ResourcePermission<'scaffolder-template'>[]; +// @alpha +export const taskCancelPermission: ResourcePermission<'scaffolder-task'>; + +// @alpha +export const taskCreatePermission: ResourcePermission<'scaffolder-task'>; + +// @alpha +export const taskReadPermission: ResourcePermission<'scaffolder-task'>; + // @alpha export const templateParameterReadPermission: ResourcePermission<'scaffolder-template'>; From 2b959e049fd976396f0d97ee621c48deaa5565cb Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Fri, 26 Apr 2024 10:21:50 -0400 Subject: [PATCH 08/24] chore: merge dependency imports Signed-off-by: Frank Kong --- plugins/scaffolder-react/src/extensions/rjsf.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-react/src/extensions/rjsf.ts b/plugins/scaffolder-react/src/extensions/rjsf.ts index cacedad080..b81f06758f 100644 --- a/plugins/scaffolder-react/src/extensions/rjsf.ts +++ b/plugins/scaffolder-react/src/extensions/rjsf.ts @@ -14,7 +14,14 @@ * limitations under the License. */ -import { ComponentType, ElementType, FormEvent, ReactNode, Ref } from 'react'; +import { + ComponentType, + ElementType, + FormEvent, + HTMLAttributes, + ReactNode, + Ref, +} from 'react'; import { ErrorSchema, FormContextType, @@ -32,7 +39,6 @@ import { Experimental_DefaultFormStateBehavior, ErrorTransformer, } from '@rjsf/utils'; -import { HTMLAttributes } from 'react'; import Form, { IChangeEvent } from '@rjsf/core'; /** From e4043011e7936cf6ea030850754ae769875f8143 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Fri, 26 Apr 2024 12:09:24 -0400 Subject: [PATCH 09/24] chore(scaffolder): fix api-report Signed-off-by: Frank Kong --- plugins/scaffolder-backend/api-report.md | 8 ++++++-- plugins/scaffolder-backend/src/service/router.ts | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index dc7917e825..e99ac56bee 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -479,8 +479,6 @@ export interface RouterOptions { lifecycle?: LifecycleService; // (undocumented) logger: Logger; - // Warning: (ae-forgotten-export) The symbol "ScaffolderPermissionRuleInput" needs to be exported by the entry point index.d.ts - // // (undocumented) permissionRules?: Array; // (undocumented) @@ -501,6 +499,12 @@ export type RunCommandOptions = ExecuteShellCommandOptions; // @public @deprecated export const ScaffolderEntitiesProcessor: typeof ScaffolderEntitiesProcessor_2; +// @public (undocumented) +export type ScaffolderPermissionRuleInput = + | TemplatePermissionRuleInput + | ActionPermissionRuleInput + | TaskPermissionRuleInput; + // @public @deprecated export type SerializedTask = SerializedTask_2; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 3fd1e14fa1..228f7bed12 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -104,7 +104,11 @@ import { } from '@backstage/plugin-auth-node'; import { InternalTaskSecrets } from '../scaffolder/tasks/types'; -type ScaffolderPermissionRuleInput = +/** + * + * @public + */ +export type ScaffolderPermissionRuleInput = | TemplatePermissionRuleInput | ActionPermissionRuleInput | TaskPermissionRuleInput; From 3078ff09b7654cafde6c59062da6beba37353a70 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Fri, 26 Apr 2024 12:31:08 -0400 Subject: [PATCH 10/24] chore: update yarn.lock Signed-off-by: Frank Kong --- yarn.lock | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/yarn.lock b/yarn.lock index 3cf5a79175..55cd380654 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5684,6 +5684,7 @@ __metadata: lodash: ^4.17.21 pluralize: ^8.0.0 react-use: ^17.2.4 + swr: ^2.0.0 zen-observable: ^0.10.0 peerDependencies: react: ^16.13.1 || ^17.0.0 || ^18.0.0 @@ -6877,6 +6878,8 @@ __metadata: "@backstage/plugin-catalog": "workspace:^" "@backstage/plugin-catalog-common": "workspace:^" "@backstage/plugin-catalog-react": "workspace:^" + "@backstage/plugin-permission-common": "workspace:^" + "@backstage/plugin-permission-react": "workspace:^" "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/test-utils": "workspace:^" "@backstage/theme": "workspace:^" @@ -6907,6 +6910,7 @@ __metadata: luxon: ^3.0.0 qs: ^6.9.4 react-use: ^17.2.4 + swr: ^2.0.0 use-immer: ^0.9.0 zen-observable: ^0.10.0 zod: ^3.22.4 @@ -6937,6 +6941,7 @@ __metadata: "@backstage/plugin-catalog": "workspace:^" "@backstage/plugin-catalog-common": "workspace:^" "@backstage/plugin-catalog-react": "workspace:^" + "@backstage/plugin-permission-common": "workspace:^" "@backstage/plugin-permission-react": "workspace:^" "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/plugin-scaffolder-react": "workspace:^" @@ -6973,6 +6978,7 @@ __metadata: msw: ^1.0.0 qs: ^6.9.4 react-use: ^17.2.4 + swr: ^2.0.0 yaml: ^2.0.0 zen-observable: ^0.10.0 zod: ^3.22.4 From 83643ef0919f5dadcdb4ea7ea62bcf593dc04698 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 2 May 2024 10:16:25 -0400 Subject: [PATCH 11/24] chore: add util to perform basic permission check Signed-off-by: Frank Kong --- .../scaffolder-backend/src/service/router.ts | 147 +++++------------- .../src/util/checkPermissions.ts | 53 +++++++ 2 files changed, 95 insertions(+), 105 deletions(-) create mode 100644 plugins/scaffolder-backend/src/util/checkPermissions.ts diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 228f7bed12..3c98adc3f4 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -103,6 +103,7 @@ import { IdentityApiGetIdentityRequest, } from '@backstage/plugin-auth-node'; import { InternalTaskSecrets } from '../scaffolder/tasks/types'; +import { checkPermission } from '../util/checkPermissions'; /** * @@ -492,19 +493,11 @@ export async function createRouter( ) .get('/v2/actions', async (req, res) => { const credentials = await httpAuth.credentials(req); - - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: actionReadPermission }], - { credentials: credentials }, - ) - )[0]; - - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } + await checkPermission({ + credentials, + permissions: [actionReadPermission], + permissionService: permissions, + }); const actionsList = actionRegistry.list().map(action => { return { id: action.id, @@ -522,18 +515,11 @@ export async function createRouter( }); const credentials = await httpAuth.credentials(req); - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: taskCreatePermission }], - { credentials: credentials }, - ) - )[0]; - - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } + await checkPermission({ + credentials, + permissions: [taskCreatePermission], + permissionService: permissions, + }); const { token } = await auth.getPluginRequestToken({ onBehalfOf: credentials, @@ -612,22 +598,13 @@ export async function createRouter( }) .get('/v2/tasks', async (req, res) => { const credentials = await httpAuth.credentials(req); - - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: taskReadPermission }], - { credentials: credentials }, - ) - )[0]; - - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } + await checkPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + }); const [userEntityRef] = [req.query.createdBy].flat(); - if ( typeof userEntityRef !== 'string' && typeof userEntityRef !== 'undefined' @@ -649,19 +626,11 @@ export async function createRouter( }) .get('/v2/tasks/:taskId', async (req, res) => { const credentials = await httpAuth.credentials(req); - - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: taskReadPermission }], - { credentials: credentials }, - ) - )[0]; - - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } + await checkPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + }); const { taskId } = req.params; const task = await taskBroker.get(taskId); @@ -674,22 +643,12 @@ export async function createRouter( }) .post('/v2/tasks/:taskId/cancel', async (req, res) => { const credentials = await httpAuth.credentials(req); - - if (permissions) { - const authorizationResponses = await permissions.authorizeConditional( - [ - { permission: taskCancelPermission }, - { permission: taskReadPermission }, - ], - { credentials: credentials }, - ); - // Requires both read and cancel permissions - for (const response of authorizationResponses) { - if (response.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } - } + // Requires both read and cancel permissions + await checkPermission({ + credentials, + permissions: [taskCancelPermission, taskReadPermission], + permissionService: permissions, + }); const { taskId } = req.params; await taskBroker.cancel?.(taskId); @@ -697,19 +656,12 @@ export async function createRouter( }) .get('/v2/tasks/:taskId/eventstream', async (req, res) => { const credentials = await httpAuth.credentials(req); + await checkPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + }); - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: taskReadPermission }], - { credentials: credentials }, - ) - )[0]; - - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } const { taskId } = req.params; const after = req.query.after !== undefined ? Number(req.query.after) : undefined; @@ -759,19 +711,12 @@ export async function createRouter( }) .get('/v2/tasks/:taskId/events', async (req, res) => { const credentials = await httpAuth.credentials(req); + await checkPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + }); - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: taskReadPermission }], - { credentials: credentials }, - ) - )[0]; - - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } const { taskId } = req.params; const after = Number(req.query.after) || undefined; @@ -803,19 +748,11 @@ export async function createRouter( }) .post('/v2/dry-run', async (req, res) => { const credentials = await httpAuth.credentials(req); - - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: taskCreatePermission }], - { credentials: credentials }, - ) - )[0]; - - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } + await checkPermission({ + credentials, + permissions: [taskCreatePermission], + permissionService: permissions, + }); const bodySchema = z.object({ template: z.unknown(), diff --git a/plugins/scaffolder-backend/src/util/checkPermissions.ts b/plugins/scaffolder-backend/src/util/checkPermissions.ts new file mode 100644 index 0000000000..42d841ce2b --- /dev/null +++ b/plugins/scaffolder-backend/src/util/checkPermissions.ts @@ -0,0 +1,53 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + BackstageCredentials, + PermissionsService, +} from '@backstage/backend-plugin-api'; +import { NotAllowedError } from '@backstage/errors'; +import { + AuthorizeResult, + ResourcePermission, +} from '@backstage/plugin-permission-common'; + +export type checkPermissionOptions = { + credentials: BackstageCredentials; + permissions: ResourcePermission[]; + permissionService?: PermissionsService; +}; + +/** + * Does a basic check on permissions. Throws 403 error if any permission responds with AuthorizeResult.DENY + * @public + */ +export async function checkPermission(options: checkPermissionOptions) { + const { permissions, permissionService, credentials } = options; + if (permissionService) { + const permissionRequest = permissions.map(resourcePermission => ({ + permission: resourcePermission, + })); + const authorizationResponses = await permissionService.authorizeConditional( + permissionRequest, + { credentials: credentials }, + ); + + for (const response of authorizationResponses) { + if (response.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + } +} From a2a6a826d82e4fb578b8e0d1f9b662ba42bbcfed Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 2 May 2024 10:29:44 -0400 Subject: [PATCH 12/24] chore: remove unused imports Signed-off-by: Frank Kong --- plugins/scaffolder-backend/src/service/router.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 3c98adc3f4..0d55d20c9e 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -30,12 +30,7 @@ import { UserEntity, } from '@backstage/catalog-model'; import { Config, readDurationFromConfig } from '@backstage/config'; -import { - InputError, - NotAllowedError, - NotFoundError, - stringifyError, -} from '@backstage/errors'; +import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { HumanDuration, JsonObject, JsonValue } from '@backstage/types'; import { @@ -79,10 +74,7 @@ import { import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; -import { - AuthorizeResult, - PermissionRuleParams, -} from '@backstage/plugin-permission-common'; +import { PermissionRuleParams } from '@backstage/plugin-permission-common'; import { createConditionAuthorizer, createPermissionIntegrationRouter, From e5b903599f676edda9ac4d51419a5a931bd3a8d0 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 2 May 2024 12:07:30 -0400 Subject: [PATCH 13/24] chore: update unit tests Signed-off-by: Frank Kong --- plugins/scaffolder-backend/src/service/router.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 2118f46b22..4443b28d09 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -895,6 +895,11 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ it('filters steps that the user is not authorized to see', async () => { jest .spyOn(permissionApi, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + ]) .mockImplementation(async () => [ { result: AuthorizeResult.ALLOW, From ea7cb44de53845959c6f9acec2f1a4d21b66404f Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 2 May 2024 22:36:30 -0400 Subject: [PATCH 14/24] chore: apply suggestions Signed-off-by: Frank Kong --- plugins/catalog/package.json | 2 +- .../src/components/AboutCard/AboutCard.tsx | 3 +- .../tasks/NunjucksWorkflowRunner.ts | 2 +- .../scaffolder-backend/src/service/router.ts | 38 ++++++++++++------- .../src/util/checkPermissions.ts | 6 +-- plugins/scaffolder-common/src/permissions.ts | 18 +-------- .../components/TemplateCard/TemplateCard.tsx | 1 - .../components/OngoingTask/ContextMenu.tsx | 9 +---- .../components/OngoingTask/OngoingTask.tsx | 8 +--- yarn.lock | 4 +- 10 files changed, 36 insertions(+), 55 deletions(-) diff --git a/plugins/catalog/package.json b/plugins/catalog/package.json index da973aadb5..d22d9778e6 100644 --- a/plugins/catalog/package.json +++ b/plugins/catalog/package.json @@ -89,7 +89,7 @@ "@testing-library/react": "^15.0.0", "@testing-library/user-event": "^14.0.0", "@types/pluralize": "^0.0.33", - "swr": "^2.0.0" + "swr": "^2.2.5" }, "peerDependencies": { "react": "^16.13.1 || ^17.0.0 || ^18.0.0", diff --git a/plugins/catalog/src/components/AboutCard/AboutCard.tsx b/plugins/catalog/src/components/AboutCard/AboutCard.tsx index b63666f80c..ee6147c54e 100644 --- a/plugins/catalog/src/components/AboutCard/AboutCard.tsx +++ b/plugins/catalog/src/components/AboutCard/AboutCard.tsx @@ -116,10 +116,9 @@ export function AboutCard(props: AboutCardProps) { const { allowed: canRefresh } = useEntityPermission( catalogEntityRefreshPermission, ); - const { kind, name, namespace } = entity.metadata; + const { allowed: canCreateTemplateTask } = usePermission({ permission: taskCreatePermission, - resourceRef: `${kind}:${namespace}/${name}`, }); const entitySourceLocation = getEntitySourceLocation( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 5d24ac7cc7..99eb7c7bb5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -31,6 +31,7 @@ import { SecureTemplateRenderer, } from '../../lib/templating/SecureTemplater'; import { + TaskRecovery, TaskSpec, TaskSpecV1beta3, TaskStep, @@ -52,7 +53,6 @@ import { } from '@backstage/plugin-permission-common'; import { scaffolderActionRules } from '../../service/rules'; import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha'; -import { TaskRecovery } from '@backstage/plugin-scaffolder-common'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { loggerToWinstonLogger } from '@backstage/backend-common'; import { BackstageLoggerTransport, WinstonLogger } from './logger'; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0d55d20c9e..287d058030 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -30,7 +30,12 @@ import { UserEntity, } from '@backstage/catalog-model'; import { Config, readDurationFromConfig } from '@backstage/config'; -import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; +import { + InputError, + NotAllowedError, + NotFoundError, + stringifyError, +} from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { HumanDuration, JsonObject, JsonValue } from '@backstage/types'; import { @@ -43,7 +48,6 @@ import { import { RESOURCE_TYPE_SCAFFOLDER_ACTION, RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, - RESOURCE_TYPE_SCAFFOLDER_TASK, scaffolderActionPermissions, scaffolderTemplatePermissions, taskCancelPermission, @@ -74,7 +78,10 @@ import { import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; -import { PermissionRuleParams } from '@backstage/plugin-permission-common'; +import { + AuthorizeResult, + PermissionRuleParams, +} from '@backstage/plugin-permission-common'; import { createConditionAuthorizer, createPermissionIntegrationRouter, @@ -97,11 +104,7 @@ import { import { InternalTaskSecrets } from '../scaffolder/tasks/types'; import { checkPermission } from '../util/checkPermissions'; -/** - * - * @public - */ -export type ScaffolderPermissionRuleInput = +type ScaffolderPermissionRuleInput = | TemplatePermissionRuleInput | ActionPermissionRuleInput | TaskPermissionRuleInput; @@ -440,7 +443,7 @@ export async function createRouter( rules: actionRules, }, { - resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, + resourceType: 'basic', permissions: scaffolderTaskPermissions, rules: taskRules, }, @@ -485,11 +488,18 @@ export async function createRouter( ) .get('/v2/actions', async (req, res) => { const credentials = await httpAuth.credentials(req); - await checkPermission({ - credentials, - permissions: [actionReadPermission], - permissionService: permissions, - }); + if (permissions) { + const authorizationResponse = ( + await permissions.authorizeConditional( + [{ permission: actionReadPermission }], + { credentials: credentials }, + ) + )[0]; + if (authorizationResponse.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + const actionsList = actionRegistry.list().map(action => { return { id: action.id, diff --git a/plugins/scaffolder-backend/src/util/checkPermissions.ts b/plugins/scaffolder-backend/src/util/checkPermissions.ts index 42d841ce2b..40aa673b3f 100644 --- a/plugins/scaffolder-backend/src/util/checkPermissions.ts +++ b/plugins/scaffolder-backend/src/util/checkPermissions.ts @@ -20,12 +20,12 @@ import { import { NotAllowedError } from '@backstage/errors'; import { AuthorizeResult, - ResourcePermission, + BasicPermission, } from '@backstage/plugin-permission-common'; export type checkPermissionOptions = { credentials: BackstageCredentials; - permissions: ResourcePermission[]; + permissions: BasicPermission[]; permissionService?: PermissionsService; }; @@ -39,7 +39,7 @@ export async function checkPermission(options: checkPermissionOptions) { const permissionRequest = permissions.map(resourcePermission => ({ permission: resourcePermission, })); - const authorizationResponses = await permissionService.authorizeConditional( + const authorizationResponses = await permissionService.authorize( permissionRequest, { credentials: credentials }, ); diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 4a4137875a..6bd7e130ec 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -30,13 +30,6 @@ export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; */ export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action'; -/** - * Permission resource type which corresponds to a scaffolder task. - * - * @alpha - */ -export const RESOURCE_TYPE_SCAFFOLDER_TASK = 'scaffolder-task'; - /** * This permission is used to authorize actions that involve executing * an action from a template. @@ -49,6 +42,7 @@ export const actionExecutePermission = createPermission({ resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, }); +// TODO: Figure out whether to convert this to a basic permission or remove it completely since the current rules aren't applicable to this permission /** * This permission is used to authorize actions that involve access the action registry * @@ -101,8 +95,6 @@ export const templateStepReadPermission = createPermission({ * This permission is used to authorize actions that involve reading one or more tasks in the scaffolder, * and reading logs of tasks * - * Task cancellation would also require this permission. - * * @alpha */ export const taskReadPermission = createPermission({ @@ -110,7 +102,6 @@ export const taskReadPermission = createPermission({ attributes: { action: 'read', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, }); /** @@ -123,20 +114,16 @@ export const taskCreatePermission = createPermission({ attributes: { action: 'create', }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, }); /** - * This permission us used to authorize actions that involve the cancellation of tasks in the scaffolder. - * - * This will require the `scaffolder.task.read` permission to be authorized. + * This permission is used to authorize actions that involve the cancellation of tasks in the scaffolder. * * @alpha */ export const taskCancelPermission = createPermission({ name: 'scaffolder.task.cancel', attributes: {}, - resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, }); /** @@ -144,7 +131,6 @@ export const taskCancelPermission = createPermission({ * @alpha */ export const scaffolderPermissions = [ - actionExecutePermission, templateParameterReadPermission, templateStepReadPermission, ]; diff --git a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx index 253d588862..5d2c8aa7fe 100644 --- a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx +++ b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx @@ -112,7 +112,6 @@ export const TemplateCard = (props: TemplateCardProps) => { const { allowed: canCreateTask } = usePermission({ permission: taskCreatePermission, - resourceRef: 'task', }); const handleChoose = useCallback(() => { analytics.captureEvent('click', `Template has been opened`); diff --git a/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx b/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx index 1a5cccf142..f96f83fd2f 100644 --- a/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx @@ -77,25 +77,18 @@ export const ContextMenu = (props: ContextMenuProps) => { } }); - // Used dummy string value for `resourceRef` since `allowed` field will always return `false` if `resourceRef` is `undefined` const { allowed: canCancelTask } = usePermission({ permission: taskCancelPermission, - resourceRef: 'task', }); const { allowed: canReadTask } = usePermission({ permission: taskReadPermission, - resourceRef: 'task', }); const { allowed: canCreateTask } = usePermission({ permission: taskCreatePermission, - resourceRef: 'task', }); - // Cancel endpoint requires user to have both read and cancel permissions - const cancelNotAllowed = !(canReadTask && canCancelTask); - // Start Over endpoint requires user to have both read (to grab parameters) and create (to create new task) permissions const canStartOver = canReadTask && canCreateTask; @@ -150,7 +143,7 @@ export const ContextMenu = (props: ContextMenuProps) => { disabled={ !cancelEnabled || cancelStatus !== 'not-executed' || - cancelNotAllowed + !canCancelTask } data-testid="cancel-task" > diff --git a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx index b7af4b6618..f4099028ba 100644 --- a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx @@ -91,22 +91,16 @@ export const OngoingTask = (props: { // Used dummy string value for `resourceRef` since `allowed` field will always return `false` if `resourceRef` is `undefined` const { allowed: canCancelTask } = usePermission({ permission: taskCancelPermission, - resourceRef: 'task', }); const { allowed: canReadTask } = usePermission({ permission: taskReadPermission, - resourceRef: 'task', }); const { allowed: canCreateTask } = usePermission({ permission: taskCreatePermission, - resourceRef: 'task', }); - // Cancel endpoint requires user to have both read and cancel permissions - const cancelNotAllowed = !(canReadTask && canCancelTask); - // Start Over endpoint requires user to have both read (to grab parameters) and create (to create new task) permissions const canStartOver = canReadTask && canCreateTask; @@ -228,7 +222,7 @@ export const OngoingTask = (props: { disabled={ !cancelEnabled || cancelStatus !== 'not-executed' || - cancelNotAllowed + !canCancelTask } onClick={triggerCancel} data-testid="cancel-button" diff --git a/yarn.lock b/yarn.lock index bf16ce2937..f5d34f9c95 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5684,7 +5684,7 @@ __metadata: lodash: ^4.17.21 pluralize: ^8.0.0 react-use: ^17.2.4 - swr: ^2.0.0 + swr: ^2.2.5 zen-observable: ^0.10.0 peerDependencies: react: ^16.13.1 || ^17.0.0 || ^18.0.0 @@ -39520,7 +39520,7 @@ __metadata: languageName: node linkType: hard -"swr@npm:^2.0.0": +"swr@npm:^2.0.0, swr@npm:^2.2.5": version: 2.2.5 resolution: "swr@npm:2.2.5" dependencies: From b75d78761cfa1876346de726ed5258da08c88fbb Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 2 May 2024 22:53:12 -0400 Subject: [PATCH 15/24] chore(scaffolder-backend): update unit tests for router Signed-off-by: Frank Kong --- .../scaffolder-backend/src/service/router.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 4443b28d09..c129859c49 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -235,6 +235,11 @@ describe('createRouter', () => { result: AuthorizeResult.ALLOW, }, ]); + jest.spyOn(permissionApi, 'authorize').mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + ]); }); afterEach(() => { @@ -741,6 +746,11 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ result: AuthorizeResult.ALLOW, }, ]); + jest.spyOn(permissionApi, 'authorize').mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + ]); }); afterEach(() => { @@ -895,11 +905,6 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ it('filters steps that the user is not authorized to see', async () => { jest .spyOn(permissionApi, 'authorizeConditional') - .mockImplementationOnce(async () => [ - { - result: AuthorizeResult.ALLOW, - }, - ]) .mockImplementation(async () => [ { result: AuthorizeResult.ALLOW, From e01a2e93caeaec35b95afa50189df1b2548d402c Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Thu, 2 May 2024 23:16:04 -0400 Subject: [PATCH 16/24] chore: fix tsc errors and update api-report Signed-off-by: Frank Kong --- plugins/scaffolder-backend/api-report.md | 14 +-------- .../scaffolder-backend/src/service/router.ts | 30 +++++-------------- plugins/scaffolder-common/api-report-alpha.md | 17 ++++------- 3 files changed, 14 insertions(+), 47 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index e99ac56bee..e6738497fe 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -36,7 +36,6 @@ import { PermissionsService } from '@backstage/backend-plugin-api'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; import { RESOURCE_TYPE_SCAFFOLDER_ACTION } from '@backstage/plugin-scaffolder-common/alpha'; -import { RESOURCE_TYPE_SCAFFOLDER_TASK } from '@backstage/plugin-scaffolder-common/alpha'; import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE } from '@backstage/plugin-scaffolder-common/alpha'; import { ScaffolderEntitiesProcessor as ScaffolderEntitiesProcessor_2 } from '@backstage/plugin-catalog-backend-module-scaffolder-entity-model'; import { Schema } from 'jsonschema'; @@ -502,8 +501,7 @@ export const ScaffolderEntitiesProcessor: typeof ScaffolderEntitiesProcessor_2; // @public (undocumented) export type ScaffolderPermissionRuleInput = | TemplatePermissionRuleInput - | ActionPermissionRuleInput - | TaskPermissionRuleInput; + | ActionPermissionRuleInput; // @public @deprecated export type SerializedTask = SerializedTask_2; @@ -580,16 +578,6 @@ export class TaskManager implements TaskContext_2 { ): Promise; } -// @public (undocumented) -export type TaskPermissionRuleInput< - TParams extends PermissionRuleParams = PermissionRuleParams, -> = PermissionRule< - TemplateEntityStepV1beta3 | TemplateParametersV1beta3, - {}, - typeof RESOURCE_TYPE_SCAFFOLDER_TASK, - TParams ->; - // @public @deprecated (undocumented) export type TaskSecrets = TaskSecrets_2; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 287d058030..ff1796e71f 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -104,10 +104,13 @@ import { import { InternalTaskSecrets } from '../scaffolder/tasks/types'; import { checkPermission } from '../util/checkPermissions'; -type ScaffolderPermissionRuleInput = +/** + * + * @public + */ +export type ScaffolderPermissionRuleInput = | TemplatePermissionRuleInput - | ActionPermissionRuleInput - | TaskPermissionRuleInput; + | ActionPermissionRuleInput; /** * @@ -145,23 +148,6 @@ function isActionPermissionRuleInput( return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_ACTION; } -/** - * - * @public - */ -export type TaskPermissionRuleInput< - TParams extends PermissionRuleParams = PermissionRuleParams, -> = PermissionRule< - TemplateEntityStepV1beta3 | TemplateParametersV1beta3, - {}, - typeof RESOURCE_TYPE_SCAFFOLDER_TASK, - TParams ->; -function isTaskPermissionRuleInput( - permissionRule: ScaffolderPermissionRuleInput, -): permissionRule is TaskPermissionRuleInput { - return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_TASK; -} /** * RouterOptions * @@ -418,14 +404,12 @@ export async function createRouter( const actionRules: ActionPermissionRuleInput[] = Object.values( scaffolderActionRules, ); - const taskRules: TaskPermissionRuleInput[] = []; if (permissionRules) { templateRules.push( ...permissionRules.filter(isTemplatePermissionRuleInput), ); actionRules.push(...permissionRules.filter(isActionPermissionRuleInput)); - taskRules.push(...permissionRules.filter(isTaskPermissionRuleInput)); } const isAuthorized = createConditionAuthorizer(Object.values(templateRules)); @@ -445,7 +429,7 @@ export async function createRouter( { resourceType: 'basic', permissions: scaffolderTaskPermissions, - rules: taskRules, + rules: [], }, ], }); diff --git a/plugins/scaffolder-common/api-report-alpha.md b/plugins/scaffolder-common/api-report-alpha.md index 3762b0b773..a06f3123e7 100644 --- a/plugins/scaffolder-common/api-report-alpha.md +++ b/plugins/scaffolder-common/api-report-alpha.md @@ -3,6 +3,7 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +import { BasicPermission } from '@backstage/plugin-permission-common'; import { ResourcePermission } from '@backstage/plugin-permission-common'; // @alpha @@ -14,9 +15,6 @@ export const actionReadPermission: ResourcePermission<'scaffolder-action'>; // @alpha export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action'; -// @alpha -export const RESOURCE_TYPE_SCAFFOLDER_TASK = 'scaffolder-task'; - // @alpha export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; @@ -24,25 +22,22 @@ export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; export const scaffolderActionPermissions: ResourcePermission<'scaffolder-action'>[]; // @alpha -export const scaffolderPermissions: ( - | ResourcePermission<'scaffolder-action'> - | ResourcePermission<'scaffolder-template'> -)[]; +export const scaffolderPermissions: ResourcePermission<'scaffolder-template'>[]; // @alpha -export const scaffolderTaskPermissions: ResourcePermission<'scaffolder-task'>[]; +export const scaffolderTaskPermissions: BasicPermission[]; // @alpha export const scaffolderTemplatePermissions: ResourcePermission<'scaffolder-template'>[]; // @alpha -export const taskCancelPermission: ResourcePermission<'scaffolder-task'>; +export const taskCancelPermission: BasicPermission; // @alpha -export const taskCreatePermission: ResourcePermission<'scaffolder-task'>; +export const taskCreatePermission: BasicPermission; // @alpha -export const taskReadPermission: ResourcePermission<'scaffolder-task'>; +export const taskReadPermission: BasicPermission; // @alpha export const templateParameterReadPermission: ResourcePermission<'scaffolder-template'>; From 84c87637f3b98e3f0679df780e47c92f56c59dd5 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Fri, 3 May 2024 10:39:47 -0400 Subject: [PATCH 17/24] chore(scaffolder-backend): revert addition of internal type Signed-off-by: Frank Kong --- plugins/scaffolder-backend/api-report.md | 9 +++------ plugins/scaffolder-backend/src/service/router.ts | 16 +++++----------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index e6738497fe..6af1ac04a6 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -479,7 +479,9 @@ export interface RouterOptions { // (undocumented) logger: Logger; // (undocumented) - permissionRules?: Array; + permissionRules?: Array< + TemplatePermissionRuleInput | ActionPermissionRuleInput + >; // (undocumented) permissions?: PermissionsService; // (undocumented) @@ -498,11 +500,6 @@ export type RunCommandOptions = ExecuteShellCommandOptions; // @public @deprecated export const ScaffolderEntitiesProcessor: typeof ScaffolderEntitiesProcessor_2; -// @public (undocumented) -export type ScaffolderPermissionRuleInput = - | TemplatePermissionRuleInput - | ActionPermissionRuleInput; - // @public @deprecated export type SerializedTask = SerializedTask_2; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index ff1796e71f..759c5ca8d7 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -104,14 +104,6 @@ import { import { InternalTaskSecrets } from '../scaffolder/tasks/types'; import { checkPermission } from '../util/checkPermissions'; -/** - * - * @public - */ -export type ScaffolderPermissionRuleInput = - | TemplatePermissionRuleInput - | ActionPermissionRuleInput; - /** * * @public @@ -125,7 +117,7 @@ export type TemplatePermissionRuleInput< TParams >; function isTemplatePermissionRuleInput( - permissionRule: ScaffolderPermissionRuleInput, + permissionRule: TemplatePermissionRuleInput | ActionPermissionRuleInput, ): permissionRule is TemplatePermissionRuleInput { return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_TEMPLATE; } @@ -143,7 +135,7 @@ export type ActionPermissionRuleInput< TParams >; function isActionPermissionRuleInput( - permissionRule: ScaffolderPermissionRuleInput, + permissionRule: TemplatePermissionRuleInput | ActionPermissionRuleInput, ): permissionRule is ActionPermissionRuleInput { return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_ACTION; } @@ -176,7 +168,9 @@ export interface RouterOptions { additionalTemplateFilters?: Record; additionalTemplateGlobals?: Record; permissions?: PermissionsService; - permissionRules?: Array; + permissionRules?: Array< + TemplatePermissionRuleInput | ActionPermissionRuleInput + >; auth?: AuthService; httpAuth?: HttpAuthService; identity?: IdentityApi; From 2ba6e52f40ecc2b8b12bc1cd4505b903be1b2074 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Tue, 7 May 2024 12:02:17 -0400 Subject: [PATCH 18/24] chore: remove action read scaffolder permission Signed-off-by: Frank Kong --- .../scaffolder-backend/src/service/router.ts | 34 ++--------------- plugins/scaffolder-common/src/permissions.ts | 37 ++++++------------- 2 files changed, 15 insertions(+), 56 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 759c5ca8d7..384b9f5340 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -30,12 +30,7 @@ import { UserEntity, } from '@backstage/catalog-model'; import { Config, readDurationFromConfig } from '@backstage/config'; -import { - InputError, - NotAllowedError, - NotFoundError, - stringifyError, -} from '@backstage/errors'; +import { InputError, NotFoundError, stringifyError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { HumanDuration, JsonObject, JsonValue } from '@backstage/types'; import { @@ -56,7 +51,6 @@ import { templateParameterReadPermission, templateStepReadPermission, scaffolderTaskPermissions, - actionReadPermission, } from '@backstage/plugin-scaffolder-common/alpha'; import express from 'express'; import Router from 'express-promise-router'; @@ -78,10 +72,7 @@ import { import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; -import { - AuthorizeResult, - PermissionRuleParams, -} from '@backstage/plugin-permission-common'; +import { PermissionRuleParams } from '@backstage/plugin-permission-common'; import { createConditionAuthorizer, createPermissionIntegrationRouter, @@ -420,12 +411,8 @@ export async function createRouter( permissions: scaffolderActionPermissions, rules: actionRules, }, - { - resourceType: 'basic', - permissions: scaffolderTaskPermissions, - rules: [], - }, ], + permissions: scaffolderTaskPermissions, }); router.use(permissionIntegrationRouter); @@ -464,20 +451,7 @@ export async function createRouter( }); }, ) - .get('/v2/actions', async (req, res) => { - const credentials = await httpAuth.credentials(req); - if (permissions) { - const authorizationResponse = ( - await permissions.authorizeConditional( - [{ permission: actionReadPermission }], - { credentials: credentials }, - ) - )[0]; - if (authorizationResponse.result === AuthorizeResult.DENY) { - throw new NotAllowedError(); - } - } - + .get('/v2/actions', async (_req, res) => { const actionsList = actionRegistry.list().map(action => { return { id: action.id, diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 6bd7e130ec..c441b48d5d 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -42,19 +42,6 @@ export const actionExecutePermission = createPermission({ resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, }); -// TODO: Figure out whether to convert this to a basic permission or remove it completely since the current rules aren't applicable to this permission -/** - * This permission is used to authorize actions that involve access the action registry - * - * @alpha - */ -export const actionReadPermission = createPermission({ - name: 'scaffolder.action.read', - attributes: { - action: 'read', - }, - resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION, -}); /** * This permission is used to authorize actions that involve reading * one or more parameters from a template. @@ -126,15 +113,6 @@ export const taskCancelPermission = createPermission({ attributes: {}, }); -/** - * List of all the scaffolder permissions - * @alpha - */ -export const scaffolderPermissions = [ - templateParameterReadPermission, - templateStepReadPermission, -]; - /** * List of the scaffolder permissions that are associated with template steps and parameters. * @alpha @@ -148,10 +126,7 @@ export const scaffolderTemplatePermissions = [ * List of the scaffolder permissions that are associated with scaffolder actions. * @alpha */ -export const scaffolderActionPermissions = [ - actionExecutePermission, - actionReadPermission, -]; +export const scaffolderActionPermissions = [actionExecutePermission]; /** * List of the scaffolder permissions that are associated with scaffolder tasks. @@ -162,3 +137,13 @@ export const scaffolderTaskPermissions = [ taskCreatePermission, taskReadPermission, ]; + +/** + * List of all the scaffolder permissions + * @alpha + */ +export const scaffolderPermissions = [ + ...scaffolderTemplatePermissions, + ...scaffolderActionPermissions, + ...scaffolderTaskPermissions, +]; From a1735a9f112323453ccbd2aeda849f47ce84119f Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Tue, 7 May 2024 15:46:41 -0400 Subject: [PATCH 19/24] chore(scaffolder-backend): update api-report Signed-off-by: Frank Kong --- plugins/scaffolder-common/api-report-alpha.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder-common/api-report-alpha.md b/plugins/scaffolder-common/api-report-alpha.md index a06f3123e7..36b6a7cdb0 100644 --- a/plugins/scaffolder-common/api-report-alpha.md +++ b/plugins/scaffolder-common/api-report-alpha.md @@ -9,9 +9,6 @@ import { ResourcePermission } from '@backstage/plugin-permission-common'; // @alpha export const actionExecutePermission: ResourcePermission<'scaffolder-action'>; -// @alpha -export const actionReadPermission: ResourcePermission<'scaffolder-action'>; - // @alpha export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action'; @@ -22,7 +19,11 @@ export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template'; export const scaffolderActionPermissions: ResourcePermission<'scaffolder-action'>[]; // @alpha -export const scaffolderPermissions: ResourcePermission<'scaffolder-template'>[]; +export const scaffolderPermissions: ( + | BasicPermission + | ResourcePermission<'scaffolder-action'> + | ResourcePermission<'scaffolder-template'> +)[]; // @alpha export const scaffolderTaskPermissions: BasicPermission[]; From 9a0c4795b7a9dc5b31e088bec4e3e8b127d6ccb7 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Wed, 8 May 2024 13:39:52 -0400 Subject: [PATCH 20/24] docs(scaffolder-backend): update documentation with new scaffolder permissions Signed-off-by: Frank Kong --- ...der-tasks-parameters-steps-and-actions.md} | 67 +++++++++++++++++-- microsite/sidebars.json | 2 +- 2 files changed, 63 insertions(+), 6 deletions(-) rename docs/features/software-templates/{authorizing-parameters-steps-and-actions.md => authorizing-scaffolder-tasks-parameters-steps-and-actions.md} (77%) diff --git a/docs/features/software-templates/authorizing-parameters-steps-and-actions.md b/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md similarity index 77% rename from docs/features/software-templates/authorizing-parameters-steps-and-actions.md rename to docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md index 072f750a14..76bbf39d30 100644 --- a/docs/features/software-templates/authorizing-parameters-steps-and-actions.md +++ b/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md @@ -1,10 +1,10 @@ --- -id: authorizing-parameters-steps-and-actions -title: 'Authorizing parameters, steps and actions' -description: How to authorize part of a template +id: authorizing-scaffolder-tasks-parameters-steps-and-actions +title: 'Authorizing scaffolder tasks parameters, steps and actions' +description: How to authorize part of a template and authorize scaffolder task access --- -The scaffolder plugin integrates with the Backstage [permission framework](../../permissions/overview.md), which allows you to control access to certain parameters and steps in your templates based on the user executing the template. +The scaffolder plugin integrates with the Backstage [permission framework](../../permissions/overview.md), which allows you to control access to certain parameters and steps in your templates based on the user executing the template. It also allows you to control access to scaffolder tasks. ### Authorizing parameters and steps @@ -174,7 +174,64 @@ class ExamplePermissionPolicy implements PermissionPolicy { } ``` -Although the rules exported by the scaffolder are simple, combining them can help you achieve more complex cases. +### Authorizing scaffolder tasks + +The scaffolder plugin also exposes permissions that can restrict access to tasks, task logs, task creation, and task cancellation. This can be useful if you want to control who has access to the scaffolder. + +```ts title="packages/src/backend/plugins/permissions.ts" +/* highlight-add-start */ +import { + taskCancelPermission, + taskCreatePermission, + taskReadPermission, +} from '@backstage/plugin-scaffolder-common/alpha'; +/* highlight-add-end */ + +class ExamplePermissionPolicy implements PermissionPolicy { + async handle( + request: PolicyQuery, + user?: BackstageIdentityResponse, + ): Promise { + /* highlight-add-start */ + if (isPermission(request.permission, taskCreatePermission)) { + if (user?.identity.userEntityRef === 'user:default/spiderman') { + return { + result: AuthorizeResult.ALLOW, + }; + } + } + if (isPermission(request.permission, taskCancelPermission)) { + if (user?.identity.userEntityRef === 'user:default/spiderman') { + return { + result: AuthorizeResult.ALLOW, + }; + } + } + if (isPermission(request.permission, taskReadPermission)) { + if (user?.identity.userEntityRef === 'user:default/spiderman') { + return { + result: AuthorizeResult.ALLOW, + }; + } + } + /* highlight-add-end */ + + return { + result: AuthorizeResult.DENY, + }; + } +} +``` + +In the provided example permission policy, we only grant the `spiderman` user permissions to perform/access the following actions/resources: + +- Read all scaffolder tasks and their associated events/logs. +- Cancel any ongoing scaffolder tasks. +- Trigger software templates, which effectively creates new scaffolder tasks. + +Any other user would be denied access to these actions/resources. + +Although the rules exported by the scaffolder are simple, combining them can help you achieve more complex use cases. ### Authorizing in the New Backend System diff --git a/microsite/sidebars.json b/microsite/sidebars.json index 6d9ba3a680..f2fc4afdaa 100644 --- a/microsite/sidebars.json +++ b/microsite/sidebars.json @@ -129,7 +129,7 @@ "features/software-templates/writing-tests-for-actions", "features/software-templates/writing-custom-field-extensions", "features/software-templates/writing-custom-step-layouts", - "features/software-templates/authorizing-parameters-steps-and-actions", + "features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions", "features/software-templates/migrating-to-rjsf-v5", "features/software-templates/migrating-from-v1beta2-to-v1beta3" ] From 9a328699b8d6db802ddc2f03388c823d432dea10 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Wed, 8 May 2024 13:44:59 -0400 Subject: [PATCH 21/24] chore: update changesets Signed-off-by: Frank Kong --- .changeset/tender-seas-listen.md | 1 - .changeset/weak-gifts-occur.md | 1 - 2 files changed, 2 deletions(-) diff --git a/.changeset/tender-seas-listen.md b/.changeset/tender-seas-listen.md index cfcea07574..86b0bf4618 100644 --- a/.changeset/tender-seas-listen.md +++ b/.changeset/tender-seas-listen.md @@ -9,4 +9,3 @@ updated the ContextMenu, ActionsPage, OngoingTask and TemplateCard frontend comp - `scaffolder.task.create` - `scaffolder.task.cancel` - `scaffolder.task.read` -- `scaffolder.action.read` diff --git a/.changeset/weak-gifts-occur.md b/.changeset/weak-gifts-occur.md index c1a65e7dff..7834c0a9d9 100644 --- a/.changeset/weak-gifts-occur.md +++ b/.changeset/weak-gifts-occur.md @@ -8,4 +8,3 @@ added the following new permissions to the scaffolder backend endpoints: - `scaffolder.task.create` - `scaffolder.task.cancel` - `scaffolder.task.read` -- `scaffolder.action.read` From a112bb59ea5f54093edd9d99fdd8b2e88398da81 Mon Sep 17 00:00:00 2001 From: Frank Kong <50030060+Zaperex@users.noreply.github.com> Date: Tue, 21 May 2024 08:50:05 -0400 Subject: [PATCH 22/24] Update plugins/scaffolder-backend/src/util/checkPermissions.ts Co-authored-by: Vincenzo Scamporlino Signed-off-by: Frank Kong <50030060+Zaperex@users.noreply.github.com> --- plugins/scaffolder-backend/src/util/checkPermissions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/util/checkPermissions.ts b/plugins/scaffolder-backend/src/util/checkPermissions.ts index 40aa673b3f..b6c0395fbb 100644 --- a/plugins/scaffolder-backend/src/util/checkPermissions.ts +++ b/plugins/scaffolder-backend/src/util/checkPermissions.ts @@ -36,8 +36,8 @@ export type checkPermissionOptions = { export async function checkPermission(options: checkPermissionOptions) { const { permissions, permissionService, credentials } = options; if (permissionService) { - const permissionRequest = permissions.map(resourcePermission => ({ - permission: resourcePermission, + const permissionRequest = permissions.map(permission => ({ + permission, })); const authorizationResponses = await permissionService.authorize( permissionRequest, From 3d71ade01ef2bafbe42a01739e71779c065f53f6 Mon Sep 17 00:00:00 2001 From: Frank Kong <50030060+Zaperex@users.noreply.github.com> Date: Tue, 21 May 2024 10:14:53 -0400 Subject: [PATCH 23/24] Update docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md Co-authored-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Signed-off-by: Frank Kong <50030060+Zaperex@users.noreply.github.com> --- ...authorizing-scaffolder-tasks-parameters-steps-and-actions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md b/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md index 9ca4a230cc..27696af1a9 100644 --- a/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md +++ b/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md @@ -176,7 +176,7 @@ class ExamplePermissionPolicy implements PermissionPolicy { ### Authorizing scaffolder tasks -The scaffolder plugin also exposes permissions that can restrict access to tasks, task logs, task creation, and task cancellation. This can be useful if you want to control who has access to the scaffolder. +The scaffolder plugin also exposes permissions that can restrict access to tasks, task logs, task creation, and task cancellation. This can be useful if you want to control who has access to these areas of the scaffolder. ```ts title="packages/src/backend/plugins/permissions.ts" /* highlight-add-start */ From a1218fc1718f48197c1ac044556b7b7de36c4cb4 Mon Sep 17 00:00:00 2001 From: Frank Kong Date: Tue, 21 May 2024 10:56:23 -0400 Subject: [PATCH 24/24] chore: address review comments for docs Signed-off-by: Frank Kong --- ...ctions.md => authorizing-scaffolder-template-details.md} | 6 +++--- microsite/docusaurus.config.ts | 4 ++++ microsite/sidebars.json | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) rename docs/features/software-templates/{authorizing-scaffolder-tasks-parameters-steps-and-actions.md => authorizing-scaffolder-template-details.md} (97%) diff --git a/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md b/docs/features/software-templates/authorizing-scaffolder-template-details.md similarity index 97% rename from docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md rename to docs/features/software-templates/authorizing-scaffolder-template-details.md index 27696af1a9..127677757c 100644 --- a/docs/features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions.md +++ b/docs/features/software-templates/authorizing-scaffolder-template-details.md @@ -1,7 +1,7 @@ --- -id: authorizing-scaffolder-tasks-parameters-steps-and-actions -title: 'Authorizing scaffolder tasks parameters, steps and actions' -description: How to authorize part of a template and authorize scaffolder task access +id: authorizing-scaffolder-template-details +title: 'Authorizing scaffolder tasks, parameters, steps, and actions' +description: How to authorize parts of a template and authorize scaffolder task access --- The scaffolder plugin integrates with the Backstage [permission framework](../../permissions/overview.md), which allows you to control access to certain parameters and steps in your templates based on the user executing the template. It also allows you to control access to scaffolder tasks. diff --git a/microsite/docusaurus.config.ts b/microsite/docusaurus.config.ts index e697c5024f..75a67406b6 100644 --- a/microsite/docusaurus.config.ts +++ b/microsite/docusaurus.config.ts @@ -171,6 +171,10 @@ const config: Config = { from: '/docs/getting-started/configuration', to: '/docs/getting-started/#next-steps', }, + { + from: '/docs/features/software-templates/authorizing-parameters-steps-and-actions', + to: '/docs/features/software-templates/authorizing-scaffolder-template-details', + }, ], }, ], diff --git a/microsite/sidebars.json b/microsite/sidebars.json index bfb9215705..a3d9f4dbec 100644 --- a/microsite/sidebars.json +++ b/microsite/sidebars.json @@ -130,7 +130,7 @@ "features/software-templates/writing-tests-for-actions", "features/software-templates/writing-custom-field-extensions", "features/software-templates/writing-custom-step-layouts", - "features/software-templates/authorizing-scaffolder-tasks-parameters-steps-and-actions", + "features/software-templates/authorizing-scaffolder-template-details", "features/software-templates/migrating-to-rjsf-v5", "features/software-templates/migrating-from-v1beta2-to-v1beta3" ]