From 211bdc6361d4686ed93d9113a472a03950fd4b06 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Wed, 29 Apr 2026 18:33:13 +1000 Subject: [PATCH 1/2] PM-4393: Guard project workspace routes What was broken The previous PM-4393 fixes blocked unauthorized project challenge list and challenge detail URLs, but other project workspace URLs still mounted for users who were not members of the project. QA found TM, PM, and copilot users could open project engagement, engagement detail, users, and asset library URLs and in some cases add project members. Root cause The existing project access check was applied inside challenge pages only. Project-scoped route entries for engagements, users, and assets rendered their child pages before confirming project membership, allowing those pages to fetch and display child project data. What was changed Added a reusable ProjectRouteAccessGuard that checks the current route project with checkProjectAccess before rendering protected route content. Wrapped the project engagement list/detail/application/assignment/feedback/experience routes, project users route, and project asset library route so unauthorized users see the required project access message. Any added/updated tests Added ProjectRouteAccessGuard tests covering allowed access, loading, denied membership, and failed project fetches. --- .../ProjectRouteAccessGuard.spec.tsx | 192 ++++++++++++++++++ .../ProjectRouteAccessGuard.tsx | 76 +++++++ .../ProjectRouteAccessGuard/index.ts | 1 + src/apps/work/src/lib/components/index.ts | 1 + src/apps/work/src/work-app.routes.tsx | 59 +++++- 5 files changed, 319 insertions(+), 10 deletions(-) create mode 100644 src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx create mode 100644 src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx create mode 100644 src/apps/work/src/lib/components/ProjectRouteAccessGuard/index.ts diff --git a/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx new file mode 100644 index 000000000..3b51e79fd --- /dev/null +++ b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx @@ -0,0 +1,192 @@ +/* eslint-disable no-var, global-require, @typescript-eslint/no-var-requires */ +/* eslint-disable import/no-extraneous-dependencies, ordered-imports/ordered-imports */ +import type { + Context, + PropsWithChildren, +} from 'react' +import { + render, + screen, +} from '@testing-library/react' +import { + MemoryRouter, + Route, + Routes, +} from 'react-router-dom' + +import { WorkAppContextModel } from '../../models' +import { useFetchProject } from '../../hooks' +import { checkProjectAccess } from '../../utils' + +import { + PROJECT_ACCESS_DENIED_MESSAGE, + ProjectRouteAccessGuard, +} from './ProjectRouteAccessGuard' + +var mockWorkAppContext: Context + +jest.mock('~/apps/review/src/lib', () => ({ + PageWrapper: ( + props: PropsWithChildren<{ + pageTitle?: string + }>, + ) => ( +
+

{props.pageTitle}

+
{props.children}
+
+ ), +}), { + virtual: true, +}) +jest.mock('~/libs/ui', () => ({ + Button: (props: { label: string }) => ( + + ), + LoadingSpinner: () =>
Loading Spinner
, +}), { + virtual: true, +}) +jest.mock('../../contexts', () => { + const React = require('react') as typeof import('react') + + mockWorkAppContext = React.createContext({ + isAdmin: false, + isAnonymous: false, + isCopilot: false, + isManager: false, + isReadOnly: false, + loginUserInfo: undefined, + userRoles: [], + }) + + return { + WorkAppContext: mockWorkAppContext, + } +}) +jest.mock('../../hooks', () => ({ + useFetchProject: jest.fn(), +})) +jest.mock('../../utils', () => ({ + checkProjectAccess: jest.fn(), +})) + +const mockedUseFetchProject = useFetchProject as jest.Mock +const mockedCheckProjectAccess = checkProjectAccess as jest.Mock + +const defaultContextValue: WorkAppContextModel = { + isAdmin: false, + isAnonymous: false, + isCopilot: false, + isManager: true, + isReadOnly: false, + loginUserInfo: { + email: 'manager@example.com', + exp: 0, + handle: 'manager-user', + iat: 0, + roles: ['Project Manager'], + userId: 12345, + } as WorkAppContextModel['loginUserInfo'], + userRoles: ['Project Manager'], +} + +function renderGuard( + route: string, + contextValue: WorkAppContextModel = defaultContextValue, +): void { + const MockWorkAppContext = mockWorkAppContext + + render( + + + + +
Protected Project Users
+ + )} + /> + + + , + ) +} + +describe('ProjectRouteAccessGuard', () => { + beforeEach(() => { + jest.clearAllMocks() + mockedUseFetchProject.mockReturnValue({ + error: undefined, + isLoading: false, + mutate: jest.fn(), + project: { + id: 200, + members: [{ + userId: 12345, + }], + }, + }) + mockedCheckProjectAccess.mockReturnValue(true) + }) + + it('renders the protected route when project access is allowed', () => { + renderGuard('/projects/200/users') + + expect(mockedCheckProjectAccess) + .toHaveBeenCalledWith(defaultContextValue.userRoles, 12345, expect.objectContaining({ id: 200 })) + expect(screen.getByText('Protected Project Users')) + .toBeTruthy() + }) + + it('shows loading while project access is resolving', () => { + mockedUseFetchProject.mockReturnValue({ + error: undefined, + isLoading: true, + mutate: jest.fn(), + project: undefined, + }) + + renderGuard('/projects/200/users') + + expect(screen.getByRole('heading', { level: 1, name: 'Users' })) + .toBeTruthy() + expect(screen.getByText('Loading Spinner')) + .toBeTruthy() + expect(screen.queryByText('Protected Project Users')) + .toBeNull() + expect(mockedCheckProjectAccess) + .not.toHaveBeenCalled() + }) + + it('shows the project access denial message when project access is rejected', () => { + mockedCheckProjectAccess.mockReturnValue(false) + + renderGuard('/projects/200/users') + + expect(screen.getByRole('heading', { level: 1, name: 'Users' })) + .toBeTruthy() + expect(screen.getByText(PROJECT_ACCESS_DENIED_MESSAGE)) + .toBeTruthy() + expect(screen.queryByText('Protected Project Users')) + .toBeNull() + }) + + it('shows the project access denial message when the project fetch fails', () => { + mockedUseFetchProject.mockReturnValue({ + error: new Error('Forbidden'), + isLoading: false, + mutate: jest.fn(), + project: undefined, + }) + + renderGuard('/projects/200/users') + + expect(screen.getByText(PROJECT_ACCESS_DENIED_MESSAGE)) + .toBeTruthy() + expect(screen.queryByText('Protected Project Users')) + .toBeNull() + }) +}) diff --git a/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx new file mode 100644 index 000000000..3c75a69e3 --- /dev/null +++ b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx @@ -0,0 +1,76 @@ +import { + FC, + PropsWithChildren, + useContext, +} from 'react' +import { useParams } from 'react-router-dom' + +import { PageWrapper } from '~/apps/review/src/lib' + +import { WorkAppContext } from '../../contexts' +import { useFetchProject } from '../../hooks' +import { WorkAppContextModel } from '../../models' +import { checkProjectAccess } from '../../utils' +import { ErrorMessage } from '../ErrorMessage' +import { LoadingSpinner } from '../LoadingSpinner' + +export const PROJECT_ACCESS_DENIED_MESSAGE + = 'You don’t have access to this project. Please contact support@topcoder.com.' + +interface ProjectRouteAccessGuardProps extends PropsWithChildren { + pageTitle: string +} + +/** + * Blocks project-scoped Work routes until the current user has project access. + * + * @param props child route content and fallback page title used while access is loading or denied. + * @returns child route content when the project exists and the caller is an admin or project member. + * @remarks Used by project workspace routes so unauthorized users do not mount pages that fetch project child data. + * @throws Does not throw; project fetch failures render the standard project access denial message. + */ +export const ProjectRouteAccessGuard: FC = ( + props: ProjectRouteAccessGuardProps, +) => { + const params: Readonly<{ projectId?: string }> = useParams<'projectId'>() + const projectId = params.projectId?.trim() + + const workAppContext = useContext(WorkAppContext) as WorkAppContextModel + const projectResult = useFetchProject(projectId || undefined) + + if (!projectId) { + return <>{props.children} + } + + if (projectResult.isLoading) { + return ( + + + + ) + } + + const hasProjectAccess = checkProjectAccess( + workAppContext.userRoles, + workAppContext.loginUserInfo?.userId, + projectResult.project, + ) + + if (projectResult.error || !hasProjectAccess) { + return ( + + + + ) + } + + return <>{props.children} +} + +export default ProjectRouteAccessGuard diff --git a/src/apps/work/src/lib/components/ProjectRouteAccessGuard/index.ts b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/index.ts new file mode 100644 index 000000000..c9831bfb9 --- /dev/null +++ b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/index.ts @@ -0,0 +1 @@ +export * from './ProjectRouteAccessGuard' diff --git a/src/apps/work/src/lib/components/index.ts b/src/apps/work/src/lib/components/index.ts index ee606c570..29122df08 100644 --- a/src/apps/work/src/lib/components/index.ts +++ b/src/apps/work/src/lib/components/index.ts @@ -22,6 +22,7 @@ export * from './Pagination' export * from './ProjectCard' export * from './ProjectBillingAccountExpiredNotice' export * from './ProjectListTabs' +export * from './ProjectRouteAccessGuard' export * from './ProjectStatus' export * from './PaymentFormModal' export * from './PaymentHistoryModal' diff --git a/src/apps/work/src/work-app.routes.tsx b/src/apps/work/src/work-app.routes.tsx index 68758d959..250452f52 100644 --- a/src/apps/work/src/work-app.routes.tsx +++ b/src/apps/work/src/work-app.routes.tsx @@ -39,7 +39,10 @@ import { usersRouteId, } from './config/routes.config' import { WORK_MANAGER_ALLOWED_ROLES } from './config/access.config' -import { ErrorMessage } from './lib/components' +import { + ErrorMessage, + ProjectRouteAccessGuard, +} from './lib/components' import { WorkAppContext } from './lib/contexts' import { WorkAppContextModel } from './lib/models' import { canViewAllEngagements } from './lib/utils' @@ -222,7 +225,11 @@ export const workRoutes: ReadonlyArray = [ }, { authRequired: true, - element: , + element: ( + + + + ), id: projectAssetsRouteId, route: '/projects/:projectId/assets', title: 'Project Assets', @@ -261,48 +268,76 @@ export const workRoutes: ReadonlyArray = [ }, { authRequired: true, - element: , + element: ( + + + + ), route: '/projects/:projectId/engagements', title: 'Engagements', }, { authRequired: true, - element: , + element: ( + + + + ), id: engagementCreateRouteId, route: '/projects/:projectId/engagements/new', title: 'Create Engagement', }, { authRequired: true, - element: , + element: ( + + + + ), id: engagementEditRouteId, route: '/projects/:projectId/engagements/:engagementId', title: 'Edit Engagement', }, { authRequired: true, - element: , + element: ( + + + + ), id: engagementApplicationsRouteId, route: '/projects/:projectId/engagements/:engagementId/applications', title: 'Applications', }, { authRequired: true, - element: , + element: ( + + + + ), id: engagementAssignmentsRouteId, route: '/projects/:projectId/engagements/:engagementId/assignments', title: 'Assignments', }, { authRequired: true, - element: , + element: ( + + + + ), id: engagementFeedbackRouteId, route: '/projects/:projectId/engagements/:engagementId/assignments/:assignmentId/feedback', title: 'Feedback', }, { authRequired: true, - element: , + element: ( + + + + ), id: engagementExperienceRouteId, route: '/projects/:projectId/engagements/:engagementId/assignments/:assignmentId/experience', title: 'Experience', @@ -349,7 +384,11 @@ export const workRoutes: ReadonlyArray = [ }, { authRequired: true, - element: , + element: ( + + + + ), id: usersRouteId, route: '/projects/:projectId/users', title: 'Users', From 2de89ee43cb179709b405b9be43d6b44cc46040c Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 1 May 2026 10:24:25 +1000 Subject: [PATCH 2/2] Fix project guard revalidation access --- .../ProjectRouteAccessGuard.spec.tsx | 27 +++++++++++++++++++ .../ProjectRouteAccessGuard.tsx | 5 ++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx index 3b51e79fd..15862142d 100644 --- a/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx +++ b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.spec.tsx @@ -141,6 +141,30 @@ describe('ProjectRouteAccessGuard', () => { .toBeTruthy() }) + it('renders the protected route when cached project access survives a revalidation error', () => { + mockedUseFetchProject.mockReturnValue({ + error: new Error('Network unavailable'), + isLoading: false, + mutate: jest.fn(), + project: { + id: 200, + members: [{ + userId: 12345, + }], + }, + }) + mockedCheckProjectAccess.mockReturnValue(true) + + renderGuard('/projects/200/users') + + expect(mockedCheckProjectAccess) + .toHaveBeenCalledWith(defaultContextValue.userRoles, 12345, expect.objectContaining({ id: 200 })) + expect(screen.getByText('Protected Project Users')) + .toBeTruthy() + expect(screen.queryByText(PROJECT_ACCESS_DENIED_MESSAGE)) + .toBeNull() + }) + it('shows loading while project access is resolving', () => { mockedUseFetchProject.mockReturnValue({ error: undefined, @@ -181,9 +205,12 @@ describe('ProjectRouteAccessGuard', () => { mutate: jest.fn(), project: undefined, }) + mockedCheckProjectAccess.mockReturnValue(false) renderGuard('/projects/200/users') + expect(mockedCheckProjectAccess) + .toHaveBeenCalledWith(defaultContextValue.userRoles, 12345, undefined) expect(screen.getByText(PROJECT_ACCESS_DENIED_MESSAGE)) .toBeTruthy() expect(screen.queryByText('Protected Project Users')) diff --git a/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx index 3c75a69e3..21dc756f4 100644 --- a/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx +++ b/src/apps/work/src/lib/components/ProjectRouteAccessGuard/ProjectRouteAccessGuard.tsx @@ -27,7 +27,8 @@ interface ProjectRouteAccessGuardProps extends PropsWithChildren { * @param props child route content and fallback page title used while access is loading or denied. * @returns child route content when the project exists and the caller is an admin or project member. * @remarks Used by project workspace routes so unauthorized users do not mount pages that fetch project child data. - * @throws Does not throw; project fetch failures render the standard project access denial message. + * Access decisions use cached project data when available, so SWR revalidation errors do not block authorized users. + * @throws Does not throw; missing project access renders the standard project access denial message. */ export const ProjectRouteAccessGuard: FC = ( props: ProjectRouteAccessGuardProps, @@ -59,7 +60,7 @@ export const ProjectRouteAccessGuard: FC = ( projectResult.project, ) - if (projectResult.error || !hasProjectAccess) { + if (!hasProjectAccess) { return (