diff --git a/.changeset/tender-seas-listen.md b/.changeset/tender-seas-listen.md new file mode 100644 index 0000000000..86b0bf4618 --- /dev/null +++ b/.changeset/tender-seas-listen.md @@ -0,0 +1,11 @@ +--- +'@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` diff --git a/.changeset/weak-gifts-occur.md b/.changeset/weak-gifts-occur.md new file mode 100644 index 0000000000..7834c0a9d9 --- /dev/null +++ b/.changeset/weak-gifts-occur.md @@ -0,0 +1,10 @@ +--- +'@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` diff --git a/docs/features/software-templates/authorizing-parameters-steps-and-actions.md b/docs/features/software-templates/authorizing-scaffolder-template-details.md similarity index 77% rename from docs/features/software-templates/authorizing-parameters-steps-and-actions.md rename to docs/features/software-templates/authorizing-scaffolder-template-details.md index 105c336538..127677757c 100644 --- a/docs/features/software-templates/authorizing-parameters-steps-and-actions.md +++ b/docs/features/software-templates/authorizing-scaffolder-template-details.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-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. +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 these areas of 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/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 624fe83029..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-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" ] diff --git a/plugins/catalog/package.json b/plugins/catalog/package.json index 44283c44dc..488bc05b4b 100644 --- a/plugins/catalog/package.json +++ b/plugins/catalog/package.json @@ -88,7 +88,8 @@ "@testing-library/jest-dom": "^6.0.0", "@testing-library/react": "^15.0.0", "@testing-library/user-event": "^14.0.0", - "@types/pluralize": "^0.0.33" + "@types/pluralize": "^0.0.33", + "swr": "^2.2.5" }, "peerDependencies": { "react": "^16.13.1 || ^17.0.0 || ^18.0.0", diff --git a/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx b/plugins/catalog/src/components/AboutCard/AboutCard.test.tsx index f90942dcd8..fcea659fc6 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, + })); + 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..ee6147c54e 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'; @@ -115,6 +117,10 @@ export function AboutCard(props: AboutCardProps) { catalogEntityRefreshPermission, ); + const { allowed: canCreateTemplateTask } = usePermission({ + permission: taskCreatePermission, + }); + const entitySourceLocation = getEntitySourceLocation( entity, scmIntegrationsApi, @@ -172,7 +178,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/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 6ae01c60cd..85664e1a6c 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.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 4d04699f2a..8ba26c68a4 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(() => { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 43618ed5a7..384b9f5340 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -45,8 +45,12 @@ import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, scaffolderActionPermissions, scaffolderTemplatePermissions, + taskCancelPermission, + taskCreatePermission, + taskReadPermission, templateParameterReadPermission, templateStepReadPermission, + scaffolderTaskPermissions, } from '@backstage/plugin-scaffolder-common/alpha'; import express from 'express'; import Router from 'express-promise-router'; @@ -54,6 +58,7 @@ import { validate } from 'jsonschema'; import { Logger } from 'winston'; import { z } from 'zod'; import { + TemplateAction, TaskBroker, TemplateFilter, TemplateGlobal, @@ -67,7 +72,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 { PermissionRuleParams } from '@backstage/plugin-permission-common'; import { createConditionAuthorizer, @@ -89,6 +93,7 @@ import { IdentityApiGetIdentityRequest, } from '@backstage/plugin-auth-node'; import { InternalTaskSecrets } from '../scaffolder/tasks/types'; +import { checkPermission } from '../util/checkPermissions'; /** * @@ -407,6 +412,7 @@ export async function createRouter( rules: actionRules, }, ], + permissions: scaffolderTaskPermissions, }); router.use(permissionIntegrationRouter); @@ -463,6 +469,12 @@ export async function createRouter( }); const credentials = await httpAuth.credentials(req); + await checkPermission({ + credentials, + permissions: [taskCreatePermission], + permissionService: permissions, + }); + const { token } = await auth.getPluginRequestToken({ onBehalfOf: credentials, targetPluginId: 'catalog', @@ -539,8 +551,14 @@ export async function createRouter( res.status(201).json({ id: result.taskId }); }) .get('/v2/tasks', async (req, res) => { - const [userEntityRef] = [req.query.createdBy].flat(); + const credentials = await httpAuth.credentials(req); + await checkPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + }); + const [userEntityRef] = [req.query.createdBy].flat(); if ( typeof userEntityRef !== 'string' && typeof userEntityRef !== 'undefined' @@ -561,6 +579,13 @@ export async function createRouter( res.status(200).json(tasks); }) .get('/v2/tasks/:taskId', async (req, res) => { + const credentials = await httpAuth.credentials(req); + await checkPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + }); + const { taskId } = req.params; const task = await taskBroker.get(taskId); if (!task) { @@ -571,11 +596,26 @@ export async function createRouter( res.status(200).json(task); }) .post('/v2/tasks/:taskId/cancel', async (req, res) => { + const credentials = await httpAuth.credentials(req); + // Requires both read and cancel permissions + await checkPermission({ + credentials, + permissions: [taskCancelPermission, taskReadPermission], + permissionService: permissions, + }); + 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); + await checkPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + }); + const { taskId } = req.params; const after = req.query.after !== undefined ? Number(req.query.after) : undefined; @@ -624,6 +664,13 @@ 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, + }); + const { taskId } = req.params; const after = Number(req.query.after) || undefined; @@ -654,6 +701,13 @@ export async function createRouter( }); }) .post('/v2/dry-run', async (req, res) => { + const credentials = await httpAuth.credentials(req); + await checkPermission({ + credentials, + permissions: [taskCreatePermission], + permissionService: permissions, + }); + const bodySchema = z.object({ template: z.unknown(), values: z.record(z.unknown()), @@ -671,8 +725,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-backend/src/util/checkPermissions.ts b/plugins/scaffolder-backend/src/util/checkPermissions.ts new file mode 100644 index 0000000000..b6c0395fbb --- /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, + BasicPermission, +} from '@backstage/plugin-permission-common'; + +export type checkPermissionOptions = { + credentials: BackstageCredentials; + permissions: BasicPermission[]; + 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(permission => ({ + permission, + })); + const authorizationResponses = await permissionService.authorize( + permissionRequest, + { credentials: credentials }, + ); + + for (const response of authorizationResponses) { + if (response.result === AuthorizeResult.DENY) { + throw new NotAllowedError(); + } + } + } +} diff --git a/plugins/scaffolder-common/api-report-alpha.md b/plugins/scaffolder-common/api-report-alpha.md index de9dffd1b9..36b6a7cdb0 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 @@ -19,13 +20,26 @@ export const scaffolderActionPermissions: ResourcePermission<'scaffolder-action' // @alpha export const scaffolderPermissions: ( + | BasicPermission | ResourcePermission<'scaffolder-action'> | ResourcePermission<'scaffolder-template'> )[]; +// @alpha +export const scaffolderTaskPermissions: BasicPermission[]; + // @alpha export const scaffolderTemplatePermissions: ResourcePermission<'scaffolder-template'>[]; +// @alpha +export const taskCancelPermission: BasicPermission; + +// @alpha +export const taskCreatePermission: BasicPermission; + +// @alpha +export const taskReadPermission: BasicPermission; + // @alpha export const templateParameterReadPermission: ResourcePermission<'scaffolder-template'>; diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index 5c4c367889..c441b48d5d 100644 --- a/plugins/scaffolder-common/src/permissions.ts +++ b/plugins/scaffolder-common/src/permissions.ts @@ -79,14 +79,39 @@ export const templateStepReadPermission = createPermission({ }); /** - * List of all the scaffolder permissions + * This permission is used to authorize actions that involve reading one or more tasks in the scaffolder, + * and reading logs of tasks + * * @alpha */ -export const scaffolderPermissions = [ - actionExecutePermission, - templateParameterReadPermission, - templateStepReadPermission, -]; +export const taskReadPermission = createPermission({ + name: 'scaffolder.task.read', + attributes: { + action: 'read', + }, +}); + +/** + * 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', + }, +}); + +/** + * 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: {}, +}); /** * List of the scaffolder permissions that are associated with template steps and parameters. @@ -102,3 +127,23 @@ 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, +]; + +/** + * List of all the scaffolder permissions + * @alpha + */ +export const scaffolderPermissions = [ + ...scaffolderTemplatePermissions, + ...scaffolderActionPermissions, + ...scaffolderTaskPermissions, +]; diff --git a/plugins/scaffolder-react/package.json b/plugins/scaffolder-react/package.json index 0a8d229bfe..0256f47d48 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/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'; /** 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..febdc7c99e 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,9 @@ 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 { 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 +54,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -79,6 +84,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -110,6 +116,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -139,6 +146,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -174,6 +182,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -213,6 +222,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -257,6 +267,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -305,6 +316,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -347,6 +359,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -386,6 +399,7 @@ describe('TemplateCard', () => { storageApi: MockStorageApi.create(), }), ], + [permissionApiRef, new MockPermissionApi()], ]} > @@ -403,4 +417,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 4d4583a7d7..5d2c8aa7fe 100644 --- a/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx +++ b/plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx @@ -39,6 +39,8 @@ import LanguageIcon from '@material-ui/icons/Language'; import React, { useCallback } 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: { @@ -108,6 +110,9 @@ export const TemplateCard = (props: TemplateCardProps) => { !!props.additionalLinks?.length || !!template.metadata.links?.length; const displayDefaultDivider = !hasTags && !hasLinks; + const { allowed: canCreateTask } = usePermission({ + permission: taskCreatePermission, + }); const handleChoose = useCallback(() => { analytics.captureEvent('click', `Template has been opened`); onSelected?.(template); @@ -196,14 +201,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/package.json b/plugins/scaffolder/package.json index 73e231644a..9236784abd 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", 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 1dd6a48f64..f96f83fd2f 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 { useAnalytics, 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; @@ -71,6 +77,21 @@ export const ContextMenu = (props: ContextMenuProps) => { } }); + const { allowed: canCancelTask } = usePermission({ + permission: taskCancelPermission, + }); + + const { allowed: canReadTask } = usePermission({ + permission: taskReadPermission, + }); + + const { allowed: canCreateTask } = usePermission({ + permission: taskCreatePermission, + }); + + // 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'} /> - + @@ -115,7 +140,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 f50b76e14a..f4099028ba 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: { @@ -82,6 +88,22 @@ 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, + }); + + const { allowed: canReadTask } = usePermission({ + permission: taskReadPermission, + }); + + const { allowed: canCreateTask } = usePermission({ + permission: taskCreatePermission, + }); + + // 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); @@ -197,7 +219,11 @@ export const OngoingTask = (props: {
diff --git a/yarn.lock b/yarn.lock index d006daa23c..f7aaf741ac 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5668,6 +5668,7 @@ __metadata: lodash: ^4.17.21 pluralize: ^8.0.0 react-use: ^17.2.4 + swr: ^2.2.5 zen-observable: ^0.10.0 peerDependencies: react: ^16.13.1 || ^17.0.0 || ^18.0.0 @@ -6887,6 +6888,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:^" @@ -6917,6 +6920,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 @@ -6947,6 +6951,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:^" @@ -6983,6 +6988,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 @@ -40994,7 +41000,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: