diff --git a/.changeset/weak-bags-behave.md b/.changeset/weak-bags-behave.md new file mode 100644 index 0000000000..57bf1903f1 --- /dev/null +++ b/.changeset/weak-bags-behave.md @@ -0,0 +1,13 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-common': minor +'@backstage/plugin-scaffolder-react': minor +'@backstage/plugin-scaffolder-node': minor +'@backstage/plugin-scaffolder': minor +--- + +BREAKING `/alpha`: Converted `scaffolder.task.read` and `scaffolder.task.cancel` into Resource Permissions. + +BREAKING `/alpha`: Added a new scaffolder rule `isTaskOwner` for `scaffolder.task.read` and `scaffolder.task.cancel` to allow for conditional permission policies such as restricting access to tasks and task events based on task creators. + +BREAKING `/alpha`: Retrying a task now requires both `scaffolder.task.read` and `scaffolder.task.create` permissions, replacing the previous requirement of `scaffolder.task.read` and `scaffolder.task.cancel`. diff --git a/docs/features/software-templates/authorizing-scaffolder-template-details.md b/docs/features/software-templates/authorizing-scaffolder-template-details.md index d4798b2540..7ff2cd9377 100644 --- a/docs/features/software-templates/authorizing-scaffolder-template-details.md +++ b/docs/features/software-templates/authorizing-scaffolder-template-details.md @@ -73,7 +73,7 @@ class ExamplePermissionPolicy implements PermissionPolicy { isPermission(request.permission, templateParameterReadPermission) || isPermission(request.permission, templateStepReadPermission) ) { - if (user?.info.userEntityRef === 'user:default/spiderman') + if (user?.info.userEntityRef === 'user:default/bob') return createScaffolderTemplateConditionalDecision(request.permission, { not: scaffolderTemplateConditions.hasTag({ tag: 'secret' }), }); @@ -87,7 +87,7 @@ class ExamplePermissionPolicy implements PermissionPolicy { } ``` -In this example, the user `spiderman` is not authorized to read parameters or steps marked with the `secret` tag. +In this example, the user `bob` is not authorized to read parameters or steps marked with the `secret` tag. By combining this feature with restricting the ingestion of templates in the Catalog as recommended in our threat model, you can create a solid system to restrict certain actions. @@ -113,7 +113,7 @@ class ExamplePermissionPolicy implements PermissionPolicy { ): Promise { /* highlight-add-start */ if (isPermission(request.permission, actionExecutePermission)) { - if (user?.info.userEntityRef === 'user:default/spiderman') { + if (user?.info.userEntityRef === 'user:default/alice') { return createScaffolderActionConditionalDecision(request.permission, { not: scaffolderActionConditions.hasActionId({ actionId: 'debug:log', @@ -130,10 +130,10 @@ class ExamplePermissionPolicy implements PermissionPolicy { } ``` -With this permission policy, the user `spiderman` won't be able to execute the `debug:log` action. +With this permission policy, the user `alice` won't be able to execute the `debug:log` action. You can also restrict the input provided to the action by combining multiple rules. -In the example below, `spiderman` won't be able to execute `debug:log` when passing `{ "message": "not-this!" }` as action input: +In the example below, `alice` won't be able to execute `debug:log` when passing `{ "message": "not-this!" }` as action input: ```ts title="packages/backend/src/plugins/permission.ts" /* highlight-add-start */ @@ -151,7 +151,7 @@ class ExamplePermissionPolicy implements PermissionPolicy { ): Promise { /* highlight-add-start */ if (isPermission(request.permission, actionExecutePermission)) { - if (user?.info.userEntityRef === 'user:default/spiderman') { + if (user?.info.userEntityRef === 'user:default/alice') { return createScaffolderActionConditionalDecision(request.permission, { not: { allOf: [ @@ -176,7 +176,9 @@ 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 these areas of 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. You can configure policies to restrict access to tasks to only their creators, while also granting specific users the permission to view all tasks. + +The following is a simple example of how to do this: ```ts title="packages/src/backend/plugins/permissions.ts" /* highlight-add-start */ @@ -185,6 +187,20 @@ import { taskCreatePermission, taskReadPermission, } from '@backstage/plugin-scaffolder-common/alpha'; +import { + AuthorizeResult, + isPermission, + PolicyDecision, +} from '@backstage/plugin-permission-common'; +import { + PermissionPolicy, + PolicyQuery, + PolicyQueryUser, +} from '@backstage/plugin-permission-node'; +import { + createScaffolderTaskConditionalDecision, + scaffolderTaskConditions, +} from '@backstage/plugin-scaffolder-backend/alpha'; /* highlight-add-end */ class ExamplePermissionPolicy implements PermissionPolicy { @@ -193,26 +209,58 @@ class ExamplePermissionPolicy implements PermissionPolicy { user?: PolicyQueryUser, ): Promise { /* highlight-add-start */ - if (isPermission(request.permission, taskCreatePermission)) { - if (user?.info.userEntityRef === 'user:default/spiderman') { + if (isPermission(request.permission, taskReadPermission)) { + // Allow alice to read any task + if (user?.info.userEntityRef === 'user:default/alice') { return { result: AuthorizeResult.ALLOW, }; } + + // Allow users to read tasks they created + return createScaffolderTaskConditionalDecision( + request.permission, + scaffolderTaskConditions.isTaskOwner({ + createdBy: user?.info.userEntityRef ? [user?.info.userEntityRef] : [], + }), + ); + } + + if (isPermission(request.permission, taskCreatePermission)) { + const userArray = ['user:default/bob', 'user:default/alice']; + const allowed = userArray.some( + allowedUser => user?.info.userEntityRef === allowedUser, + ); + if (allowed) { + return { + result: AuthorizeResult.ALLOW, + }; + } + return { + result: AuthorizeResult.DENY, + }; } if (isPermission(request.permission, taskCancelPermission)) { - if (user?.info.userEntityRef === 'user:default/spiderman') { - return { - result: AuthorizeResult.ALLOW, - }; - } - } - if (isPermission(request.permission, taskReadPermission)) { - if (user?.info.userEntityRef === 'user:default/spiderman') { + // Allow bob to cancel only his tasks + if (user?.info.userEntityRef === 'user:default/bob') { + return createScaffolderTaskConditionalDecision( + request.permission, + scaffolderTaskConditions.isTaskOwner({ + createdBy: user?.info.userEntityRef + ? [user?.info.userEntityRef] + : [], + }), + ); + } + // Allow alice to cancel any task + if (user?.info.userEntityRef === 'user:default/alice') { return { result: AuthorizeResult.ALLOW, }; } + return { + result: AuthorizeResult.DENY, + }; } /* highlight-add-end */ @@ -223,13 +271,21 @@ class ExamplePermissionPolicy implements PermissionPolicy { } ``` -In the provided example permission policy, we only grant the `spiderman` user permissions to perform/access the following actions/resources: +In the provided example permission policy, we only grant the user `bob` permissions to perform/access the following actions/resources: -- Read all scaffolder tasks and their associated events/logs. -- Cancel any ongoing scaffolder tasks. +- Read scaffolder tasks and their associated events/logs for tasks created by `bob`. +- Cancel ongoing scaffolder tasks created by `bob`. - Trigger software templates, which effectively creates new scaffolder tasks. -Any other user would be denied access to these actions/resources. +On the other hand, the user `alice` is granted: + +- Read access to all scaffolder tasks and their associated events/logs. +- The ability to cancel any ongoing scaffolder task. +- The ability to trigger software templates. + +All other users are granted permissions to perform/access the following actions/resources: + +- Read scaffolder tasks and their associated events/logs for tasks created by the user. Although the rules exported by the scaffolder are simple, combining them can help you achieve more complex use cases. diff --git a/plugins/scaffolder-backend/report-alpha.api.md b/plugins/scaffolder-backend/report-alpha.api.md index 958d68fc25..966d71acaa 100644 --- a/plugins/scaffolder-backend/report-alpha.api.md +++ b/plugins/scaffolder-backend/report-alpha.api.md @@ -10,6 +10,8 @@ import { PermissionCondition } from '@backstage/plugin-permission-common'; import { PermissionCriteria } from '@backstage/plugin-permission-common'; import { PermissionRule } from '@backstage/plugin-permission-node'; import { ResourcePermission } from '@backstage/plugin-permission-common'; +import { SerializedTask } from '@backstage/plugin-scaffolder-node'; +import { TaskFilter } from '@backstage/plugin-scaffolder-node'; import { TemplateEntityStepV1beta3 } from '@backstage/plugin-scaffolder-common'; import { TemplateParametersV1beta3 } from '@backstage/plugin-scaffolder-common'; @@ -19,6 +21,12 @@ export const createScaffolderActionConditionalDecision: ( conditions: PermissionCriteria>, ) => ConditionalPolicyDecision; +// @alpha (undocumented) +export const createScaffolderTaskConditionalDecision: ( + permission: ResourcePermission<'scaffolder-task'>, + conditions: PermissionCriteria>, +) => ConditionalPolicyDecision; + // @alpha export const createScaffolderTemplateConditionalDecision: ( permission: ResourcePermission<'scaffolder-template'>, @@ -76,6 +84,18 @@ export const scaffolderActionConditions: Conditions<{ >; }>; +// @alpha +export const scaffolderTaskConditions: Conditions<{ + isTaskOwner: PermissionRule< + SerializedTask, + TaskFilter, + 'scaffolder-task', + { + createdBy: string[]; + } + >; +}>; + // @alpha export const scaffolderTemplateConditions: Conditions<{ hasTag: PermissionRule< diff --git a/plugins/scaffolder-backend/report.api.md b/plugins/scaffolder-backend/report.api.md index 7ece0f84ea..ab1c01ca16 100644 --- a/plugins/scaffolder-backend/report.api.md +++ b/plugins/scaffolder-backend/report.api.md @@ -16,6 +16,7 @@ import { HumanDuration } from '@backstage/types'; import { JsonObject } from '@backstage/types'; import { Knex } from 'knex'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { PermissionRule } from '@backstage/plugin-permission-node'; import { PermissionRuleParams } from '@backstage/plugin-permission-common'; @@ -27,6 +28,7 @@ import { SerializedTaskEvent } from '@backstage/plugin-scaffolder-node'; import { TaskBroker } from '@backstage/plugin-scaffolder-node'; import { TaskCompletionState } from '@backstage/plugin-scaffolder-node'; import { TaskContext } from '@backstage/plugin-scaffolder-node'; +import { TaskFilters } from '@backstage/plugin-scaffolder-node'; import { TaskRecovery } from '@backstage/plugin-scaffolder-common'; import { TaskSecrets } from '@backstage/plugin-scaffolder-node'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; @@ -351,6 +353,7 @@ export class DatabaseTaskStore implements TaskStore { order: 'asc' | 'desc'; field: string; }[]; + permissionFilters?: PermissionCriteria; }): Promise<{ tasks: SerializedTask[]; totalTasks?: number; @@ -486,6 +489,7 @@ export interface TaskStore { limit?: number; offset?: number; }; + permissionFilters?: PermissionCriteria; order?: { order: 'asc' | 'desc'; field: string; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts index f95381e0ed..03a85ad971 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts @@ -25,6 +25,8 @@ import { } from '@backstage/backend-test-utils'; import fs from 'fs-extra'; import { EventsService } from '@backstage/plugin-events-node'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; +import { TaskFilters } from '@backstage/plugin-scaffolder-node'; const createStore = async (events?: EventsService) => { const manager = DatabaseManager.fromConfig( @@ -237,6 +239,56 @@ describe('DatabaseTaskStore', () => { expect(tasks[0].id).toBeDefined(); }); + it('should filter tasks based on permissionFilters', async () => { + const { store } = await createStore(); + + await store.createTask({ + spec: {} as TaskSpec, + createdBy: 'user:default/one', + }); + + await store.createTask({ + spec: {} as TaskSpec, + createdBy: 'user:default/two', + }); + + await store.createTask({ + spec: {} as TaskSpec, + createdBy: 'user:default/three', + }); + + await store.createTask({ + spec: {} as TaskSpec, + createdBy: 'user:default/one', + }); + + await store.createTask({ + spec: {} as TaskSpec, + createdBy: 'user:default/four', + }); + + const permissionFilters: PermissionCriteria = { + not: { + key: 'created_by', + values: ['user:default/three', 'user:default/four'], + }, + }; + + const { tasks, totalTasks } = await store.list({ + permissionFilters: permissionFilters, + }); + + expect(totalTasks).toBe(3); + + const createdByList = tasks.map(task => task.createdBy); + expect(createdByList).toEqual( + expect.arrayContaining(['user:default/one', 'user:default/two']), + ); + expect(createdByList).not.toEqual( + expect.arrayContaining(['user:default/three', 'user:default/four']), + ); + }); + it('should sent an event to start cancelling the task', async () => { const { store } = await createStore(eventsService); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index f81a79a5a0..f07d827f45 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -35,6 +35,7 @@ import { SerializedTask, SerializedTaskEvent, TaskEventType, + TaskFilter, TaskSecrets, TaskStatus, } from '@backstage/plugin-scaffolder-node'; @@ -48,6 +49,14 @@ import { } from '@backstage/plugin-scaffolder-node/alpha'; import { flattenParams } from '../../service/helpers'; import { EventsService } from '@backstage/plugin-events-node'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; +import { + isAndCriteria, + isNotCriteria, + isOrCriteria, +} from '@backstage/plugin-permission-node'; +import { TaskFilters } from '@backstage/plugin-scaffolder-node'; +import { compact } from 'lodash'; const migrationsDir = resolvePackagePath( '@backstage/plugin-scaffolder-backend', @@ -195,6 +204,48 @@ export class DatabaseTaskStore implements TaskStore { } } + private isTaskFilter(filter: any): filter is TaskFilter { + return filter.hasOwnProperty('key'); + } + + private parseFilter( + filter: PermissionCriteria, + query: Knex.QueryBuilder, + db: Knex, + negate: boolean = false, + ): Knex.QueryBuilder { + if (isNotCriteria(filter)) { + return this.parseFilter(filter.not, query, db, !negate); + } + + if (this.isTaskFilter(filter)) { + const values: string[] = compact(filter.values) ?? []; + if (negate) { + query.whereNotIn(filter.key, values); + } else { + query.whereIn(filter.key, values); + } + + return query; + } + + return query[negate ? 'andWhereNot' : 'andWhere'](subQuery => { + if (isOrCriteria(filter)) { + for (const subFilter of filter.anyOf ?? []) { + subQuery.orWhere(subQueryInner => + this.parseFilter(subFilter, subQueryInner, db, false), + ); + } + } else if (isAndCriteria(filter)) { + for (const subFilter of filter.allOf ?? []) { + subQuery.andWhere(subQueryInner => + this.parseFilter(subFilter, subQueryInner, db, false), + ); + } + } + }); + } + async list(options: { createdBy?: string; status?: TaskStatus; @@ -207,16 +258,31 @@ export class DatabaseTaskStore implements TaskStore { offset?: number; }; order?: { order: 'asc' | 'desc'; field: string }[]; + permissionFilters?: PermissionCriteria; }): Promise<{ tasks: SerializedTask[]; totalTasks?: number }> { - const { createdBy, status, pagination, order, filters } = options ?? {}; + const { createdBy, status, pagination, order, filters, permissionFilters } = + options ?? {}; const queryBuilder = this.db('tasks'); - if (createdBy || filters?.createdBy) { - const arr: string[] = flattenParams( - createdBy, - filters?.createdBy, - ); - queryBuilder.whereIn('created_by', [...new Set(arr)]); + const createdByValues = flattenParams( + createdBy, + filters?.createdBy, + ); + + const combinedPermissionFilters: + | PermissionCriteria + | undefined = + createdByValues.length > 0 + ? { + allOf: [ + { key: 'created_by', values: createdByValues }, + ...(permissionFilters ? [permissionFilters] : []), + ], + } + : permissionFilters; + + if (combinedPermissionFilters) { + this.parseFilter(combinedPermissionFilters, queryBuilder, this.db); } if (status || filters?.status) { @@ -271,24 +337,29 @@ export class DatabaseTaskStore implements TaskStore { throw new NotFoundError(`No task with id '${taskId}' found`); } try { - const spec = JSON.parse(result.spec); - const secrets = result.secrets ? JSON.parse(result.secrets) : undefined; - const state = this.getState(result); - return { - id: result.id, - spec, - status: result.status, - lastHeartbeatAt: parseSqlDateToIsoString(result.last_heartbeat_at), - createdAt: parseSqlDateToIsoString(result.created_at), - createdBy: result.created_by ?? undefined, - secrets, - state, - }; + return this.parseTaskRow(result); } catch (error) { throw new Error(`Failed to parse spec of task '${taskId}', ${error}`); } } + private parseTaskRow(result: RawDbTaskRow): SerializedTask { + const spec = JSON.parse(result.spec); + const secrets = result.secrets ? JSON.parse(result.secrets) : undefined; + const state = this.getState(result); + + return { + id: result.id, + spec, + status: result.status, + lastHeartbeatAt: parseSqlDateToIsoString(result.last_heartbeat_at), + createdAt: parseSqlDateToIsoString(result.created_at), + createdBy: result.created_by ?? undefined, + secrets, + state, + }; + } + async createTask( options: TaskStoreCreateTaskOptions, ): Promise { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index c88a1f687c..3f303443a4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -29,6 +29,7 @@ import { TaskBrokerDispatchOptions, TaskCompletionState, TaskContext, + TaskFilters, TaskSecrets, TaskStatus, } from '@backstage/plugin-scaffolder-node'; @@ -42,6 +43,7 @@ import ObservableImpl from 'zen-observable'; import { DefaultWorkspaceService, WorkspaceService } from './WorkspaceService'; import { readDuration } from './helper'; import { InternalTaskSecrets, TaskStore } from './types'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; type TaskState = { checkpoints: CheckpointState; @@ -269,6 +271,7 @@ export class StorageTaskBroker implements TaskBroker { offset?: number; }; order?: { order: 'asc' | 'desc'; field: string }[]; + permissionFilters?: PermissionCriteria; }): Promise<{ tasks: SerializedTask[]; totalTasks?: number }> { if (!this.storage.list) { throw new Error( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index a3d9e3b741..62ffb171ff 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -23,7 +23,9 @@ import { SerializedTaskEvent, SerializedTask, TaskStatus, + TaskFilters, } from '@backstage/plugin-scaffolder-node'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; /** * TaskStoreEmitOptions @@ -131,6 +133,7 @@ export interface TaskStore { limit?: number; offset?: number; }; + permissionFilters?: PermissionCriteria; order?: { order: 'asc' | 'desc'; field: string }[]; }): Promise<{ tasks: SerializedTask[]; totalTasks?: number }>; diff --git a/plugins/scaffolder-backend/src/service/alpha.ts b/plugins/scaffolder-backend/src/service/alpha.ts index 7d22c9d262..ac14f0b1ba 100644 --- a/plugins/scaffolder-backend/src/service/alpha.ts +++ b/plugins/scaffolder-backend/src/service/alpha.ts @@ -17,9 +17,14 @@ import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, RESOURCE_TYPE_SCAFFOLDER_ACTION, + RESOURCE_TYPE_SCAFFOLDER_TASK, } from '@backstage/plugin-scaffolder-common/alpha'; import { createConditionExports } from '@backstage/plugin-permission-node'; -import { scaffolderTemplateRules, scaffolderActionRules } from './rules'; +import { + scaffolderTemplateRules, + scaffolderActionRules, + scaffolderTaskRules, +} from './rules'; const templateConditionExports = createConditionExports({ pluginId: 'scaffolder', @@ -33,6 +38,12 @@ const actionsConditionExports = createConditionExports({ rules: scaffolderActionRules, }); +const taskConditionExports = createConditionExports({ + pluginId: 'scaffolder', + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, + rules: scaffolderTaskRules, +}); + /** * `createScaffolderTemplateConditionalDecision` can be used when authoring policies to * create conditional decisions. It requires a permission of type @@ -90,3 +101,17 @@ export const createScaffolderActionConditionalDecision = * @alpha */ export const scaffolderActionConditions = actionsConditionExports.conditions; + +/** + * @alpha + */ +export const createScaffolderTaskConditionalDecision = + taskConditionExports.createConditionalDecision; + +/** + * These conditions are used when creating conditional decisions for scaffolder + * tasks that are returned by authorization policies. + * + * @alpha + */ +export const scaffolderTaskConditions = taskConditionExports.conditions; diff --git a/plugins/scaffolder-backend/src/service/permissions.ts b/plugins/scaffolder-backend/src/service/permissions.ts index 9b12e115b9..b73cce7a74 100644 --- a/plugins/scaffolder-backend/src/service/permissions.ts +++ b/plugins/scaffolder-backend/src/service/permissions.ts @@ -21,9 +21,20 @@ import { } from '@backstage/plugin-scaffolder-common'; import { RESOURCE_TYPE_SCAFFOLDER_ACTION, + RESOURCE_TYPE_SCAFFOLDER_TASK, RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, } from '@backstage/plugin-scaffolder-common/alpha'; import { PermissionRuleParams } from '@backstage/plugin-permission-common'; +import { SerializedTask } from '@backstage/plugin-scaffolder-node'; + +/** + * + * @public + */ +export type ScaffolderPermissionRuleInput = + | TemplatePermissionRuleInput + | ActionPermissionRuleInput + | TaskPermissionRuleInput; /** * @public @@ -37,7 +48,7 @@ export type TemplatePermissionRuleInput< TParams >; export function isTemplatePermissionRuleInput( - permissionRule: TemplatePermissionRuleInput | ActionPermissionRuleInput, + permissionRule: ScaffolderPermissionRuleInput, ): permissionRule is TemplatePermissionRuleInput { return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_TEMPLATE; } @@ -55,7 +66,27 @@ export type ActionPermissionRuleInput< TParams >; export 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< + SerializedTask, + { + key: string; + values?: string[]; + }, + typeof RESOURCE_TYPE_SCAFFOLDER_TASK, + TParams +>; +export function isTaskPermissionRuleInput( + permissionRule: ScaffolderPermissionRuleInput, +): permissionRule is TaskPermissionRuleInput { + return permissionRule.resourceType === RESOURCE_TYPE_SCAFFOLDER_TASK; +} diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 2820c8b8dc..bebfadb9b1 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -948,6 +948,36 @@ describe('scaffolder router', () => { order: [{ order: 'desc', field: 'created_at' }], }); }); + + it('disallows users from seeing tasks they do not own', async () => { + const { router, taskBroker, permissions } = await createTestRouter(); + jest + .spyOn(permissions, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + conditions: { + resourceType: 'scaffolder-task', + rule: 'IS_TASK_OWNER', + params: { createdBy: ['user'] }, + }, + pluginId: 'scaffolder', + resourceType: 'scaffolder-task', + result: AuthorizeResult.CONDITIONAL, + }, + ]); + const response = await request(router).get( + `/v2/tasks?createdBy=not-user`, + ); + expect(taskBroker.list).toHaveBeenCalledWith({ + filters: { createdBy: ['not-user'], status: undefined }, + order: undefined, + pagination: { limit: undefined, offset: undefined }, + permissionFilters: { key: 'created_by', values: ['user'] }, + }); + expect(response.status).toBe(200); + expect(response.body.totalTasks).toBe(0); + expect(response.body.tasks).toEqual([]); + }); }); describe('GET /v2/tasks/:taskId', () => { @@ -969,11 +999,52 @@ describe('scaffolder router', () => { expect(response.body.status).toBe('completed'); expect(response.body.secrets).toBeUndefined(); }); + it('disallows users from seeing tasks they do not own', async () => { + const { router, permissions, taskBroker } = await createTestRouter(); + jest + .spyOn(permissions, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + conditions: { + resourceType: 'scaffolder-task', + rule: 'IS_TASK_OWNER', + params: { createdBy: ['user'] }, + }, + pluginId: 'scaffolder', + resourceType: 'scaffolder-task', + result: AuthorizeResult.CONDITIONAL, + }, + ]); + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { + __initiatorCredentials: JSON.stringify(credentials), + }, + createdBy: 'not-user', + }); + + const response = await request(router).get(`/v2/tasks/a-random-id`); + expect(taskBroker.get).toHaveBeenCalledWith('a-random-id'); + expect(response.error).not.toBeFalsy(); + }); }); describe('GET /v2/tasks/:taskId/eventstream', () => { it('should return log messages', async () => { const { router, taskBroker } = await createTestRouter(); + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { + __initiatorCredentials: JSON.stringify(credentials), + }, + createdBy: '', + }); let subscriber: ZenObservable.SubscriptionObserver<{ events: SerializedTaskEvent[]; }>; @@ -1059,6 +1130,16 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ it('should return log messages with after query', async () => { const { router, taskBroker } = await createTestRouter(); + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { + __initiatorCredentials: JSON.stringify(credentials), + }, + createdBy: '', + }); let subscriber: ZenObservable.SubscriptionObserver<{ events: SerializedTaskEvent[]; }>; @@ -1123,6 +1204,16 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ describe('GET /v2/tasks/:taskId/events', () => { it('should return log messages', async () => { const { router, taskBroker } = await createTestRouter(); + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { + __initiatorCredentials: JSON.stringify(credentials), + }, + createdBy: '', + }); let subscriber: ZenObservable.SubscriptionObserver<{ events: SerializedTaskEvent[]; }>; @@ -1183,6 +1274,16 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ it('should return log messages with after query', async () => { const { router, taskBroker } = await createTestRouter(); + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { + __initiatorCredentials: JSON.stringify(credentials), + }, + createdBy: '', + }); let subscriber: ZenObservable.SubscriptionObserver<{ events: SerializedTaskEvent[]; }>; @@ -1209,6 +1310,40 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); expect(subscriber!.closed).toBe(true); }); + it('disallows users from seeing events for tasks they do not own', async () => { + const { permissions, router, taskBroker } = await createTestRouter(); + + jest + .spyOn(permissions, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + conditions: { + resourceType: 'scaffolder-task', + rule: 'IS_TASK_OWNER', + params: { createdBy: ['user'] }, + }, + pluginId: 'scaffolder', + resourceType: 'scaffolder-task', + result: AuthorizeResult.CONDITIONAL, + }, + ]); + (taskBroker.get as jest.Mocked['get']).mockResolvedValue({ + id: 'a-random-id', + spec: {} as any, + status: 'completed', + createdAt: '', + secrets: { + __initiatorCredentials: JSON.stringify(credentials), + }, + createdBy: 'not-user', + }); + + const response = await request(router).get( + `/v2/tasks/a-random-id/events`, + ); + expect(taskBroker.get).toHaveBeenCalledWith('a-random-id'); + expect(response.error).not.toBeFalsy(); + }); }); describe('POST /v2/dry-run', () => { @@ -1236,6 +1371,35 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ expect.anything(), ); }); + it('disallows users from seeing tasks they do not own', async () => { + const { permissions, router, taskBroker } = await createTestRouter(); + jest + .spyOn(permissions, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + conditions: { + resourceType: 'scaffolder-task', + rule: 'IS_TASK_OWNER', + params: { createdBy: ['user'] }, + }, + pluginId: 'scaffolder', + resourceType: 'scaffolder-task', + result: AuthorizeResult.CONDITIONAL, + }, + ]); + const response = await request(router).get( + `/v2/tasks?createdBy=not-user`, + ); + expect(taskBroker.list).toHaveBeenCalledWith({ + filters: { createdBy: ['not-user'], status: undefined }, + order: undefined, + pagination: { limit: undefined, offset: undefined }, + permissionFilters: { key: 'created_by', values: ['user'] }, + }); + expect(response.status).toBe(200); + expect(response.body.totalTasks).toBe(0); + expect(response.body.tasks).toEqual([]); + }); }); describe('GET /v2/autocomplete/:provider/:resource', () => { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0ea6293f36..6e476d9a19 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -42,6 +42,8 @@ import { EventsService } from '@backstage/plugin-events-node'; import { createConditionAuthorizer, createPermissionIntegrationRouter, + createConditionTransformer, + ConditionTransformer, } from '@backstage/plugin-permission-node'; import { TaskSpec, @@ -51,7 +53,9 @@ import { import { RESOURCE_TYPE_SCAFFOLDER_ACTION, RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, + RESOURCE_TYPE_SCAFFOLDER_TASK, scaffolderActionPermissions, + scaffolderTaskPermissions, scaffolderPermissions, scaffolderTemplatePermissions, taskCancelPermission, @@ -89,7 +93,11 @@ import { import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { InternalTaskSecrets } from '../scaffolder/tasks/types'; -import { checkPermission } from '../util/checkPermissions'; +import { + checkPermission, + checkTaskPermission, + getAuthorizeConditions, +} from '../util/checkPermissions'; import { findTemplate, getEntityBaseUrl, @@ -97,7 +105,7 @@ import { parseNumberParam, parseStringsParam, } from './helpers'; -import { scaffolderActionRules, scaffolderTemplateRules } from './rules'; + import { convertFiltersToRecord, convertGlobalsToRecord, @@ -107,6 +115,9 @@ import { } from '../util/templating'; import { createDefaultFilters } from '../lib/templating/filters/createDefaultFilters'; import { + ScaffolderPermissionRuleInput, + TaskPermissionRuleInput, + isTaskPermissionRuleInput, ActionPermissionRuleInput, isActionPermissionRuleInput, isTemplatePermissionRuleInput, @@ -114,6 +125,14 @@ import { } from './permissions'; import { CatalogService } from '@backstage/plugin-catalog-node'; +import { + scaffolderActionRules, + scaffolderTemplateRules, + scaffolderTaskRules, +} from './rules'; + +import { TaskFilters } from '@backstage/plugin-scaffolder-node'; + /** * RouterOptions */ @@ -139,9 +158,7 @@ export interface RouterOptions { | CreatedTemplateGlobal[]; additionalWorkspaceProviders?: Record; permissions?: PermissionsService; - permissionRules?: Array< - TemplatePermissionRuleInput | ActionPermissionRuleInput - >; + permissionRules?: Array; auth: AuthService; httpAuth: HttpAuthService; events?: EventsService; @@ -312,15 +329,24 @@ export async function createRouter( const actionRules: ActionPermissionRuleInput[] = Object.values( scaffolderActionRules, ); + const taskRules: TaskPermissionRuleInput[] = + Object.values(scaffolderTaskRules); if (permissionRules) { templateRules.push( ...permissionRules.filter(isTemplatePermissionRuleInput), ); actionRules.push(...permissionRules.filter(isActionPermissionRuleInput)); + taskRules.push(...permissionRules.filter(isTaskPermissionRuleInput)); } - const isAuthorized = createConditionAuthorizer(Object.values(templateRules)); + const isTemplateAuthorized = createConditionAuthorizer( + Object.values(templateRules), + ); + const isTaskAuthorized = createConditionAuthorizer(Object.values(taskRules)); + + const taskTransformConditions: ConditionTransformer = + createConditionTransformer(Object.values(taskRules)); const permissionIntegrationRouter = createPermissionIntegrationRouter({ resources: [ @@ -334,6 +360,11 @@ export async function createRouter( permissions: scaffolderActionPermissions, rules: actionRules, }, + { + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, + permissions: scaffolderTaskPermissions, + rules: taskRules, + }, ], permissions: scaffolderPermissions, }); @@ -532,11 +563,6 @@ export async function createRouter( try { const credentials = await httpAuth.credentials(req); - await checkPermission({ - credentials, - permissions: [taskReadPermission], - permissionService: permissions, - }); if (!taskBroker.list) { throw new Error( @@ -564,6 +590,13 @@ export async function createRouter( const limit = parseNumberParam(req.query.limit, 'limit'); const offset = parseNumberParam(req.query.offset, 'offset'); + const taskPermissionFilters = await getAuthorizeConditions({ + credentials: credentials, + permission: taskReadPermission, + permissionService: permissions, + transformConditions: taskTransformConditions, + }); + const tasks = await taskBroker.list({ filters: { createdBy, @@ -574,6 +607,7 @@ export async function createRouter( limit: limit ? limit[0] : undefined, offset: offset ? offset[0] : undefined, }, + permissionFilters: taskPermissionFilters, }); await auditorEvent?.success(); @@ -598,13 +632,17 @@ export async function createRouter( try { const credentials = await httpAuth.credentials(req); - await checkPermission({ + + const task = await taskBroker.get(taskId); + + await checkTaskPermission({ credentials, permissions: [taskReadPermission], permissionService: permissions, + task: task, + isTaskAuthorized, }); - const task = await taskBroker.get(taskId); if (!task) { throw new NotFoundError(`Task with id ${taskId} does not exist`); } @@ -634,11 +672,14 @@ export async function createRouter( try { const credentials = await httpAuth.credentials(req); + const task = await taskBroker.get(taskId); // Requires both read and cancel permissions - await checkPermission({ + await checkTaskPermission({ credentials, permissions: [taskCancelPermission, taskReadPermission], permissionService: permissions, + task: task, + isTaskAuthorized, }); await taskBroker.cancel?.(taskId); @@ -666,13 +707,23 @@ export async function createRouter( try { const credentials = await httpAuth.credentials(req); - // Requires both read and cancel permissions + const task = await taskBroker.get(taskId); + + // Requires both read and create permissions await checkPermission({ credentials, - permissions: [taskCreatePermission, taskReadPermission], + permissions: [taskCreatePermission], permissionService: permissions, }); + await checkTaskPermission({ + credentials, + permissions: [taskReadPermission], + permissionService: permissions, + task: task, + isTaskAuthorized, + }); + await auditorEvent?.success(); const { token } = await auth.getPluginRequestToken({ @@ -711,10 +762,14 @@ export async function createRouter( try { const credentials = await httpAuth.credentials(req); - await checkPermission({ + const task = await taskBroker.get(taskId); + + await checkTaskPermission({ credentials, permissions: [taskReadPermission], permissionService: permissions, + task: task, + isTaskAuthorized, }); const after = @@ -783,10 +838,14 @@ export async function createRouter( try { const credentials = await httpAuth.credentials(req); - await checkPermission({ + const task = await taskBroker.get(taskId); + + await checkTaskPermission({ credentials, permissions: [taskReadPermission], permissionService: permissions, + task: task, + isTaskAuthorized, }); const after = Number(req.query.after) || undefined; @@ -1023,18 +1082,18 @@ export async function createRouter( // Authorize parameters if (Array.isArray(template.spec.parameters)) { template.spec.parameters = template.spec.parameters.filter(step => - isAuthorized(parameterDecision, step), + isTemplateAuthorized(parameterDecision, step), ); } else if ( template.spec.parameters && - !isAuthorized(parameterDecision, template.spec.parameters) + !isTemplateAuthorized(parameterDecision, template.spec.parameters) ) { template.spec.parameters = undefined; } // Authorize steps template.spec.steps = template.spec.steps.filter(step => - isAuthorized(stepDecision, step), + isTemplateAuthorized(stepDecision, step), ); return template; diff --git a/plugins/scaffolder-backend/src/service/rules.test.ts b/plugins/scaffolder-backend/src/service/rules.test.ts index 8e24cacb32..85e393247a 100644 --- a/plugins/scaffolder-backend/src/service/rules.test.ts +++ b/plugins/scaffolder-backend/src/service/rules.test.ts @@ -22,10 +22,13 @@ import { hasProperty, hasStringProperty, hasTag, + isTaskOwner, } from './rules'; import { createConditionAuthorizer } from '@backstage/plugin-permission-node'; import { RESOURCE_TYPE_SCAFFOLDER_ACTION } from '@backstage/plugin-scaffolder-common/alpha'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { SerializedTask } from '@backstage/plugin-scaffolder-node'; +import { TaskSpec } from '@backstage/plugin-scaffolder-common'; describe('hasTag', () => { describe('apply', () => { @@ -523,3 +526,77 @@ describe('hasStringProperty', () => { ); }); }); + +describe('isTaskOwner', () => { + describe('apply', () => { + const task: SerializedTask = { + id: 'a-random-id', + spec: {} as TaskSpec, + status: 'completed', + createdAt: '', + createdBy: 'user:default/user-1', + }; + it('returns false when createdBy is an empty array', () => { + expect( + isTaskOwner.apply(task, { + createdBy: [], + }), + ).toEqual(false); + }); + it('returns false when createdBy is not matched (single user in createdBy)', () => { + expect( + isTaskOwner.apply(task, { + createdBy: ['not-matched'], + }), + ).toEqual(false); + }); + it('returns true when createdBy matches (single user in createdBy)', () => { + expect( + isTaskOwner.apply(task, { + createdBy: ['user:default/user-1'], + }), + ).toEqual(true); + }); + it('returns false when createdBy is not matched (multiple users in createdBy)', () => { + expect( + isTaskOwner.apply(task, { + createdBy: [ + 'user:default/user-2', + 'user:default/user-3', + 'user:default/user-4', + ], + }), + ).toEqual(false); + }); + it('returns true when createdBy matches (multiple users in createdBy)', () => { + expect( + isTaskOwner.apply(task, { + createdBy: [ + 'user:default/user-1', + 'user:default/user-2', + 'user:default/user-3', + ], + }), + ).toEqual(true); + }); + }); + describe('toQuery', () => { + it('returns the correct query filter with values (single user in createdBy)', () => { + expect( + isTaskOwner.toQuery({ + createdBy: ['user:default/user-1'], + }), + ).toEqual({ key: 'created_by', values: ['user:default/user-1'] }); + }); + }); + it('returns the correct query filter with values (multiple users in createdBy)', () => { + expect( + isTaskOwner.toQuery({ + createdBy: ['user:default/user-1', 'user:default/user-2'], + }), + ).toEqual({ + key: 'created_by', + values: ['user:default/user-1', 'user:default/user-2'], + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts index b197810757..5753668089 100644 --- a/plugins/scaffolder-backend/src/service/rules.ts +++ b/plugins/scaffolder-backend/src/service/rules.ts @@ -15,9 +15,11 @@ */ import { makeCreatePermissionRule } from '@backstage/plugin-permission-node'; + import { RESOURCE_TYPE_SCAFFOLDER_TEMPLATE, RESOURCE_TYPE_SCAFFOLDER_ACTION, + RESOURCE_TYPE_SCAFFOLDER_TASK, } from '@backstage/plugin-scaffolder-common/alpha'; import { @@ -25,6 +27,8 @@ import { TemplateParametersV1beta3, } from '@backstage/plugin-scaffolder-common'; +import { SerializedTask, TaskFilter } from '@backstage/plugin-scaffolder-node'; + import { z } from 'zod'; import { JsonObject, JsonPrimitive } from '@backstage/types'; import { get } from 'lodash'; @@ -129,6 +133,37 @@ function buildHasProperty>({ }); } +export const createTaskPermissionRule = makeCreatePermissionRule< + SerializedTask, + TaskFilter, + typeof RESOURCE_TYPE_SCAFFOLDER_TASK +>(); + +export const isTaskOwner = createTaskPermissionRule({ + name: 'IS_TASK_OWNER', + description: 'Allows tasks created by certain users to be accessible', + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, + paramsSchema: z.object({ + createdBy: z + .array(z.string()) + .describe( + 'List of creater entity refs; only tasks created by these users will be viewable', + ), + }), + apply: (resource, { createdBy }) => { + if (!resource.createdBy) { + return false; + } + return createdBy.includes(resource.createdBy); + }, + toQuery: ({ createdBy }) => { + return { + key: 'created_by', + values: createdBy, + }; + }, +}); + export const scaffolderTemplateRules = { hasTag }; export const scaffolderActionRules = { hasActionId, @@ -136,3 +171,4 @@ export const scaffolderActionRules = { hasNumberProperty, hasStringProperty, }; +export const scaffolderTaskRules = { isTaskOwner }; diff --git a/plugins/scaffolder-backend/src/util/checkPermissions.ts b/plugins/scaffolder-backend/src/util/checkPermissions.ts index b6c0395fbb..08a4ae1b13 100644 --- a/plugins/scaffolder-backend/src/util/checkPermissions.ts +++ b/plugins/scaffolder-backend/src/util/checkPermissions.ts @@ -21,7 +21,13 @@ import { NotAllowedError } from '@backstage/errors'; import { AuthorizeResult, BasicPermission, + PermissionCriteria, + PolicyDecision, + ResourcePermission, } from '@backstage/plugin-permission-common'; +import { ConditionTransformer } from '@backstage/plugin-permission-node'; +import { SerializedTask } from '@backstage/plugin-scaffolder-node'; +import { TaskFilters } from '@backstage/plugin-scaffolder-node'; export type checkPermissionOptions = { credentials: BackstageCredentials; @@ -29,6 +35,24 @@ export type checkPermissionOptions = { permissionService?: PermissionsService; }; +export type checkTaskPermissionOptions = { + credentials: BackstageCredentials; + permissions: ResourcePermission[]; + permissionService?: PermissionsService; + task: SerializedTask; + isTaskAuthorized: ( + decision: PolicyDecision, + resource: SerializedTask | undefined, + ) => boolean; +}; + +export type authorizeConditionsOptions = { + credentials: BackstageCredentials; + permission: ResourcePermission; + permissionService?: PermissionsService; + transformConditions: ConditionTransformer; +}; + /** * Does a basic check on permissions. Throws 403 error if any permission responds with AuthorizeResult.DENY * @public @@ -51,3 +75,55 @@ export async function checkPermission(options: checkPermissionOptions) { } } } + +/** + * Does a conditional permission check for scaffolder task reading and cancellation. + * Throws 403 error if permission responds with AuthorizeResult.DENY, or does not resolve to true during the conditional rule check + * @public + */ +export async function checkTaskPermission(options: checkTaskPermissionOptions) { + const { + permissions, + permissionService, + credentials, + task, + isTaskAuthorized, + } = options; + if (permissionService) { + const permissionRequest = permissions.map(permission => ({ + permission, + })); + const authorizationResponses = await permissionService.authorizeConditional( + permissionRequest, + { credentials }, + ); + for (const response of authorizationResponses) { + if ( + response.result === AuthorizeResult.DENY || + !isTaskAuthorized(response, task) + ) { + throw new NotAllowedError(); + } + } + } +} + +/** Fetches and transforms authorization conditions into filters, or returns `undefined` if the decision is not conditional. + * @public + */ +export const getAuthorizeConditions = async ( + options: authorizeConditionsOptions, +): Promise | undefined> => { + const { permission, permissionService, credentials, transformConditions } = + options; + if (permissionService) { + const [taskDecision] = await permissionService.authorizeConditional( + [{ permission: permission }], + { credentials }, + ); + if (taskDecision.result === AuthorizeResult.CONDITIONAL) { + return transformConditions(taskDecision.conditions); + } + } + return undefined; +}; diff --git a/plugins/scaffolder-common/report-alpha.api.md b/plugins/scaffolder-common/report-alpha.api.md index 97a3de63c5..0bed459cc8 100644 --- a/plugins/scaffolder-common/report-alpha.api.md +++ b/plugins/scaffolder-common/report-alpha.api.md @@ -12,6 +12,9 @@ export const actionExecutePermission: 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,22 +26,26 @@ export const scaffolderPermissions: ( | BasicPermission | ResourcePermission<'scaffolder-action'> | ResourcePermission<'scaffolder-template'> + | ResourcePermission<'scaffolder-task'> )[]; // @alpha -export const scaffolderTaskPermissions: BasicPermission[]; +export const scaffolderTaskPermissions: ( + | BasicPermission + | ResourcePermission<'scaffolder-task'> +)[]; // @alpha export const scaffolderTemplatePermissions: ResourcePermission<'scaffolder-template'>[]; // @alpha -export const taskCancelPermission: BasicPermission; +export const taskCancelPermission: ResourcePermission<'scaffolder-task'>; // @alpha export const taskCreatePermission: BasicPermission; // @alpha -export const taskReadPermission: BasicPermission; +export const taskReadPermission: ResourcePermission<'scaffolder-task'>; // @alpha export const templateManagementPermission: BasicPermission; diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts index e758291e75..36c72072e9 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 scaffolder tasks + * + * @alpha + */ +export const RESOURCE_TYPE_SCAFFOLDER_TASK = 'scaffolder-task'; + /** * This permission is used to authorize actions that involve executing * an action from a template. @@ -89,6 +96,7 @@ export const taskReadPermission = createPermission({ attributes: { action: 'read', }, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, }); /** @@ -111,6 +119,7 @@ export const taskCreatePermission = createPermission({ export const taskCancelPermission = createPermission({ name: 'scaffolder.task.cancel', attributes: {}, + resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK, }); /** diff --git a/plugins/scaffolder-node/package.json b/plugins/scaffolder-node/package.json index 9483dd1aee..86dae0b48b 100644 --- a/plugins/scaffolder-node/package.json +++ b/plugins/scaffolder-node/package.json @@ -58,6 +58,7 @@ "@backstage/catalog-model": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/integration": "workspace:^", + "@backstage/plugin-permission-common": "workspace:^", "@backstage/plugin-scaffolder-common": "workspace:^", "@backstage/types": "workspace:^", "@isomorphic-git/pgp-plugin": "^0.0.7", diff --git a/plugins/scaffolder-node/report.api.md b/plugins/scaffolder-node/report.api.md index bceb9980b6..d0fd036f29 100644 --- a/plugins/scaffolder-node/report.api.md +++ b/plugins/scaffolder-node/report.api.md @@ -10,6 +10,7 @@ import { JsonObject } from '@backstage/types'; import { JsonValue } from '@backstage/types'; import { LoggerService } from '@backstage/backend-plugin-api'; import { Observable } from '@backstage/types'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; import { Schema } from 'jsonschema'; import { ScmIntegrationRegistry } from '@backstage/integration'; import { ScmIntegrations } from '@backstage/integration'; @@ -374,6 +375,7 @@ export interface TaskBroker { order: 'asc' | 'desc'; field: string; }[]; + permissionFilters?: PermissionCriteria; }): Promise<{ tasks: SerializedTask[]; totalTasks?: number; @@ -453,6 +455,25 @@ export interface TaskContext { // @public export type TaskEventType = 'completion' | 'log' | 'cancelled' | 'recovered'; +// @public +export type TaskFilter = { + key: string; + values?: string[]; +}; + +// @public +export type TaskFilters = + | { + anyOf: TaskFilter[]; + } + | { + allOf: TaskFilter[]; + } + | { + not: TaskFilter; + } + | TaskFilter; + // @public export type TaskSecrets = Record & { backstageToken?: string; diff --git a/plugins/scaffolder-node/src/tasks/index.ts b/plugins/scaffolder-node/src/tasks/index.ts index 930de95237..7e93e84cc0 100644 --- a/plugins/scaffolder-node/src/tasks/index.ts +++ b/plugins/scaffolder-node/src/tasks/index.ts @@ -18,6 +18,8 @@ export type { TaskSecrets, SerializedTask, SerializedTaskEvent, + TaskFilter, + TaskFilters, TaskBroker, TaskBrokerDispatchOptions, TaskBrokerDispatchResult, diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index d2403f205a..b55a2729b7 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -15,6 +15,7 @@ */ import { BackstageCredentials } from '@backstage/backend-plugin-api'; +import { PermissionCriteria } from '@backstage/plugin-permission-common'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { JsonObject, Observable } from '@backstage/types'; import { UpdateTaskCheckpointOptions } from '@backstage/plugin-scaffolder-node/alpha'; @@ -105,6 +106,25 @@ export type TaskBrokerDispatchOptions = { createdBy?: string; }; +/** + * TaskFilter + * @public + */ +export type TaskFilter = { + key: string; + values?: string[]; +}; + +/** + * TaskFilters + * @public + */ +export type TaskFilters = + | { anyOf: TaskFilter[] } + | { allOf: TaskFilter[] } + | { not: TaskFilter } + | TaskFilter; + /** * Task * @@ -183,6 +203,7 @@ export interface TaskBroker { offset?: number; }; order?: { order: 'asc' | 'desc'; field: string }[]; + permissionFilters?: PermissionCriteria; }): Promise<{ tasks: SerializedTask[]; totalTasks?: number }>; /** diff --git a/plugins/scaffolder-react/src/next/components/ScaffolderPageContextMenu/ScaffolderPageContextMenu.tsx b/plugins/scaffolder-react/src/next/components/ScaffolderPageContextMenu/ScaffolderPageContextMenu.tsx index 9546792983..2df4ff3b71 100644 --- a/plugins/scaffolder-react/src/next/components/ScaffolderPageContextMenu/ScaffolderPageContextMenu.tsx +++ b/plugins/scaffolder-react/src/next/components/ScaffolderPageContextMenu/ScaffolderPageContextMenu.tsx @@ -30,7 +30,6 @@ import Functions from '@material-ui/icons/Functions'; import MoreVert from '@material-ui/icons/MoreVert'; import { SyntheticEvent, useState } from 'react'; import { usePermission } from '@backstage/plugin-permission-react'; -import { taskReadPermission } from '@backstage/plugin-scaffolder-common/alpha'; import { templateManagementPermission } from '@backstage/plugin-scaffolder-common/alpha'; import { scaffolderReactTranslationRef } from '../../../translation'; @@ -69,10 +68,6 @@ export function ScaffolderPageContextMenu( const classes = useStyles(); const [anchorEl, setAnchorEl] = useState(); - const { allowed: canReadTasks } = usePermission({ - permission: taskReadPermission, - }); - const { allowed: canManageTemplates } = usePermission({ permission: templateManagementPermission, }); @@ -164,7 +159,7 @@ export function ScaffolderPageContextMenu( /> )} - {onTasksClicked && canReadTasks && ( + {onTasksClicked && ( diff --git a/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx b/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx index 6d48cec125..d6b5b7116e 100644 --- a/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx +++ b/plugins/scaffolder/src/components/ListTasksPage/ListTasksPage.tsx @@ -91,14 +91,22 @@ const ListTaskPageContent = (props: MyTaskPageProps) => { if (error) { return ( - <> - - - + + + setOwnerFilter(id)} + /> + + + + + + ); } diff --git a/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx b/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx index e1a5f37ddc..2df1fc4ddb 100644 --- a/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx @@ -46,6 +46,7 @@ type ContextMenuProps = { onStartOver?: () => void; onToggleLogs?: (state: boolean) => void; onToggleButtonBar?: (state: boolean) => void; + taskId?: string; isCancelButtonDisabled: boolean; onCancel: () => void; }; @@ -67,6 +68,7 @@ export const ContextMenu = (props: ContextMenuProps) => { onStartOver, onToggleLogs, onToggleButtonBar, + taskId, } = props; const { getPageTheme } = useTheme(); const pageTheme = getPageTheme({ themeId: 'website' }); @@ -76,6 +78,7 @@ export const ContextMenu = (props: ContextMenuProps) => { const { allowed: canReadTask } = usePermission({ permission: taskReadPermission, + resourceRef: taskId, }); const { allowed: canCreateTask } = usePermission({ diff --git a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx index a576b1a9bc..5e44d5cf43 100644 --- a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx @@ -161,20 +161,43 @@ 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 () => { + it('should have cancel button be disabled when user has read permission but lacks cancel permission', async () => { const permissionApi = mockApis.permission({ - authorize: AuthorizeResult.DENY, + authorize: request => { + if (request.permission.name === 'scaffolder.task.cancel') { + return AuthorizeResult.DENY; + } + return AuthorizeResult.ALLOW; + }, }); 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'); + }); + + it('should have start over button be disabled when user has read permission but lacks create permission', async () => { + const permissionApi = mockApis.permission({ + authorize: request => { + if (request.permission.name === 'scaffolder.task.create') { + return AuthorizeResult.DENY; + } + return AuthorizeResult.ALLOW; + }, + }); + const rendered = await render(permissionApi); + + const { getByTestId } = rendered; + expect(getByTestId('start-over-button')).toHaveClass('Mui-disabled'); + + await act(async () => { + fireEvent.click(getByTestId('menu-button')); + }); + expect(getByTestId('start-over-button')).toHaveClass('Mui-disabled'); }); }); diff --git a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx index c9a119e4a1..bc0d5fe1aa 100644 --- a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx @@ -139,13 +139,14 @@ function OngoingTaskContent(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: taskId, }); const { allowed: canReadTask } = usePermission({ permission: taskReadPermission, + resourceRef: taskId, }); const { allowed: canCreateTask } = usePermission({ @@ -269,6 +270,7 @@ function OngoingTaskContent(props: { onRetry={triggerRetry} onToggleLogs={setLogVisibleState} onToggleButtonBar={setButtonBarVisibleState} + taskId={taskId} onCancel={triggerCancel} isCancelButtonDisabled={isCancelButtonDisabled} /> diff --git a/plugins/scaffolder/src/components/Router/Router.tsx b/plugins/scaffolder/src/components/Router/Router.tsx index 91567c1ddd..d8dee73849 100644 --- a/plugins/scaffolder/src/components/Router/Router.tsx +++ b/plugins/scaffolder/src/components/Router/Router.tsx @@ -60,10 +60,7 @@ import { CustomFieldsPage, } from '../../alpha/components/TemplateEditorPage'; import { RequirePermission } from '@backstage/plugin-permission-react'; -import { - taskReadPermission, - templateManagementPermission, -} from '@backstage/plugin-scaffolder-common/alpha'; +import { templateManagementPermission } from '@backstage/plugin-scaffolder-common/alpha'; import { useApp } from '@backstage/core-plugin-api'; import { FormField, OpaqueFormField } from '@internal/scaffolder'; import { useAsync, useMountEffect } from '@react-hookz/web'; @@ -182,11 +179,9 @@ export const InternalRouter = ( - - + } /> - - - } + element={} />