From 0ac6a9346ca1c5157ab90c0fefea3774f0ea2f5f Mon Sep 17 00:00:00 2001 From: Rayna-Yu Date: Mon, 8 Jun 2026 18:17:26 -0400 Subject: [PATCH 1/2] add validation to patch --- apps/backend/lambdas/users/handler.ts | 49 +++-- .../lambdas/users/test/user.unit.test.ts | 185 +++++++++++++++++- .../backend/lambdas/users/validation-utils.ts | 58 ++++++ 3 files changed, 279 insertions(+), 13 deletions(-) create mode 100644 apps/backend/lambdas/users/validation-utils.ts diff --git a/apps/backend/lambdas/users/handler.ts b/apps/backend/lambdas/users/handler.ts index aa6fc503..7db06090 100644 --- a/apps/backend/lambdas/users/handler.ts +++ b/apps/backend/lambdas/users/handler.ts @@ -1,6 +1,7 @@ import { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; import db from './db' import { authenticateRequest, checkAuthorization, AuthContext } from './auth'; +import { UserValidationUtils } from './validation-utils'; function requireAuth(authContext: AuthContext, level: Parameters[1], resourceUserId?: number | string): APIGatewayProxyResult | undefined { const authCheck = checkAuthorization(authContext, level, resourceUserId); @@ -124,15 +125,31 @@ export const handler = async (event: any): Promise => { let user = await db.selectFrom("branch.users").where("user_id", "=", Number(userId)).selectAll().executeTakeFirst(); if (!user) return json(404, { message: 'User not found' }); - // vars to update - let email = body.email as string; - let name = body.name as string; - let isAdmin = body.isAdmin as boolean; - let profileImage = body.profileImage as string | undefined; + const updates: { email?: string; name?: string; is_admin?: boolean; profile_image?: string } = {}; + + const emailResult = UserValidationUtils.validateEmail(body.email); + if (!emailResult.isValid) return json(400, { message: emailResult.error }); + if (emailResult.value != null) updates.email = emailResult.value; + + const nameResult = UserValidationUtils.validateName(body.name); + if (!nameResult.isValid) return json(400, { message: nameResult.error }); + if (nameResult.value != null) updates.name = nameResult.value; + + const isAdminResult = UserValidationUtils.validateIsAdmin(body.isAdmin); + if (!isAdminResult.isValid) return json(400, { message: isAdminResult.error }); + if (isAdminResult.value != null) updates.is_admin = isAdminResult.value; + + const profileImageResult = UserValidationUtils.validateProfileImage(body.profileImage); + if (!profileImageResult.isValid) return json(400, { message: profileImageResult.error }); + if (profileImageResult.value != null) updates.profile_image = profileImageResult.value; + + if (Object.keys(updates).length === 0) { + return json(400, { message: 'No valid fields provided to update' }); + } // update await db.updateTable('branch.users') - .set({ email, name, is_admin: isAdmin, profile_image: profileImage }) + .set(updates) .where('user_id', '=', Number(userId)) .execute(); @@ -168,14 +185,22 @@ export const handler = async (event: any): Promise => { ? (JSON.parse(event.body) as Record) : {}; - // extract fields to create user - let email = body.email as string; - let name = body.name as string; - let isAdmin = body.isAdmin as boolean; - if (!email || !name || typeof isAdmin !== 'boolean') { + // email, name, and isAdmin are required on create + if (!body.email || !body.name || typeof body.isAdmin !== 'boolean') { return json(400, { message: 'email, name, and isAdmin are required' }); } - let profile_image = body.profileImage as string; + + // validate the type/format of each field + const emailResult = UserValidationUtils.validateEmail(body.email); + if (!emailResult.isValid) return json(400, { message: emailResult.error }); + + const profileImageResult = UserValidationUtils.validateProfileImage(body.profileImage); + if (!profileImageResult.isValid) return json(400, { message: profileImageResult.error }); + + const email = emailResult.value as string; + const name = body.name as string; + const isAdmin = body.isAdmin; + const profile_image = profileImageResult.value ?? undefined; // Check if user with this email already exists const existingUser = await db diff --git a/apps/backend/lambdas/users/test/user.unit.test.ts b/apps/backend/lambdas/users/test/user.unit.test.ts index 97d73278..dba96684 100644 --- a/apps/backend/lambdas/users/test/user.unit.test.ts +++ b/apps/backend/lambdas/users/test/user.unit.test.ts @@ -54,6 +54,45 @@ function postEvent(body: Record) { }; } +// Helper to create a PATCH event for /{userId} +function patchEvent(userId: string | number, body: unknown) { + return { + rawPath: `/${userId}`, + requestContext: { + http: { + method: 'PATCH', + }, + }, + body: typeof body === 'string' ? body : JSON.stringify(body), + }; +} + +function mockExistingUserForPatch(updated?: Record) { + const existing = { + user_id: 1, + name: 'Existing User', + email: 'existing@example.com', + is_admin: false, + profile_image: null, + }; + + mockDb.selectFrom.mockReturnValue({ + where: jest.fn().mockReturnValue({ + selectAll: jest.fn().mockReturnValue({ + executeTakeFirst: (jest.fn() as any).mockResolvedValue(updated ?? existing), + }), + }), + }); + + mockDb.updateTable.mockReturnValue({ + set: jest.fn().mockReturnValue({ + where: jest.fn().mockReturnValue({ + execute: (jest.fn() as any).mockResolvedValue(undefined), + }), + }), + }); +} + function mockAdminAuth() { mockAuthenticateRequest.mockResolvedValue({ isAuthenticated: true, @@ -307,7 +346,7 @@ describe('POST /users unit tests', () => { expect(json).toHaveProperty('body'); }); - test('409: returns 409 when user already exists', async () => { + test('409: POST returns 409 when user already exists', async () => { // Mock: user already exists const whereChain = { selectAll: jest.fn().mockReturnValue({ @@ -338,3 +377,147 @@ describe('POST /users unit tests', () => { }); }); }); + +describe('PATCH /users/{userId} unit tests', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockAdminAuth(); + }); + + describe('Input Validation', () => { + test('400: invalid email format', async () => { + mockExistingUserForPatch(); + + const res = await handler(patchEvent(1, { email: 'not-an-email' })); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('email'); + }); + + test('400: email is not a string', async () => { + mockExistingUserForPatch(); + + const res = await handler(patchEvent(1, { email: 123 })); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('email'); + }); + + test('400: name is not a string', async () => { + mockExistingUserForPatch(); + + const res = await handler(patchEvent(1, { name: 42 })); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('name'); + }); + + test('400: empty name field', async () => { + mockExistingUserForPatch(); + + const res = await handler(patchEvent(1, { name: ' ' })); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('name'); + }); + + test('400: isAdmin is not a boolean', async () => { + mockExistingUserForPatch(); + + const res = await handler(patchEvent(1, { isAdmin: 'yes' })); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('isAdmin'); + }); + + test('400: profileImage is not a string', async () => { + mockExistingUserForPatch(); + + const res = await handler(patchEvent(1, { profileImage: 5 })); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('profileImage'); + }); + + test('400: no valid fields provided', async () => { + mockExistingUserForPatch(); + + const res = await handler(patchEvent(1, {})); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('No valid fields'); + }); + + }); + + describe('Success Cases', () => { + test('404: returns 404 when user does not exist', async () => { + mockDb.selectFrom.mockReturnValue({ + where: jest.fn().mockReturnValue({ + selectAll: jest.fn().mockReturnValue({ + executeTakeFirst: (jest.fn() as any).mockResolvedValue(null), + }), + }), + }); + + const res = await handler(patchEvent(999, { name: 'Whoever' })); + + expect(res.statusCode).toBe(404); + const json = JSON.parse(res.body); + expect(json).toHaveProperty('message'); + }); + + test('200: valid partial update returns updated fields', async () => { + mockExistingUserForPatch({ + user_id: 1, + name: 'Existing User', + email: 'new@example.com', + is_admin: true, + profile_image: null, + }); + + const res = await handler(patchEvent(1, { email: 'new@example.com', isAdmin: true })); + + expect(res.statusCode).toBe(200); + const json = JSON.parse(res.body); + expect(json).toHaveProperty('ok'); + expect(json).toHaveProperty('route'); + expect(json).toHaveProperty('body'); + expect(json.body.email).toBe('new@example.com'); + expect(json.body.isAdmin).toBe(true); + }); + + test('200: only the provided field is written to the database', async () => { + mockExistingUserForPatch({ + user_id: 1, + name: 'New Name', + email: 'existing@example.com', + is_admin: false, + profile_image: null, + }); + + const res = await handler(patchEvent(1, { name: 'New Name' })); + + expect(res.statusCode).toBe(200); + // Only the provided field should be passed to .set(). toStrictEqual (unlike + // toEqual) does NOT ignore undefined keys, so this fails if the handler ever + // regresses to setting every column and leaving omitted ones undefined. + const setCall = (mockDb.updateTable.mock.results[0].value.set as jest.Mock).mock.calls[0][0]; + expect(setCall).toStrictEqual({ name: 'New Name' }); + }); + }); +}); diff --git a/apps/backend/lambdas/users/validation-utils.ts b/apps/backend/lambdas/users/validation-utils.ts new file mode 100644 index 00000000..6b512abf --- /dev/null +++ b/apps/backend/lambdas/users/validation-utils.ts @@ -0,0 +1,58 @@ +// Result type for input validation operations +export type ValidationResult = { + isValid: boolean; + value?: T; + error?: string; +}; + +// Utility class for validating user-related input fields. +export class UserValidationUtils { + static readonly EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + + // Validates email - if provided, must be a correctly formatted string + static validateEmail(input: unknown): ValidationResult { + if (input === undefined || input === null || input === '') { + return { isValid: true, value: null }; + } + if (typeof input !== 'string') { + return { isValid: false, error: 'email must be a string' }; + } + if (!this.EMAIL_REGEX.test(input)) { + return { isValid: false, error: 'Invalid email format' }; + } + return { isValid: true, value: input }; + } + + // Validates name - if provided, must be a non-empty string + static validateName(input: unknown): ValidationResult { + if (input === undefined || input === null) { + return { isValid: true, value: null }; + } + if (typeof input !== 'string' || input.trim().length === 0) { + return { isValid: false, error: 'name must be a non-empty string' }; + } + return { isValid: true, value: input }; + } + + // Validates isAdmin - if provided, must be a boolean + static validateIsAdmin(input: unknown): ValidationResult { + if (input === undefined || input === null) { + return { isValid: true, value: null }; + } + if (typeof input !== 'boolean') { + return { isValid: false, error: 'isAdmin must be a boolean' }; + } + return { isValid: true, value: input }; + } + + // Validates profileImage - if provided, must be a string + static validateProfileImage(input: unknown): ValidationResult { + if (input === undefined || input === null) { + return { isValid: true, value: null }; + } + if (typeof input !== 'string') { + return { isValid: false, error: 'profileImage must be a string' }; + } + return { isValid: true, value: input }; + } +} From a646e2674e1256f799a34196b09e3c3b705a9b8e Mon Sep 17 00:00:00 2001 From: Rayna-Yu Date: Sun, 14 Jun 2026 20:21:40 -0400 Subject: [PATCH 2/2] address pr comments --- apps/backend/lambdas/users/handler.ts | 12 +++++++++--- .../lambdas/users/test/user.unit.test.ts | 17 ++++++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/apps/backend/lambdas/users/handler.ts b/apps/backend/lambdas/users/handler.ts index 7db06090..911619b7 100644 --- a/apps/backend/lambdas/users/handler.ts +++ b/apps/backend/lambdas/users/handler.ts @@ -186,7 +186,7 @@ export const handler = async (event: any): Promise => { : {}; // email, name, and isAdmin are required on create - if (!body.email || !body.name || typeof body.isAdmin !== 'boolean') { + if (!body.email || !body.name || body.isAdmin === undefined || body.isAdmin === null) { return json(400, { message: 'email, name, and isAdmin are required' }); } @@ -194,12 +194,18 @@ export const handler = async (event: any): Promise => { const emailResult = UserValidationUtils.validateEmail(body.email); if (!emailResult.isValid) return json(400, { message: emailResult.error }); + const nameResult = UserValidationUtils.validateName(body.name); + if (!nameResult.isValid) return json(400, { message: nameResult.error }); + + const isAdminResult = UserValidationUtils.validateIsAdmin(body.isAdmin); + if (!isAdminResult.isValid) return json(400, { message: isAdminResult.error }); + const profileImageResult = UserValidationUtils.validateProfileImage(body.profileImage); if (!profileImageResult.isValid) return json(400, { message: profileImageResult.error }); const email = emailResult.value as string; - const name = body.name as string; - const isAdmin = body.isAdmin; + const name = nameResult.value as string; + const isAdmin = isAdminResult.value as boolean; const profile_image = profileImageResult.value ?? undefined; // Check if user with this email already exists diff --git a/apps/backend/lambdas/users/test/user.unit.test.ts b/apps/backend/lambdas/users/test/user.unit.test.ts index dba96684..c0ad16c5 100644 --- a/apps/backend/lambdas/users/test/user.unit.test.ts +++ b/apps/backend/lambdas/users/test/user.unit.test.ts @@ -271,7 +271,7 @@ describe('POST /users unit tests', () => { expect(res.statusCode).toBe(400); const json = JSON.parse(res.body); expect(json.message).toBeDefined(); - expect(json.message).toContain('required'); + expect(json.message).toContain('isAdmin'); }); test('400: empty email field', async () => { @@ -301,6 +301,21 @@ describe('POST /users unit tests', () => { const json = JSON.parse(res.body); expect(json.message).toContain('required'); }); + + test('400: name is not a string', async () => { + const res = await handler( + postEvent({ + name: 42, + email: 'john@example.com', + isAdmin: false, + }) + ); + + expect(res.statusCode).toBe(400); + const json = JSON.parse(res.body); + expect(json.message).toBeDefined(); + expect(json.message).toContain('name'); + }); }); describe('Response Format', () => {