From e0530680cbd11ca87badec28ea766d1357fd5f7c Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 26 Jul 2026 22:24:14 +0200 Subject: [PATCH] feat(server): adopt zod DTOs for the settings body contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SettingUpsertDto/SettingsBulkDto wrap the existing @trek/shared schemas; both SettingsController entries drop off the body-contract allow-list (boot ratchet). Per the todo precedent the dead bespoke 400 guards go: missing/empty key and a non-record settings body now answer with the pipe envelope ({ error: 'key: …' } / { error: 'settings: …' }) instead of 'Key is required' / 'Settings object is required', and arrays for settings are rejected. The local masked-value literal now imports MASKED_SETTING_VALUE from @trek/shared. The settings e2e registers ZodValidationPipe and pins the DTO 400s plus the bulk masked-sentinel skip. --- .../nest/common/body-contract-allow-list.ts | 2 - .../src/nest/settings/settings.controller.ts | 29 ++++----- server/src/nest/settings/settings.dto.ts | 11 ++++ server/tests/e2e/settings.e2e.test.ts | 64 ++++++++++++++----- .../unit/nest/settings.controller.test.ts | 10 +-- 5 files changed, 76 insertions(+), 40 deletions(-) create mode 100644 server/src/nest/settings/settings.dto.ts diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index cd65117c5..f28efaf81 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -132,8 +132,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'ReservationsController.update', 'ReservationsController.updatePositions', 'ReservationsController.updateTravelers', - 'SettingsController.bulk', - 'SettingsController.upsert', 'SynologyMemoriesController.putSettings', 'SynologyMemoriesController.search', 'SynologyMemoriesController.test', diff --git a/server/src/nest/settings/settings.controller.ts b/server/src/nest/settings/settings.controller.ts index 6372b655a..79e6fe537 100644 --- a/server/src/nest/settings/settings.controller.ts +++ b/server/src/nest/settings/settings.controller.ts @@ -1,17 +1,18 @@ import { Body, Controller, Get, HttpCode, HttpException, Post, Put, UseGuards } from '@nestjs/common'; +import { MASKED_SETTING_VALUE } from '@trek/shared'; import type { User } from '../../types'; import { SettingsService } from './settings.service'; +import { SettingUpsertDto, SettingsBulkDto } from './settings.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -const MASKED_VALUE = '••••••••'; - /** - * /api/settings — per-user key/value preferences. - * - * Byte-identical to the legacy Express route (server/src/routes/settings.ts): - * get-all, single upsert (400 without a key, no-op on the masked sentinel), and - * bulk upsert (400 without an object, 500 on a write error). All answer 200. + * /api/settings — per-user key/value preferences: get-all, single upsert + * (no-op on the masked sentinel), and bulk upsert (500 on a write error). All + * answer 200. Bodies validate against the @trek/shared settings schemas via + * the DTO classes in settings.dto.ts + the global ZodValidationPipe (400 with + * the standard `{ error }` envelope on mismatch — this replaced the legacy + * bespoke 'Key is required' / 'Settings object is required' checks). */ @Controller('api/settings') @UseGuards(JwtAuthGuard) @@ -24,12 +25,9 @@ export class SettingsController { } @Put() - upsert(@CurrentUser() user: User, @Body() body: { key?: string; value?: unknown }) { - if (!body.key) { - throw new HttpException({ error: 'Key is required' }, 400); - } + upsert(@CurrentUser() user: User, @Body() body: SettingUpsertDto) { // The client echoes a redacted secret back unchanged — treat as a no-op. - if (body.value === MASKED_VALUE) { + if (body.value === MASKED_SETTING_VALUE) { return { success: true, key: body.key, unchanged: true }; } this.settings.upsertSetting(user.id, body.key, body.value); @@ -38,12 +36,9 @@ export class SettingsController { @Post('bulk') @HttpCode(200) // Express answers bulk with res.json (200), not the POST-default 201. - bulk(@CurrentUser() user: User, @Body() body: { settings?: unknown }) { - if (!body.settings || typeof body.settings !== 'object') { - throw new HttpException({ error: 'Settings object is required' }, 400); - } + bulk(@CurrentUser() user: User, @Body() body: SettingsBulkDto) { try { - const updated = this.settings.bulkUpsertSettings(user.id, body.settings as Record); + const updated = this.settings.bulkUpsertSettings(user.id, body.settings); return { success: true, updated }; } catch (err) { console.error('Error saving settings:', err); diff --git a/server/src/nest/settings/settings.dto.ts b/server/src/nest/settings/settings.dto.ts new file mode 100644 index 000000000..e5e94feca --- /dev/null +++ b/server/src/nest/settings/settings.dto.ts @@ -0,0 +1,11 @@ +import { createZodDto } from 'nestjs-zod'; +import { settingUpsertRequestSchema, settingsBulkRequestSchema } from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared settings 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 SettingUpsertDto extends createZodDto(settingUpsertRequestSchema) {} +export class SettingsBulkDto extends createZodDto(settingsBulkRequestSchema) {} diff --git a/server/tests/e2e/settings.e2e.test.ts b/server/tests/e2e/settings.e2e.test.ts index 930576d43..83b715c11 100644 --- a/server/tests/e2e/settings.e2e.test.ts +++ b/server/tests/e2e/settings.e2e.test.ts @@ -1,12 +1,15 @@ /** * Settings e2e — exercises the migrated /api/settings endpoints through the real - * JwtAuthGuard against a temp SQLite db. The settings service is mocked; this - * focuses on auth, the 400 guards, the masked-sentinel no-op and status codes. + * JwtAuthGuard against a temp SQLite db. DI-native: SettingsService runs its + * real SQL over the temp db (no service mock); this covers auth, the DTO 400s + * (ZodValidationPipe), the masked-sentinel no-op, status codes and the actual + * persisted rows. */ import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; +import { DatabaseModule } from '../../src/nest/database/database.module'; import { Test } from '@nestjs/testing'; import { seedUser, sessionCookie } from './harness'; @@ -17,28 +20,28 @@ const { db } = vi.hoisted(() => { tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); + tmp.exec(`CREATE TABLE settings (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, + key TEXT NOT NULL, value TEXT, UNIQUE(user_id, key));`); + tmp.exec('CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);'); return { db: tmp }; }); vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); -const { settingsSvc } = vi.hoisted(() => ({ - settingsSvc: { getUserSettings: vi.fn(), upsertSetting: vi.fn(), bulkUpsertSettings: vi.fn() }, -})); -vi.mock('../../src/services/settingsService', () => settingsSvc); - import { SettingsModule } from '../../src/nest/settings/settings.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; describe('Settings e2e (real auth guard + temp SQLite)', () => { let server: Server; let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [SettingsModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, SettingsModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); + nest.useGlobalPipes(new ZodValidationPipe()); await nest.init(); return nest; } @@ -47,11 +50,12 @@ describe('Settings e2e (real auth guard + temp SQLite)', () => { seedUser(db as never, { id: 1 }); app = await build(); server = app.getHttpServer(); - settingsSvc.getUserSettings.mockReturnValue({ theme: 'dark' }); - settingsSvc.bulkUpsertSettings.mockReturnValue(2); }); - beforeEach(() => vi.clearAllMocks()); + beforeEach(() => { + db.exec('DELETE FROM settings'); + db.exec('DELETE FROM app_settings'); + }); afterAll(async () => { await app.close(); @@ -62,29 +66,57 @@ describe('Settings e2e (real auth guard + temp SQLite)', () => { }); it('200 list with a session', async () => { - settingsSvc.getUserSettings.mockReturnValue({ theme: 'dark' }); + db.prepare("INSERT INTO settings (user_id, key, value) VALUES (1, 'theme', 'dark')").run(); const res = await request(server).get('/api/settings').set('Cookie', sessionCookie(1)); expect(res.status).toBe(200); expect(res.body).toEqual({ settings: { theme: 'dark' } }); }); - it('PUT 400 without a key', async () => { + it('PUT 400 without a key (pipe envelope via SettingUpsertDto)', async () => { const res = await request(server).put('/api/settings').set('Cookie', sessionCookie(1)).send({ value: 'x' }); expect(res.status).toBe(400); - expect(res.body).toEqual({ error: 'Key is required' }); + expect(res.body.error).toMatch(/^key: /); + expect(db.prepare('SELECT COUNT(*) AS n FROM settings').get()).toEqual({ n: 0 }); + }); + + it('POST /bulk 400 without a settings object (pipe envelope via SettingsBulkDto)', async () => { + const res = await request(server).post('/api/settings/bulk').set('Cookie', sessionCookie(1)).send({ settings: null }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/^settings: /); + }); + + it('PUT persists the setting', async () => { + const res = await request(server).put('/api/settings').set('Cookie', sessionCookie(1)).send({ key: 'language', value: 'fr' }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ success: true, key: 'language', value: 'fr' }); + const row = db.prepare("SELECT value FROM settings WHERE user_id = 1 AND key = 'language'").get() as { value: string }; + expect(row.value).toBe('fr'); }); it('PUT no-ops on the masked sentinel', async () => { const res = await request(server).put('/api/settings').set('Cookie', sessionCookie(1)).send({ key: 'secret', value: '••••••••' }); expect(res.status).toBe(200); expect(res.body).toEqual({ success: true, key: 'secret', unchanged: true }); - expect(settingsSvc.upsertSetting).not.toHaveBeenCalled(); + expect(db.prepare("SELECT COUNT(*) AS n FROM settings WHERE user_id = 1 AND key = 'secret'").get()).toEqual({ n: 0 }); }); it('POST /bulk 200', async () => { - settingsSvc.bulkUpsertSettings.mockReturnValue(2); const res = await request(server).post('/api/settings/bulk').set('Cookie', sessionCookie(1)).send({ settings: { a: 1, b: 2 } }); expect(res.status).toBe(200); expect(res.body).toEqual({ success: true, updated: 2 }); + const rows = db.prepare('SELECT key, value FROM settings WHERE user_id = 1 ORDER BY key').all(); + expect(rows).toEqual([ + { key: 'a', value: '1' }, + { key: 'b', value: '2' }, + ]); + }); + + it('POST /bulk skips masked-sentinel values (a stored secret survives)', async () => { + db.prepare("INSERT INTO settings (user_id, key, value) VALUES (1, 'ntfy_token', 'tok-real')").run(); + const res = await request(server).post('/api/settings/bulk').set('Cookie', sessionCookie(1)) + .send({ settings: { ntfy_topic: 'trek', ntfy_token: '••••••••' } }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ success: true, updated: 1 }); + expect(db.prepare("SELECT value FROM settings WHERE user_id = 1 AND key = 'ntfy_token'").get()).toEqual({ value: 'tok-real' }); }); }); diff --git a/server/tests/unit/nest/settings.controller.test.ts b/server/tests/unit/nest/settings.controller.test.ts index f585485f8..654f9d0e0 100644 --- a/server/tests/unit/nest/settings.controller.test.ts +++ b/server/tests/unit/nest/settings.controller.test.ts @@ -27,9 +27,8 @@ describe('SettingsController', () => { expect(new SettingsController(svc({ getUserSettings: vi.fn().mockReturnValue({ theme: 'dark' }) } as Partial)).list(user)).toEqual({ settings: { theme: 'dark' } }); }); - it('PUT / 400 without a key', () => { - expect(thrown(() => new SettingsController(svc()).upsert(user, {}))).toEqual({ status: 400, body: { error: 'Key is required' } }); - }); + // The 400-without-a-key path moved to the global ZodValidationPipe (via + // SettingUpsertDto) — pinned by the settings e2e, not reachable here. it('PUT / no-ops on the masked sentinel without writing', () => { const upsertSetting = vi.fn(); @@ -45,8 +44,9 @@ describe('SettingsController', () => { expect(upsertSetting).toHaveBeenCalledWith(1, 'theme', 'dark'); }); - it('POST /bulk 400 without an object, 500 on a write error, else returns the count', () => { - expect(thrown(() => new SettingsController(svc()).bulk(user, {}))).toEqual({ status: 400, body: { error: 'Settings object is required' } }); + it('POST /bulk 500 on a write error, else returns the count', () => { + // The 400-without-an-object path moved to the global ZodValidationPipe + // (via SettingsBulkDto) — pinned by the settings e2e, not reachable here. vi.spyOn(console, 'error').mockImplementation(() => {}); expect(thrown(() => new SettingsController(svc({ bulkUpsertSettings: vi.fn(() => { throw new Error('db'); }) } as Partial)).bulk(user, { settings: { a: 1 } }))).toEqual({ status: 500, body: { error: 'Error saving settings' } }); expect(new SettingsController(svc({ bulkUpsertSettings: vi.fn().mockReturnValue(3) } as Partial)).bulk(user, { settings: { a: 1, b: 2, c: 3 } })).toEqual({ success: true, updated: 3 });