From bd91f00f03e156abf32d6edfc0119e92d905008c Mon Sep 17 00:00:00 2001 From: Rayna-Yu Date: Tue, 30 Jun 2026 18:55:24 -0400 Subject: [PATCH 1/2] add approve/decline --- apps/backend/lambdas/expenditures/README.md | 1 + apps/backend/lambdas/expenditures/handler.ts | 69 +++++++++- .../backend/lambdas/expenditures/openapi.yaml | 34 +++++ .../test/expenditures.e2e.test.ts | 90 +++++++++++- .../test/expenditures.unit.test.ts | 128 +++++++++++++++++- .../lambdas/expenditures/validation-utils.ts | 15 ++ infrastructure/aws/api_gateway.tf | 2 +- 7 files changed, 333 insertions(+), 6 deletions(-) diff --git a/apps/backend/lambdas/expenditures/README.md b/apps/backend/lambdas/expenditures/README.md index cf1ca79e..c9f60225 100644 --- a/apps/backend/lambdas/expenditures/README.md +++ b/apps/backend/lambdas/expenditures/README.md @@ -11,6 +11,7 @@ Lambda for tracking project expenditures. | GET | /health | Health check | | GET | /expenditures | | | POST | /expenditures | | +| PATCH | /expenditures/{id}/status | | ## Setup diff --git a/apps/backend/lambdas/expenditures/handler.ts b/apps/backend/lambdas/expenditures/handler.ts index 53c2a859..4fb37487 100644 --- a/apps/backend/lambdas/expenditures/handler.ts +++ b/apps/backend/lambdas/expenditures/handler.ts @@ -1,7 +1,16 @@ import { APIGatewayProxyResult } from 'aws-lambda'; import db from './db'; import { ExpenditureValidationUtils } from './validation-utils'; -import { authenticateRequest } from './auth'; +import { authenticateRequest, checkAuthorization, AuthContext } from './auth'; + +function requireAuth(authContext: AuthContext, level: Parameters[1], resourceUserId?: number | string): APIGatewayProxyResult | undefined { + const authCheck = checkAuthorization(authContext, level, resourceUserId); + if (!authCheck.allowed) { + return authContext.isAuthenticated + ? json(403, { message: authCheck.reason || 'Forbidden' }) + : json(401, { message: 'Authentication required' }); + } +} export const handler = async (event: any): Promise => { try { @@ -166,7 +175,61 @@ export const handler = async (event: any): Promise => { }, }); } - // <<< ROUTES-END + + // PATCH /expenditures/{id}/status — approve/decline (admin only) + // (dev server strips the /expenditures prefix, so match the trailing /{id}/status) + const statusSegments = normalizedPath.split('/').filter(Boolean); + if ((statusSegments.length >= 2 && statusSegments[statusSegments.length - 1] === 'status') && method === 'PATCH') { + const authContext = await authenticateRequest(event); + const authError = requireAuth(authContext, 'ADMIN'); + if (authError) return authError; + + const id = statusSegments[statusSegments.length - 2]; + if (!/^\d+$/.test(id) || parseInt(id, 10) < 1) { + return json(400, { message: 'id must be a positive integer' }); + } + + const body = event.body ? JSON.parse(event.body) as Record : {}; + + // Only 'approved' or 'denied' may be set through this endpoint + const statusResult = ExpenditureValidationUtils.validateApprovalStatus(body.status); + if (statusResult instanceof Error) { + return json(400, { message: statusResult.message }); + } + + // make sure expenditure exists + const expenditure = await db + .selectFrom('branch.expenditures') + .where('expenditure_id', '=', Number(id)) + .selectAll() + .executeTakeFirst(); + + if (!expenditure) { + return json(404, { message: 'Expenditure not found' }); + } + + // update + await db + .updateTable('branch.expenditures') + .set({ status: statusResult }) + .where('expenditure_id', '=', Number(id)) + .execute(); + + // get updated expenditure + const updated = await db + .selectFrom('branch.expenditures') + .where('expenditure_id', '=', Number(id)) + .selectAll() + .executeTakeFirst(); + + return json(200, { + ok: true, + route: 'PATCH /expenditures/{id}/status', + pathParams: { id }, + body: { expenditureId: updated!.expenditure_id, status: updated!.status }, + }); + } + // <<< ROUTES-END return json(404, { message: 'Not Found', path: normalizedPath, method }); } catch (err) { @@ -182,7 +245,7 @@ function json(statusCode: number, body: unknown): APIGatewayProxyResult { 'Content-Type': 'application/json', 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Headers': 'Content-Type,Authorization', - 'Access-Control-Allow-Methods': 'GET,POST,PUT,DELETE,OPTIONS' + 'Access-Control-Allow-Methods': 'GET,POST,PUT,PATCH,DELETE,OPTIONS' }, body: JSON.stringify(body) }; diff --git a/apps/backend/lambdas/expenditures/openapi.yaml b/apps/backend/lambdas/expenditures/openapi.yaml index a42f5b9b..1a8f88bc 100644 --- a/apps/backend/lambdas/expenditures/openapi.yaml +++ b/apps/backend/lambdas/expenditures/openapi.yaml @@ -101,3 +101,37 @@ paths: responses: '201': description: Success + + /expenditures/{id}/status: + patch: + summary: PATCH /expenditures/{id}/status + description: Approve or decline an expenditure. Admin only. + parameters: + - in: path + name: id + required: true + schema: + type: string + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - status + properties: + status: + type: string + enum: [approved, denied] + responses: + '200': + description: Expenditure status updated + '400': + description: Invalid id or status + '401': + description: Authentication required + '403': + description: Admin access required + '404': + description: Expenditure not found diff --git a/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts b/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts index ed41f432..ad38357a 100644 --- a/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts +++ b/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts @@ -7,9 +7,24 @@ import { Pool } from 'pg'; jest.mock('../auth'); import { handler } from '../handler'; -import { authenticateRequest } from '../auth'; +import { authenticateRequest, checkAuthorization } from '../auth'; const mockAuthenticateRequest = authenticateRequest as jest.MockedFunction; +const mockCheckAuthorization = checkAuthorization as jest.MockedFunction; + +mockCheckAuthorization.mockImplementation((authContext, requiredAccess, resourceUserId?) => { + if (requiredAccess === 'PUBLIC') return { allowed: true }; + if (!authContext.isAuthenticated || !authContext.user) return { allowed: false, reason: 'Authentication required' }; + if (requiredAccess === 'ADMIN') { + const isAdmin = authContext.user.isAdmin ?? false; + return { allowed: isAdmin, reason: isAdmin ? undefined : 'Admin access required' }; + } + if (requiredAccess === 'ADMIN_OR_SELF') { + const allowed = (authContext.user.isAdmin ?? false) || authContext.user.userId === Number(resourceUserId); + return { allowed, reason: allowed ? undefined : 'Admin access or resource ownership required' }; + } + return { allowed: false, reason: 'Unknown access level' }; +}); const pool = new Pool({ host: 'localhost', @@ -42,6 +57,15 @@ function getEvent(path: string, queryStringParameters?: Record) }; } +function patchStatusEvent(id: number | string, body: Record) { + return { + rawPath: `/expenditures/${id}/status`, + requestContext: { http: { method: 'PATCH' } }, + headers: { Authorization: 'Bearer fake-token' }, + body: JSON.stringify(body), + }; +} + // Auth contexts based on seed data const adminUser = { isAuthenticated: true as const, @@ -402,4 +426,68 @@ describe('Expenditures integration tests', () => { expect(res.statusCode).toBe(400); }); }); + + describe('PATCH /expenditures/{id}/status — approve/decline', () => { + async function getStatus(id: number): Promise { + const result = await pool.query('SELECT status FROM branch.expenditures WHERE expenditure_id = $1', [id]); + return result.rows[0]?.status; + } + + test('200: admin approves a pending expenditure', async () => { + mockAuthenticateRequest.mockResolvedValue(adminUser); + const res = await handler(patchStatusEvent(1, { status: 'approved' })); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).body.status).toBe('approved'); + // confirms it persisted to the database + expect(await getStatus(1)).toBe('approved'); + }); + + test('200: admin declines a pending expenditure', async () => { + mockAuthenticateRequest.mockResolvedValue(adminUser); + const res = await handler(patchStatusEvent(1, { status: 'denied' })); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).body.status).toBe('denied'); + expect(await getStatus(1)).toBe('denied'); + }); + + test('401: unauthenticated request is rejected', async () => { + mockAuthenticateRequest.mockResolvedValue({ isAuthenticated: false }); + const res = await handler(patchStatusEvent(1, { status: 'approved' })); + + expect(res.statusCode).toBe(401); + expect(await getStatus(1)).toBe('pending'); + }); + + test('403: non-admin user is rejected', async () => { + mockAuthenticateRequest.mockResolvedValue(staffUser); + const res = await handler(patchStatusEvent(1, { status: 'approved' })); + + expect(res.statusCode).toBe(403); + expect(await getStatus(1)).toBe('pending'); + }); + + test('404: expenditure not found', async () => { + mockAuthenticateRequest.mockResolvedValue(adminUser); + const res = await handler(patchStatusEvent(9999, { status: 'approved' })); + + expect(res.statusCode).toBe(404); + }); + + test('400: status not approved/denied is rejected', async () => { + mockAuthenticateRequest.mockResolvedValue(adminUser); + const res = await handler(patchStatusEvent(1, { status: 'pending' })); + + expect(res.statusCode).toBe(400); + expect(await getStatus(1)).toBe('pending'); + }); + + test('400: invalid id is rejected', async () => { + mockAuthenticateRequest.mockResolvedValue(adminUser); + const res = await handler(patchStatusEvent('abc', { status: 'approved' })); + + expect(res.statusCode).toBe(400); + }); + }); }); diff --git a/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts b/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts index 1ec35a8b..6c469a03 100644 --- a/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts +++ b/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts @@ -6,10 +6,25 @@ jest.mock('../auth'); import { handler } from '../handler'; import db from '../db'; -import { authenticateRequest } from '../auth'; +import { authenticateRequest, checkAuthorization } from '../auth'; const mockDb = db as any; const mockAuthenticateRequest = authenticateRequest as jest.MockedFunction; +const mockCheckAuthorization = checkAuthorization as jest.MockedFunction; + +mockCheckAuthorization.mockImplementation((authContext, requiredAccess, resourceUserId?) => { + if (requiredAccess === 'PUBLIC') return { allowed: true }; + if (!authContext.isAuthenticated || !authContext.user) return { allowed: false, reason: 'Authentication required' }; + if (requiredAccess === 'ADMIN') { + const isAdmin = authContext.user.isAdmin ?? false; + return { allowed: isAdmin, reason: isAdmin ? undefined : 'Admin access required' }; + } + if (requiredAccess === 'ADMIN_OR_SELF') { + const allowed = (authContext.user.isAdmin ?? false) || authContext.user.userId === Number(resourceUserId); + return { allowed, reason: allowed ? undefined : 'Admin access or resource ownership required' }; + } + return { allowed: false, reason: 'Unknown access level' }; +}); // Helper function to create a POST event function postEvent(body: Record) { @@ -590,3 +605,114 @@ describe('POST /expenditures unit tests', () => { }); }); }); + +describe('PATCH /expenditures/{id}/status unit tests', () => { + function patchStatusEvent(id: string | number, body: unknown) { + return { + rawPath: `/expenditures/${id}/status`, + requestContext: { http: { method: 'PATCH' } }, + headers: { Authorization: 'Bearer fake-token' }, + body: typeof body === 'string' ? body : JSON.stringify(body), + }; + } + + // Sets up selectFrom (existing + updated lookups) and updateTable chains. + function mockExpenditureForPatch(existing: Record | null, updated?: Record) { + mockDb.selectFrom.mockReturnValue({ + where: jest.fn().mockReturnValue({ + selectAll: jest.fn().mockReturnValue({ + executeTakeFirst: (jest.fn() as any) + .mockResolvedValueOnce(existing) + .mockResolvedValueOnce(updated ?? existing), + }), + }), + }); + + mockDb.updateTable.mockReturnValue({ + set: jest.fn().mockReturnValue({ + where: jest.fn().mockReturnValue({ + execute: (jest.fn() as any).mockResolvedValue(undefined), + }), + }), + }); + } + + beforeEach(() => { + jest.clearAllMocks(); + mockAuthenticateRequest.mockResolvedValue(adminAuthContext); + }); + + test('200: admin approves an expenditure', async () => { + mockExpenditureForPatch( + { expenditure_id: 5, status: 'pending' }, + { expenditure_id: 5, status: 'approved' }, + ); + + const res = await handler(patchStatusEvent(5, { status: 'approved' })); + + expect(res.statusCode).toBe(200); + const json = JSON.parse(res.body); + expect(json.ok).toBe(true); + expect(json.pathParams).toEqual({ id: '5' }); + expect(json.body.status).toBe('approved'); + }); + + test('200: admin declines an expenditure', async () => { + mockExpenditureForPatch( + { expenditure_id: 5, status: 'pending' }, + { expenditure_id: 5, status: 'denied' }, + ); + + const res = await handler(patchStatusEvent(5, { status: 'denied' })); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).body.status).toBe('denied'); + }); + + test('401: unauthenticated request', async () => { + mockAuthenticateRequest.mockResolvedValue({ isAuthenticated: false }); + + const res = await handler(patchStatusEvent(5, { status: 'approved' })); + + expect(res.statusCode).toBe(401); + }); + + test('403: authenticated non-admin is rejected', async () => { + mockAuthenticateRequest.mockResolvedValue({ + isAuthenticated: true, + user: { cognitoSub: 'staff-sub', userId: 2, email: 'staff@example.com', isAdmin: false }, + }); + + const res = await handler(patchStatusEvent(5, { status: 'approved' })); + + expect(res.statusCode).toBe(403); + expect(JSON.parse(res.body).message).toContain('Admin'); + }); + + test('400: invalid id', async () => { + const res = await handler(patchStatusEvent('abc', { status: 'approved' })); + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.body).message).toContain('id'); + }); + + test('400: missing status', async () => { + const res = await handler(patchStatusEvent(5, {})); + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.body).message).toContain('status is required'); + }); + + test('400: status not approved/denied (e.g. pending)', async () => { + const res = await handler(patchStatusEvent(5, { status: 'pending' })); + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.body).message).toContain('status must be one of'); + }); + + test('404: expenditure not found', async () => { + mockExpenditureForPatch(null); + + const res = await handler(patchStatusEvent(999, { status: 'approved' })); + + expect(res.statusCode).toBe(404); + expect(JSON.parse(res.body).message).toContain('not found'); + }); +}); diff --git a/apps/backend/lambdas/expenditures/validation-utils.ts b/apps/backend/lambdas/expenditures/validation-utils.ts index f80c6227..9a34b0c9 100644 --- a/apps/backend/lambdas/expenditures/validation-utils.ts +++ b/apps/backend/lambdas/expenditures/validation-utils.ts @@ -1,6 +1,9 @@ export const EXPENDITURE_STATUSES = ['approved', 'pending', 'denied', 'needs_more_info'] as const; export type ExpenditureStatus = typeof EXPENDITURE_STATUSES[number]; +export const APPROVAL_STATUSES = ['approved', 'denied'] as const; +export type ApprovalStatus = typeof APPROVAL_STATUSES[number]; + export interface ExpenditureInput { projectID: number; amount: number; @@ -76,6 +79,18 @@ export class ExpenditureValidationUtils { return status as ExpenditureStatus; } + static validateApprovalStatus(status: unknown): ApprovalStatus | Error { + if (status === undefined || status === null || status === '') { + return new Error('status is required'); + } + + if (typeof status !== 'string' || !APPROVAL_STATUSES.includes(status as ApprovalStatus)) { + return new Error(`status must be one of: ${APPROVAL_STATUSES.join(', ')}`); + } + + return status as ApprovalStatus; + } + static validateReceiptUrl(receiptUrl: unknown): string | undefined | Error { if (receiptUrl === undefined || receiptUrl === null) { return undefined; diff --git a/infrastructure/aws/api_gateway.tf b/infrastructure/aws/api_gateway.tf index 1ebde4c1..3135be90 100644 --- a/infrastructure/aws/api_gateway.tf +++ b/infrastructure/aws/api_gateway.tf @@ -14,7 +14,7 @@ locals { lambda_methods = { auth = ["GET", "POST"] donors = ["GET"] - expenditures = ["GET", "POST"] + expenditures = ["GET", "POST", "PATCH"] projects = ["GET", "POST"] reports = ["GET"] users = ["GET", "POST", "DELETE", "PATCH"] From 96526dd035cca3c3b0d9b0cd4a77a13edf55b430 Mon Sep 17 00:00:00 2001 From: Rayna-Yu Date: Thu, 9 Jul 2026 18:50:29 -0400 Subject: [PATCH 2/2] use EXPENDITURE_STATUSES and delete APPROVAL_STATUSES --- .../expenditures/test/expenditures.e2e.test.ts | 4 ++-- .../expenditures/test/expenditures.unit.test.ts | 4 ++-- apps/backend/lambdas/expenditures/validation-utils.ts | 11 ++++------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts b/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts index ad38357a..46b174db 100644 --- a/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts +++ b/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts @@ -475,9 +475,9 @@ describe('Expenditures integration tests', () => { expect(res.statusCode).toBe(404); }); - test('400: status not approved/denied is rejected', async () => { + test('400: status not valid is rejected', async () => { mockAuthenticateRequest.mockResolvedValue(adminUser); - const res = await handler(patchStatusEvent(1, { status: 'pending' })); + const res = await handler(patchStatusEvent(1, { status: 'pend' })); expect(res.statusCode).toBe(400); expect(await getStatus(1)).toBe('pending'); diff --git a/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts b/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts index 6c469a03..9d07f18f 100644 --- a/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts +++ b/apps/backend/lambdas/expenditures/test/expenditures.unit.test.ts @@ -701,8 +701,8 @@ describe('PATCH /expenditures/{id}/status unit tests', () => { expect(JSON.parse(res.body).message).toContain('status is required'); }); - test('400: status not approved/denied (e.g. pending)', async () => { - const res = await handler(patchStatusEvent(5, { status: 'pending' })); + test('400: status not valid', async () => { + const res = await handler(patchStatusEvent(5, { status: 'pend' })); expect(res.statusCode).toBe(400); expect(JSON.parse(res.body).message).toContain('status must be one of'); }); diff --git a/apps/backend/lambdas/expenditures/validation-utils.ts b/apps/backend/lambdas/expenditures/validation-utils.ts index 9a34b0c9..7c6d5dc2 100644 --- a/apps/backend/lambdas/expenditures/validation-utils.ts +++ b/apps/backend/lambdas/expenditures/validation-utils.ts @@ -1,9 +1,6 @@ export const EXPENDITURE_STATUSES = ['approved', 'pending', 'denied', 'needs_more_info'] as const; export type ExpenditureStatus = typeof EXPENDITURE_STATUSES[number]; -export const APPROVAL_STATUSES = ['approved', 'denied'] as const; -export type ApprovalStatus = typeof APPROVAL_STATUSES[number]; - export interface ExpenditureInput { projectID: number; amount: number; @@ -79,16 +76,16 @@ export class ExpenditureValidationUtils { return status as ExpenditureStatus; } - static validateApprovalStatus(status: unknown): ApprovalStatus | Error { + static validateApprovalStatus(status: unknown): ExpenditureStatus | Error { if (status === undefined || status === null || status === '') { return new Error('status is required'); } - if (typeof status !== 'string' || !APPROVAL_STATUSES.includes(status as ApprovalStatus)) { - return new Error(`status must be one of: ${APPROVAL_STATUSES.join(', ')}`); + if (typeof status !== 'string' || !EXPENDITURE_STATUSES.includes(status as ExpenditureStatus)) { + return new Error(`status must be one of: ${EXPENDITURE_STATUSES.join(', ')}`); } - return status as ApprovalStatus; + return status as ExpenditureStatus; } static validateReceiptUrl(receiptUrl: unknown): string | undefined | Error {