From 9bdd6dedf332418017c415c6dc73d7412dc05f53 Mon Sep 17 00:00:00 2001 From: Kashish Mittal Date: Thu, 13 Mar 2025 11:55:05 -0400 Subject: [PATCH] update tests for Ongoing Tasks Page Signed-off-by: Kashish Mittal --- .changeset/weak-bags-behave.md | 2 - .../scaffolder-backend/src/service/router.ts | 11 +++--- .../src/util/checkPermissions.ts | 30 +++++++++----- .../OngoingTask/OngoingTask.test.tsx | 39 +++++++++++++++++-- 4 files changed, 62 insertions(+), 20 deletions(-) diff --git a/.changeset/weak-bags-behave.md b/.changeset/weak-bags-behave.md index 2c7bc1d37c..0e103cb147 100644 --- a/.changeset/weak-bags-behave.md +++ b/.changeset/weak-bags-behave.md @@ -7,5 +7,3 @@ --- BREAKING : Added two new scaffolder rules 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 creators (`hasCreatedBy`) and granting template owners visibility into all runs of their templates (`hasTemplateEntityRefs`). - -BREAKING: Removed requirement to have both `scaffolder.task.read` and `scaffolder.task.cancel` permissions to cancel tasks. diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 72a22333b9..291fcbdae1 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -646,7 +646,7 @@ export async function createRouter( await checkTaskPermission({ credentials, - permission: taskReadPermission, + permissions: [taskReadPermission], permissionService: permissions, task: task, isTaskAuthorized, @@ -682,9 +682,10 @@ export async function createRouter( try { const credentials = await httpAuth.credentials(req); const task = await taskBroker.get(taskId); + // Requires both read and cancel permissions await checkTaskPermission({ credentials, - permission: taskCancelPermission, + permissions: [taskCancelPermission, taskReadPermission], permissionService: permissions, task: task, isTaskAuthorized, @@ -726,7 +727,7 @@ export async function createRouter( await checkTaskPermission({ credentials, - permission: taskReadPermission, + permissions: [taskReadPermission], permissionService: permissions, task: task, isTaskAuthorized, @@ -774,7 +775,7 @@ export async function createRouter( await checkTaskPermission({ credentials, - permission: taskReadPermission, + permissions: [taskReadPermission], permissionService: permissions, task: task, isTaskAuthorized, @@ -850,7 +851,7 @@ export async function createRouter( await checkTaskPermission({ credentials, - permission: taskReadPermission, + permissions: [taskReadPermission], permissionService: permissions, task: task, isTaskAuthorized, diff --git a/plugins/scaffolder-backend/src/util/checkPermissions.ts b/plugins/scaffolder-backend/src/util/checkPermissions.ts index 950f506e55..08a4ae1b13 100644 --- a/plugins/scaffolder-backend/src/util/checkPermissions.ts +++ b/plugins/scaffolder-backend/src/util/checkPermissions.ts @@ -37,7 +37,7 @@ export type checkPermissionOptions = { export type checkTaskPermissionOptions = { credentials: BackstageCredentials; - permission: ResourcePermission; + permissions: ResourcePermission[]; permissionService?: PermissionsService; task: SerializedTask; isTaskAuthorized: ( @@ -82,18 +82,28 @@ export async function checkPermission(options: checkPermissionOptions) { * @public */ export async function checkTaskPermission(options: checkTaskPermissionOptions) { - const { permission, permissionService, credentials, task, isTaskAuthorized } = - options; + const { + permissions, + permissionService, + credentials, + task, + isTaskAuthorized, + } = options; if (permissionService) { - const [taskDecision] = await permissionService.authorizeConditional( - [{ permission: permission }], + const permissionRequest = permissions.map(permission => ({ + permission, + })); + const authorizationResponses = await permissionService.authorizeConditional( + permissionRequest, { credentials }, ); - if ( - taskDecision.result === AuthorizeResult.DENY || - !isTaskAuthorized(taskDecision, task) - ) { - throw new NotAllowedError(); + for (const response of authorizationResponses) { + if ( + response.result === AuthorizeResult.DENY || + !isTaskAuthorized(response, task) + ) { + throw new NotAllowedError(); + } } } } diff --git a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx index a576b1a9bc..2ef7ddc4d5 100644 --- a/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx +++ b/plugins/scaffolder/src/components/OngoingTask/OngoingTask.test.tsx @@ -161,20 +161,53 @@ 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 render not found error page when user does not have permission to read the task', async () => { const permissionApi = mockApis.permission({ authorize: AuthorizeResult.DENY, }); + + await expect(render(permissionApi)).rejects.toThrow( + 'Reached NotFound Page', + ); + }); + + it('should have cancel button be disabled when user has read permission but lacks cancel permission', async () => { + const permissionApi = mockApis.permission({ + 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'); }); });