From d06dfbaeb6fc3d6e03307ae11cf47481aaec5e8d Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Wed, 19 Aug 2026 19:18:56 -0400 Subject: [PATCH] #393 share the reviewer scoping where in lib/auth/scopes.ts buildApplicationWhere/buildApplicationScopeWhere/buildReviewablePositionWhere replace the file-private buildBaseWhere and the two hand-rolled manager clauses in actions/applications.ts; the 'listable' | 'reviewable' argument replaces the per-caller withdrawn patches, and the omitted status key on the scope builder makes overwriting the manager scope a type error rather than a "merge, don't overwrite" comment. /applications search params are now zod-parsed instead of cast. Co-Authored-By: Claude Sonnet 4.6 --- app/(main)/(auth)/applications/page.tsx | 69 +++++++++--------- lib/auth/scopes.ts | 39 ++++++++++ lib/constants.ts | 4 ++ lib/types.ts | 11 ++- prisma/actions/applications.ts | 50 +++---------- prisma/data/applications.ts | 50 ++++--------- tests/db/authorization.test.ts | 94 +++++++++++++++++++++++++ 7 files changed, 201 insertions(+), 116 deletions(-) create mode 100644 lib/auth/scopes.ts diff --git a/app/(main)/(auth)/applications/page.tsx b/app/(main)/(auth)/applications/page.tsx index 99ad5d9e..d4b47ad2 100644 --- a/app/(main)/(auth)/applications/page.tsx +++ b/app/(main)/(auth)/applications/page.tsx @@ -1,5 +1,7 @@ import type { Metadata } from 'next'; +import { z } from 'zod/v4'; + import { getApplications, getApplicationsTotal, @@ -7,14 +9,12 @@ import { } from '@/prisma/data/applications'; import { requireManagerOrAdminOr404 } from '@/lib/auth/guards'; -import { REVIEWER_APPLICATION_STATUSES } from '@/lib/constants'; -import type { - ApplicationFilters, - ApplicationSort, - ApplicationSortDirection, - ApplicationSortField, - ReviewerStatus, -} from '@/lib/types'; +import { + APPLICATION_SORT_DIRECTIONS, + APPLICATION_SORT_FIELDS, + REVIEWER_APPLICATION_STATUSES, +} from '@/lib/constants'; +import type { ApplicationFilters } from '@/lib/types'; import { ApplicationsTable } from '@/components/features/applications-table'; import { ApplicationsToolbar } from '@/components/features/applications-toolbar'; @@ -26,8 +26,26 @@ interface ApplicationsPageProps { searchParams: Promise>; } -const VALID_SORT_FIELDS: ApplicationSortField[] = ['date', 'name', 'status']; -const VALID_SORT_DIRECTIONS: ApplicationSortDirection[] = ['asc', 'desc']; +// A single-value param is a string; a repeated one arrives as string[] — reject both +// with .catch(undefined) rather than throwing, so one bad param never sinks the rest. +const searchParamsSchema = z.object({ + positionId: z.string().trim().min(1).max(64).optional().catch(undefined), + userId: z.string().trim().min(1).max(64).optional().catch(undefined), + status: z.enum(REVIEWER_APPLICATION_STATUSES).optional().catch(undefined), + q: z.string().trim().min(1).max(200).optional().catch(undefined), + sort: z + .string() + .transform((value) => value.split(':')) + .pipe( + z.tuple([ + z.enum(APPLICATION_SORT_FIELDS), + z.enum(APPLICATION_SORT_DIRECTIONS), + ]), + ) + .transform(([field, direction]) => ({ field, direction })) + .optional() + .catch(undefined), +}); export default async function ApplicationsPage({ searchParams, @@ -36,33 +54,14 @@ export default async function ApplicationsPage({ const user = await requireManagerOrAdminOr404(); const sp = await searchParams; - - const rawStatus = typeof sp.status === 'string' ? sp.status : undefined; - const validStatus: ReviewerStatus | undefined = - rawStatus && - (REVIEWER_APPLICATION_STATUSES as readonly string[]).includes(rawStatus) - ? (rawStatus as ReviewerStatus) - : undefined; - - const rawSort = typeof sp.sort === 'string' ? sp.sort : undefined; - let validSort: ApplicationSort | undefined; - if (rawSort) { - const [rawField, rawDir] = rawSort.split(':'); - const field = rawField as ApplicationSortField; - const direction = rawDir as ApplicationSortDirection; - if ( - VALID_SORT_FIELDS.includes(field) && - VALID_SORT_DIRECTIONS.includes(direction) - ) - validSort = { field, direction }; - } + const parsed = searchParamsSchema.parse(sp); const filters: ApplicationFilters = { - positionId: typeof sp.positionId === 'string' ? sp.positionId : undefined, - status: validStatus, - userId: typeof sp.userId === 'string' ? sp.userId : undefined, - q: typeof sp.q === 'string' && sp.q.trim() ? sp.q.trim() : undefined, - sort: validSort, + positionId: parsed.positionId, + status: parsed.status, + userId: parsed.userId, + q: parsed.q, + sort: parsed.sort, }; const hasActiveFilters = !!( diff --git a/lib/auth/scopes.ts b/lib/auth/scopes.ts new file mode 100644 index 00000000..f72f9c30 --- /dev/null +++ b/lib/auth/scopes.ts @@ -0,0 +1,39 @@ +import 'server-only'; + +import type { Prisma } from '@/prisma/client'; + +import { + NON_REVIEWABLE_APPLICATION_STATUSES, + PUBLISHED_POSITION_WHERE, +} from '@/lib/constants'; +import type { Reviewer } from '@/lib/types'; + +// Admin sees every published position; a manager only the ones they manage. +export function buildReviewablePositionWhere( + user: Reviewer, +): Prisma.PositionWhereInput { + return user.isAdmin + ? PUBLISHED_POSITION_WHERE + : { ...PUBLISHED_POSITION_WHERE, managers: { some: { id: user.id } } }; +} + +// `status` omitted so a caller's own filter can't overwrite the position scoping. +export function buildApplicationScopeWhere( + user: Reviewer, +): Omit { + return { deletedAt: null, position: buildReviewablePositionWhere(user) }; +} + +// listable keeps withdrawn rows, reviewable drops them (and draft, in both). +export function buildApplicationWhere( + user: Reviewer, + scope: 'listable' | 'reviewable', +): Prisma.ApplicationWhereInput { + return { + ...buildApplicationScopeWhere(user), + status: + scope === 'reviewable' + ? { notIn: NON_REVIEWABLE_APPLICATION_STATUSES } + : { not: 'draft' }, + }; +} diff --git a/lib/constants.ts b/lib/constants.ts index da7afa0f..5a33c594 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -357,6 +357,10 @@ export const REVIEWER_APPLICATION_STATUSES = [ export const REVIEWER_APPLICATION_STATUS_OPTIONS = APPLICATION_STATUS_OPTIONS.filter((o) => o.value !== 'draft'); +// Single source for the /applications sort union and its zod enum. +export const APPLICATION_SORT_FIELDS = ['date', 'name', 'status'] as const; +export const APPLICATION_SORT_DIRECTIONS = ['asc', 'desc'] as const; + // Single source of truth for the server guard and the rendered quick actions. // Array order is display order; `draft`/`withdrawn` have no reviewer moves. export const APPLICATION_STATUS_TRANSITIONS = { diff --git a/lib/types.ts b/lib/types.ts index abe36788..872c9223 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -7,7 +7,11 @@ import type { } from '@/prisma/client'; import type { $Enums, Prisma } from '@/prisma/client'; -import type { REVIEWER_APPLICATION_STATUSES } from '@/lib/constants'; +import type { + APPLICATION_SORT_DIRECTIONS, + APPLICATION_SORT_FIELDS, + REVIEWER_APPLICATION_STATUSES, +} from '@/lib/constants'; import type { BadgeVariant } from '@/components/ui/badge'; @@ -213,8 +217,9 @@ export type Reviewer = { id: string; isAdmin: boolean }; export type ReviewerStatus = (typeof REVIEWER_APPLICATION_STATUSES)[number]; -export type ApplicationSortField = 'date' | 'name' | 'status'; -export type ApplicationSortDirection = 'asc' | 'desc'; +export type ApplicationSortField = (typeof APPLICATION_SORT_FIELDS)[number]; +export type ApplicationSortDirection = + (typeof APPLICATION_SORT_DIRECTIONS)[number]; export type ApplicationSort = { field: ApplicationSortField; direction: ApplicationSortDirection; diff --git a/prisma/actions/applications.ts b/prisma/actions/applications.ts index daa79bf7..a373755d 100644 --- a/prisma/actions/applications.ts +++ b/prisma/actions/applications.ts @@ -14,14 +14,16 @@ import type { } from '@/prisma/client'; import { requireOwnership } from '@/lib/auth/guards'; +import { + buildApplicationScopeWhere, + buildApplicationWhere, +} from '@/lib/auth/scopes'; import { getCurrentUser } from '@/lib/auth/server'; import { ANSWER_LONG_MAX_LENGTH, ANSWER_MAX_VALUES, APPLICANT_EDITABLE_APPLICATION_STATUSES, APPLICATION_STATUS_LABELS, - NON_REVIEWABLE_APPLICATION_STATUSES, - PUBLISHED_POSITION_WHERE, REVIEWER_APPLICATION_STATUSES, SHORT_ANSWER_FORMAT_ERROR_MESSAGES, TERMINAL_DECISION_STATUSES, @@ -485,26 +487,8 @@ export async function updateApplicationStatus( const { applicationId, status } = parsed.data; // Authorization folded into the query, as in getApplicationForReview. - const where = user.isAdmin - ? { - id: applicationId, - deletedAt: null, - status: { notIn: NON_REVIEWABLE_APPLICATION_STATUSES }, - position: PUBLISHED_POSITION_WHERE, - } - : { - id: applicationId, - deletedAt: null, - status: { notIn: NON_REVIEWABLE_APPLICATION_STATUSES }, - // Merge, don't overwrite — see prisma/data/applications.ts#buildBaseWhere. - position: { - ...PUBLISHED_POSITION_WHERE, - managers: { some: { id: user.id } }, - }, - }; - const application = await prisma.application.findFirst({ - where, + where: { id: applicationId, ...buildApplicationWhere(user, 'reviewable') }, select: { id: true, status: true }, }); @@ -556,26 +540,12 @@ export async function updateApplicationStatuses( // Forward-only sources: a bulk move-back would silently walk an already- // decided row backward, so it's skipped like any other ineligible id. - const where = user.isAdmin - ? { - id: { in: applicationIds }, - deletedAt: null, - status: { in: getApplicationStatusForwardSources(status) }, - position: PUBLISHED_POSITION_WHERE, - } - : { - id: { in: applicationIds }, - deletedAt: null, - status: { in: getApplicationStatusForwardSources(status) }, - // Merge, don't overwrite — see prisma/data/applications.ts#buildBaseWhere. - position: { - ...PUBLISHED_POSITION_WHERE, - managers: { some: { id: user.id } }, - }, - }; - const result = await prisma.application.updateMany({ - where, + where: { + id: { in: applicationIds }, + ...buildApplicationScopeWhere(user), + status: { in: getApplicationStatusForwardSources(status) }, + }, data: { status, updatedById: user.id }, }); diff --git a/prisma/data/applications.ts b/prisma/data/applications.ts index 419a35da..da4a66a2 100644 --- a/prisma/data/applications.ts +++ b/prisma/data/applications.ts @@ -2,6 +2,10 @@ import 'server-only'; import { $Enums, type Prisma } from '@/prisma/client'; +import { + buildApplicationWhere, + buildReviewablePositionWhere, +} from '@/lib/auth/scopes'; import { PUBLISHED_POSITION_WHERE, VISIBLE_POSITION_WHERE, @@ -91,25 +95,6 @@ function normalizeApplicationAnswers(application: ApplicationAnswersPayload): { }; } -// Keeps list, denominator, and detail page agreeing: drafts out, withdrawn in. -function buildBaseWhere(user: Reviewer) { - return user.isAdmin - ? { - deletedAt: null, - status: { not: 'draft' as const }, - position: PUBLISHED_POSITION_WHERE, - } - : { - deletedAt: null, - status: { not: 'draft' as const }, - // Merge, don't overwrite: losing the managers scoping is an authorization regression. - position: { - ...PUBLISHED_POSITION_WHERE, - managers: { some: { id: user.id } }, - }, - }; -} - // Scoped to the caller (no IDOR); returns the caller's application at any status // (draft, withdrawn, or otherwise) so the apply route decides what to render. // Predicate must match createDraftApplication's pre-create lookup, or the page @@ -167,7 +152,7 @@ export async function getApplicationForReview( user: Reviewer, ): Promise { const application = await prisma.application.findFirst({ - where: { id, ...buildBaseWhere(user) }, + where: { id, ...buildApplicationWhere(user, 'listable') }, select: { id: true, status: true, @@ -202,11 +187,7 @@ export async function getApplicationStatusCounts( ): Promise>> { const rows = await prisma.application.groupBy({ by: ['status'], - // buildBaseWhere only excludes draft — withdrawn must be excluded after the spread. - where: { - ...buildBaseWhere(reviewer), - status: { notIn: ['draft', 'withdrawn'] }, - }, + where: buildApplicationWhere(reviewer, 'reviewable'), _count: true, }); @@ -219,11 +200,7 @@ export async function getRecentApplications( take = 10, ): Promise { return prisma.application.findMany({ - // buildBaseWhere only excludes draft — withdrawn must be excluded after the spread. - where: { - ...buildBaseWhere(reviewer), - status: { notIn: ['draft', 'withdrawn'] }, - }, + where: buildApplicationWhere(reviewer, 'reviewable'), select: { id: true, status: true, @@ -242,7 +219,7 @@ export async function getApplications( user: Reviewer, filters: ApplicationFilters, ): Promise { - const baseWhere = buildBaseWhere(user); + const baseWhere = buildApplicationWhere(user, 'listable'); // Prisma DateTime filters are range-based, so a date query becomes a range. const MONTH_NAMES = [ @@ -380,19 +357,16 @@ export async function getMyRecentActivity( } export async function getApplicationsTotal(user: Reviewer): Promise { - return prisma.application.count({ where: buildBaseWhere(user) }); + return prisma.application.count({ + where: buildApplicationWhere(user, 'listable'), + }); } export async function getReviewablePositions( user: Reviewer, ): Promise<{ id: string; title: string }[]> { - // Drafts excluded: a filter shouldn't offer a position with zero visible rows. - const where = user.isAdmin - ? PUBLISHED_POSITION_WHERE - : { ...PUBLISHED_POSITION_WHERE, managers: { some: { id: user.id } } }; - return prisma.position.findMany({ - where, + where: buildReviewablePositionWhere(user), select: { id: true, title: true }, orderBy: { title: 'asc' }, }); diff --git a/tests/db/authorization.test.ts b/tests/db/authorization.test.ts index 774d1875..c96f010d 100644 --- a/tests/db/authorization.test.ts +++ b/tests/db/authorization.test.ts @@ -35,10 +35,12 @@ import type { Application, Position, User } from '@/prisma/client'; import { getApplicationForApply, getApplicationForReview, + getApplicationStatusCounts, getApplications, getApplicationsTotal, getMyApplications, getMySubmittedCount, + getRecentApplications, getReviewablePositions, } from '@/prisma/data/applications'; import { checkPositionAccess, isManager } from '@/prisma/data/managers'; @@ -381,6 +383,45 @@ describe('updateApplicationStatus', () => { }), ).rejects.toThrow('Application not found or not authorized'); }); + + it('throws for the managing manager when the position is draft or soft-deleted', async () => { + const draftPositionApplicant = await createTestUser(); + const onDraftPosition = await createTestApplication( + draftPositionApplicant, + draftPosition, + { status: 'applied' }, + ); + const deletedPositionApplicant = await createTestUser(); + const onDeletedPosition = await createTestApplication( + deletedPositionApplicant, + deletedPosition, + { status: 'applied' }, + ); + + actAs(managerA); + await expect( + updateApplicationStatus({ + applicationId: onDraftPosition.id, + status: 'reviewing', + }), + ).rejects.toThrow('Application not found or not authorized'); + await expect( + updateApplicationStatus({ + applicationId: onDeletedPosition.id, + status: 'reviewing', + }), + ).rejects.toThrow('Application not found or not authorized'); + }); + + it('throws for the managing manager when the application is withdrawn', async () => { + actAs(managerA); + await expect( + updateApplicationStatus({ + applicationId: withdrawnApplicationA.id, + status: 'reviewing', + }), + ).rejects.toThrow('Application not found or not authorized'); + }); }); describe('updateApplicationStatuses', () => { @@ -457,6 +498,59 @@ describe('updateApplicationStatuses', () => { }); expect(stillWithdrawn.status).toBe('withdrawn'); }); + + it('skips a row on a draft or soft-deleted position', async () => { + const draftPositionApplicant = await createTestUser(); + const onDraftPosition = await createTestApplication( + draftPositionApplicant, + draftPosition, + { status: 'applied' }, + ); + const deletedPositionApplicant = await createTestUser(); + const onDeletedPosition = await createTestApplication( + deletedPositionApplicant, + deletedPosition, + { status: 'applied' }, + ); + + actAs(managerA); + const result = await updateApplicationStatuses({ + applicationIds: [onDraftPosition.id, onDeletedPosition.id], + status: 'reviewing', + }); + expect(result).toEqual({ + error: + "None of the selected applications can move to Reviewing — that's only reachable from Applied, Reached out, or Interview scheduled.", + }); + + const stillDraftPosition = await prisma.application.findUniqueOrThrow({ + where: { id: onDraftPosition.id }, + select: { status: true }, + }); + expect(stillDraftPosition.status).toBe('applied'); + }); +}); + +describe('getApplicationStatusCounts / getRecentApplications listable vs reviewable', () => { + it('excludes withdrawn from the reviewable pair while getApplications keeps it', async () => { + const counts = await getApplicationStatusCounts(managerA); + expect(counts.withdrawn).toBeUndefined(); + + const recent = (await getRecentApplications(managerA)).map((a) => a.id); + expect(recent).not.toContain(withdrawnApplicationA.id); + expect(recent).toContain(applicationA1.id); + + const listable = (await getApplications(managerA, {})).map((a) => a.id); + expect(listable).toContain(withdrawnApplicationA.id); + }); + + it('scopes both to the managing manager', async () => { + const recentAsManagerB = (await getRecentApplications(managerB)).map( + (a) => a.id, + ); + expect(recentAsManagerB).not.toContain(applicationA1.id); + expect(recentAsManagerB).toContain(applicationB1.id); + }); }); describe('createOrUpdateApplicationAnswer', () => {