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: