From cf1b9e4c0681c1b9053afc12dd619cacbe4dbf3e Mon Sep 17 00:00:00 2001 From: jubnl Date: Mon, 27 Jul 2026 23:48:23 +0200 Subject: [PATCH] feat(server): validate budget request bodies with shared Zod DTOs --- server/src/nest/budget/budget.controller.ts | 70 ++++++++++--------- server/src/nest/budget/budget.dto.ts | 28 ++++++++ .../nest/common/body-contract-allow-list.ts | 9 --- .../tests/unit/nest/budget.controller.test.ts | 61 +++++----------- 4 files changed, 83 insertions(+), 85 deletions(-) create mode 100644 server/src/nest/budget/budget.dto.ts diff --git a/server/src/nest/budget/budget.controller.ts b/server/src/nest/budget/budget.controller.ts index 1ea0be54b..96f920129 100644 --- a/server/src/nest/budget/budget.controller.ts +++ b/server/src/nest/budget/budget.controller.ts @@ -15,16 +15,33 @@ import type { User } from '../../types'; import { BudgetService } from './budget.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; +import { + BudgetCreateItemDto, + BudgetUpdateItemDto, + BudgetUpdatePayersDto, + BudgetUpdateMembersDto, + BudgetToggleMemberPaidDto, + BudgetReorderItemsDto, + BudgetReorderCategoriesDto, + BudgetCreateSettlementDto, + BudgetUpdateSettlementDto, +} from './budget.dto'; /** * /api/trips/:tripId/budget — trip-scoped expense planner. * * Byte-identical to the legacy Express route (server/src/routes/budget.ts): * every handler verifies trip access (404); mutations check 'budget_edit' (403); - * create is 201, the rest 200; bespoke 400/404 bodies reproduced; mutations + * create is 201, the rest 200; bespoke 404 bodies reproduced; mutations * broadcast over WebSocket with the forwarded X-Socket-Id. Static sub-routes * (summary, settlement, reorder/*) are declared before /:id so they win over the * param. Updating total_price on a reservation-linked item syncs the price back. + * + * Bodies are validated against the @trek/shared budget schemas via budget.dto.ts + * (global ZodValidationPipe). This replaced the legacy bespoke 400s ('Name is + * required', 'from_user_id, to_user_id and amount are required', 'user_ids must + * be an array', 'payers must be an array') with the pipe's uniform + * { error: 'field: message; …' } envelope. */ @Controller('api/trips/:tripId/budget') @UseGuards(JwtAuthGuard) @@ -77,14 +94,11 @@ export class BudgetController { async createSettlement( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: { from_user_id?: number; to_user_id?: number; amount?: number; currency?: string | null }, + @Body() body: BudgetCreateSettlementDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (body.from_user_id == null || body.to_user_id == null || body.amount == null) { - throw new HttpException({ error: 'from_user_id, to_user_id and amount are required' }, 400); - } const settlement = await this.budget.createSettlement( tripId, { from_user_id: body.from_user_id, to_user_id: body.to_user_id, amount: body.amount, currency: body.currency }, @@ -99,14 +113,11 @@ export class BudgetController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('settlementId') settlementId: string, - @Body() body: { from_user_id?: number; to_user_id?: number; amount?: number; currency?: string | null }, + @Body() body: BudgetUpdateSettlementDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (body.from_user_id == null || body.to_user_id == null || body.amount == null) { - throw new HttpException({ error: 'from_user_id, to_user_id and amount are required' }, 400); - } const settlement = await this.budget.updateSettlement(settlementId, tripId, { from_user_id: body.from_user_id, to_user_id: body.to_user_id, @@ -140,15 +151,12 @@ export class BudgetController { async create( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: { name?: string; category?: string; total_price?: number; persons?: number | null; days?: number | null; note?: string | null; expense_date?: string | null; reservation_id?: number }, + @Body() body: BudgetCreateItemDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!body.name) { - throw new HttpException({ error: 'Name is required' }, 400); - } - const item = await this.budget.create(tripId, body as { name: string }); + const item = await this.budget.create(tripId, body); this.budget.broadcast(tripId, 'budget:created', { item }, socketId); return { item }; } @@ -157,13 +165,13 @@ export class BudgetController { reorderItems( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body('orderedIds') orderedIds: number[], + @Body() body: BudgetReorderItemsDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - this.budget.reorderItems(tripId, orderedIds); - this.budget.broadcast(tripId, 'budget:reordered', { orderedIds }, socketId); + this.budget.reorderItems(tripId, body.orderedIds); + this.budget.broadcast(tripId, 'budget:reordered', { orderedIds: body.orderedIds }, socketId); return { success: true }; } @@ -171,13 +179,13 @@ export class BudgetController { reorderCategories( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body('orderedCategories') orderedCategories: string[], + @Body() body: BudgetReorderCategoriesDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - this.budget.reorderCategories(tripId, orderedCategories); - this.budget.broadcast(tripId, 'budget:reordered', { orderedCategories }, socketId); + this.budget.reorderCategories(tripId, body.orderedCategories); + this.budget.broadcast(tripId, 'budget:reordered', { orderedCategories: body.orderedCategories }, socketId); return { success: true }; } @@ -186,7 +194,7 @@ export class BudgetController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, - @Body() body: Record, + @Body() body: BudgetUpdateItemDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); @@ -207,15 +215,12 @@ export class BudgetController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, - @Body('user_ids') userIds: unknown, + @Body() body: BudgetUpdateMembersDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!Array.isArray(userIds)) { - throw new HttpException({ error: 'user_ids must be an array' }, 400); - } - const result = this.budget.updateMembers(id, tripId, userIds); + const result = this.budget.updateMembers(id, tripId, body.user_ids); if (!result) { throw new HttpException({ error: 'Budget item not found' }, 404); } @@ -228,15 +233,12 @@ export class BudgetController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, - @Body('payers') payers: unknown, + @Body() body: BudgetUpdatePayersDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!Array.isArray(payers)) { - throw new HttpException({ error: 'payers must be an array' }, 400); - } - const item = this.budget.setPayers(id, tripId, payers as { user_id: number; amount: number }[]); + const item = this.budget.setPayers(id, tripId, body.payers); if (!item) { throw new HttpException({ error: 'Budget item not found' }, 404); } @@ -250,13 +252,13 @@ export class BudgetController { @Param('tripId') tripId: string, @Param('id') id: string, @Param('userId') userId: string, - @Body('paid') paid: boolean, + @Body() body: BudgetToggleMemberPaidDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const member = this.budget.toggleMemberPaid(id, tripId, userId, paid); - this.budget.broadcast(tripId, 'budget:member-paid-updated', { itemId: Number(id), userId: Number(userId), paid: paid ? 1 : 0 }, socketId); + const member = this.budget.toggleMemberPaid(id, tripId, userId, body.paid); + this.budget.broadcast(tripId, 'budget:member-paid-updated', { itemId: Number(id), userId: Number(userId), paid: body.paid ? 1 : 0 }, socketId); return { member }; } diff --git a/server/src/nest/budget/budget.dto.ts b/server/src/nest/budget/budget.dto.ts new file mode 100644 index 000000000..6a56bab77 --- /dev/null +++ b/server/src/nest/budget/budget.dto.ts @@ -0,0 +1,28 @@ +import { createZodDto } from 'nestjs-zod'; +import { + budgetCreateItemRequestSchema, + budgetUpdateItemRequestSchema, + budgetUpdatePayersRequestSchema, + budgetUpdateMembersRequestSchema, + budgetToggleMemberPaidRequestSchema, + budgetReorderItemsRequestSchema, + budgetReorderCategoriesRequestSchema, + budgetCreateSettlementRequestSchema, + budgetUpdateSettlementRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared budget contracts. The + * global ZodValidationPipe (APP_PIPE in app.module.ts) validates any @Body() + * parameter typed with one of these classes by metatype — the Zod schemas in + * shared/ remain the single source of truth for the wire contract. + */ +export class BudgetCreateItemDto extends createZodDto(budgetCreateItemRequestSchema) {} +export class BudgetUpdateItemDto extends createZodDto(budgetUpdateItemRequestSchema) {} +export class BudgetUpdatePayersDto extends createZodDto(budgetUpdatePayersRequestSchema) {} +export class BudgetUpdateMembersDto extends createZodDto(budgetUpdateMembersRequestSchema) {} +export class BudgetToggleMemberPaidDto extends createZodDto(budgetToggleMemberPaidRequestSchema) {} +export class BudgetReorderItemsDto extends createZodDto(budgetReorderItemsRequestSchema) {} +export class BudgetReorderCategoriesDto extends createZodDto(budgetReorderCategoriesRequestSchema) {} +export class BudgetCreateSettlementDto extends createZodDto(budgetCreateSettlementRequestSchema) {} +export class BudgetUpdateSettlementDto extends createZodDto(budgetUpdateSettlementRequestSchema) {} diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index 2cc4302d8..096d83d94 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -49,15 +49,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'BookingImportController.confirm', 'BookingImportController.preview', 'BookingImportController.previewAsync', - 'BudgetController.create', - 'BudgetController.createSettlement', - 'BudgetController.reorderCategories', - 'BudgetController.reorderItems', - 'BudgetController.setPayers', - 'BudgetController.toggleMemberPaid', - 'BudgetController.update', - 'BudgetController.updateMembers', - 'BudgetController.updateSettlement', 'CategoriesController.create', 'CategoriesController.update', 'CollectionsController.deleteMany', diff --git a/server/tests/unit/nest/budget.controller.test.ts b/server/tests/unit/nest/budget.controller.test.ts index df5d0bfcc..556d4186f 100644 --- a/server/tests/unit/nest/budget.controller.test.ts +++ b/server/tests/unit/nest/budget.controller.test.ts @@ -86,18 +86,9 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou }); }); - it('POST /settlements 400 when a field is missing', async () => { - const svc = makeService(); - expect(await thrownAsync(() => new BudgetController(svc).createSettlement(user, '5', { from_user_id: 1, to_user_id: 2 }))).toEqual({ - status: 400, body: { error: 'from_user_id, to_user_id and amount are required' }, - }); - expect(await thrownAsync(() => new BudgetController(svc).createSettlement(user, '5', { from_user_id: 1, amount: 5 }))).toEqual({ - status: 400, body: { error: 'from_user_id, to_user_id and amount are required' }, - }); - expect(await thrownAsync(() => new BudgetController(svc).createSettlement(user, '5', { to_user_id: 2, amount: 5 }))).toEqual({ - status: 400, body: { error: 'from_user_id, to_user_id and amount are required' }, - }); - }); + // The legacy 'from_user_id, to_user_id and amount are required' 400s are now + // produced by the global ZodValidationPipe (budget.dto.ts) before the handler + // runs — covered by the integration suite, not constructible here. it('POST /settlements creates and broadcasts (amount 0 is allowed), forwarding the display currency', async () => { const createSettlement = vi.fn().mockResolvedValue({ id: 3, amount: 0 }); @@ -130,13 +121,6 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou }); }); - it('PUT /settlements/:id 400 when a field is missing', async () => { - const svc = makeService(); - expect(await thrownAsync(() => new BudgetController(svc).updateSettlement(user, '5', '7', { from_user_id: 1, to_user_id: 2 }))).toEqual({ - status: 400, body: { error: 'from_user_id, to_user_id and amount are required' }, - }); - }); - it('PUT /settlements/:id 404 when missing', async () => { const svc = makeService({ updateSettlement: vi.fn().mockResolvedValue(null) } as Partial); expect(await thrownAsync(() => new BudgetController(svc).updateSettlement(user, '5', '7', { from_user_id: 1, to_user_id: 2, amount: 10 }))).toEqual({ @@ -163,11 +147,9 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou }); }); - it('400 when name missing', async () => { - expect(await thrownAsync(() => new BudgetController(makeService()).create(user, '5', {}))).toEqual({ - status: 400, body: { error: 'Name is required' }, - }); - }); + // The legacy 'Name is required' 400 is now produced by the global + // ZodValidationPipe (budgetCreateItemRequestSchema requires name) before + // the handler runs — covered by the integration suite. it('creates and broadcasts', async () => { const create = vi.fn().mockReturnValue({ id: 9, name: 'Hotel' }); @@ -206,15 +188,12 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou }); describe('PUT /:id/members', () => { - it('400 when user_ids is not an array', () => { - expect(thrown(() => new BudgetController(makeService()).updateMembers(user, '5', '9', 'nope'))).toEqual({ - status: 400, body: { error: 'user_ids must be an array' }, - }); - }); + // The legacy 'user_ids must be an array' 400 is now produced by the global + // ZodValidationPipe (budgetUpdateMembersRequestSchema) before the handler runs. it('404 when the item is missing', () => { const svc = makeService({ updateMembers: vi.fn().mockReturnValue(null) } as Partial); - expect(thrown(() => new BudgetController(svc).updateMembers(user, '5', '9', [2, 3]))).toEqual({ + expect(thrown(() => new BudgetController(svc).updateMembers(user, '5', '9', { user_ids: [2, 3] }))).toEqual({ status: 404, body: { error: 'Budget item not found' }, }); }); @@ -223,22 +202,20 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou const updateMembers = vi.fn().mockReturnValue({ members: [{ user_id: 2 }], item: { persons: 1 } }); const broadcast = vi.fn(); const svc = makeService({ updateMembers, broadcast } as Partial); - const res = new BudgetController(svc).updateMembers(user, '5', '9', [2], 'sock'); + const res = new BudgetController(svc).updateMembers(user, '5', '9', { user_ids: [2] }, 'sock'); expect(res).toEqual({ members: [{ user_id: 2 }], item: { persons: 1 } }); + expect(updateMembers).toHaveBeenCalledWith('9', '5', [2]); expect(broadcast).toHaveBeenCalledWith('5', 'budget:members-updated', { itemId: 9, members: [{ user_id: 2 }], persons: 1 }, 'sock'); }); }); describe('PUT /:id/payers', () => { - it('400 when payers is not an array', () => { - expect(thrown(() => new BudgetController(makeService()).setPayers(user, '5', '9', 'nope'))).toEqual({ - status: 400, body: { error: 'payers must be an array' }, - }); - }); + // The legacy 'payers must be an array' 400 is now produced by the global + // ZodValidationPipe (budgetUpdatePayersRequestSchema) before the handler runs. it('404 when the item is missing', () => { const svc = makeService({ setPayers: vi.fn().mockReturnValue(null) } as Partial); - expect(thrown(() => new BudgetController(svc).setPayers(user, '5', '9', [{ user_id: 2, amount: 10 }]))).toEqual({ + expect(thrown(() => new BudgetController(svc).setPayers(user, '5', '9', { payers: [{ user_id: 2, amount: 10 }] }))).toEqual({ status: 404, body: { error: 'Budget item not found' }, }); }); @@ -247,7 +224,7 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou const setPayers = vi.fn().mockReturnValue({ id: 9, payers: [{ user_id: 2, amount: 10 }] }); const broadcast = vi.fn(); const svc = makeService({ setPayers, broadcast } as Partial); - const res = new BudgetController(svc).setPayers(user, '5', '9', [{ user_id: 2, amount: 10 }], 'sock'); + const res = new BudgetController(svc).setPayers(user, '5', '9', { payers: [{ user_id: 2, amount: 10 }] }, 'sock'); expect(res).toEqual({ item: { id: 9, payers: [{ user_id: 2, amount: 10 }] } }); expect(setPayers).toHaveBeenCalledWith('9', '5', [{ user_id: 2, amount: 10 }]); expect(broadcast).toHaveBeenCalledWith('5', 'budget:updated', { item: { id: 9, payers: [{ user_id: 2, amount: 10 }] } }, 'sock'); @@ -258,7 +235,7 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou const toggleMemberPaid = vi.fn().mockReturnValue({ user_id: 2, paid: 1 }); const broadcast = vi.fn(); const svc = makeService({ toggleMemberPaid, broadcast } as Partial); - expect(new BudgetController(svc).toggleMemberPaid(user, '5', '9', '2', true, 'sock')).toEqual({ member: { user_id: 2, paid: 1 } }); + expect(new BudgetController(svc).toggleMemberPaid(user, '5', '9', '2', { paid: true }, 'sock')).toEqual({ member: { user_id: 2, paid: 1 } }); expect(broadcast).toHaveBeenCalledWith('5', 'budget:member-paid-updated', { itemId: 9, userId: 2, paid: 1 }, 'sock'); }); @@ -266,7 +243,7 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou const toggleMemberPaid = vi.fn().mockReturnValue({ user_id: 2, paid: 0 }); const broadcast = vi.fn(); const svc = makeService({ toggleMemberPaid, broadcast } as Partial); - new BudgetController(svc).toggleMemberPaid(user, '5', '9', '2', false, 'sock'); + new BudgetController(svc).toggleMemberPaid(user, '5', '9', '2', { paid: false }, 'sock'); expect(broadcast).toHaveBeenCalledWith('5', 'budget:member-paid-updated', { itemId: 9, userId: 2, paid: 0 }, 'sock'); }); @@ -282,9 +259,9 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou it('PUT /reorder/items + /reorder/categories broadcast budget:reordered', () => { const reorderItems = vi.fn(); const reorderCategories = vi.fn(); const broadcast = vi.fn(); const svc = makeService({ reorderItems, reorderCategories, broadcast } as Partial); - expect(new BudgetController(svc).reorderItems(user, '5', [3, 1], 'sock')).toEqual({ success: true }); + expect(new BudgetController(svc).reorderItems(user, '5', { orderedIds: [3, 1] }, 'sock')).toEqual({ success: true }); expect(reorderItems).toHaveBeenCalledWith('5', [3, 1]); - expect(new BudgetController(svc).reorderCategories(user, '5', ['food', 'fun'], 'sock')).toEqual({ success: true }); + expect(new BudgetController(svc).reorderCategories(user, '5', { orderedCategories: ['food', 'fun'] }, 'sock')).toEqual({ success: true }); expect(reorderCategories).toHaveBeenCalledWith('5', ['food', 'fun']); }); });