Files
TREK/server/tests/unit/nest/todo.controller.test.ts
T
Maurice 8c0280e287 Replace the copied requireTrip helpers with a TripAccessGuard
Every trip-scoped controller carried its own three-line requireTrip, and a
requireEdit next to it: eighteen copies of a security check, each of which had to
remember that no access is a 404 'Trip not found' and never a 403 — the
distinction that stops a stranger learning which trip ids exist. One of them
getting it wrong would have looked like nothing.

TripAccessGuard resolves :tripId once, refuses what the user cannot reach and
hands the row to the handler through a @Trip() param decorator (a param
decorator rather than a req.trip cast, because the request type that used to be
widened for that is gone). @RequirePermission carries the same action string the
domain services pass to checkPermission, so the HTTP and MCP paths cannot drift
apart on which right they demand. Status codes and bodies are unchanged.

Piloted on days, day-notes, todo, packing and files. Two things worth knowing:

Files applies the guard PER HANDLER and leaves the upload route on its own
check. Guards run before interceptors, so a class-level guard would answer 404
before multer had read the multipart body, and a response sent while the client
is still streaming gets the socket destroyed — the caller sees ECONNRESET
instead of the 404. Refusing after the body has been read costs the bandwidth
and keeps the answer readable.

The services keep verifyTripAccess and canEdit: the large majority of their
callers are *.mcp.ts tools, which never pass through an HTTP guard. canEdit lost
its incidental controller-test coverage in the move, so it now has direct tests
asserting the action and the shared flag — the same pair the guard reproduces.

Also fixes the backup zip-slip test that failed only on CI: it asserted a
drive-letter path, which is absolute on win32 and an ordinary relative segment on
Linux, so the branch it was guarding never fired there.
2026-08-09 19:13:21 +02:00

120 lines
5.4 KiB
TypeScript

import { describe, it, expect, vi } from 'vitest';
import { HttpException } from '@nestjs/common';
import { TodoController } from '../../../src/nest/todo/todo.controller';
import type { TodoService } from '../../../src/nest/todo/todo.service';
import type { User } from '../../../src/types';
const user = { id: 1, role: 'user', email: 'u@example.test' } as User;
const trip = { id: 5, user_id: 1 };
function makeService(overrides: Partial<TodoService> = {}): TodoService {
return {
verifyTripAccess: vi.fn().mockReturnValue(trip),
canEdit: vi.fn().mockReturnValue(true),
broadcast: vi.fn(),
...overrides,
} as unknown as TodoService;
}
function thrown(fn: () => unknown): { status: number; body: unknown } {
try {
fn();
} catch (err) {
expect(err).toBeInstanceOf(HttpException);
const e = err as HttpException;
return { status: e.getStatus(), body: e.getResponse() };
}
throw new Error('expected the handler to throw');
}
// The 404 "Trip not found" and 403 "No permission" cases moved to
// trip-access.guard.test.ts with the check itself.
describe('TodoController (parity with the legacy /api/trips/:tripId/todo route)', () => {
it('GET / returns items', () => {
const svc = makeService({ listItems: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial<TodoService>);
expect(new TodoController(svc).list(user, '5')).toEqual({ items: [{ id: 1 }] });
});
describe('POST /', () => {
// The missing-name 400 moved from a bespoke controller check into the
// ZodValidationPipe (todoCreateItemRequestSchema) — direct method calls
// bypass parameter pipes, so that path is covered by the e2e suite and
// the schema spec in @trek/shared.
it('creates and broadcasts', () => {
const createItem = vi.fn().mockReturnValue({ id: 9, name: 'Pack' });
const broadcast = vi.fn();
const svc = makeService({ createItem, broadcast } as Partial<TodoService>);
expect(new TodoController(svc).create(user, '5', { name: 'Pack', priority: 2 }, 'sock')).toEqual({ item: { id: 9, name: 'Pack' } });
expect(broadcast).toHaveBeenCalledWith('5', 'todo:created', { item: { id: 9, name: 'Pack' } }, 'sock');
});
});
describe('PUT /:id', () => {
it('404 when item missing', () => {
const svc = makeService({ updateItem: vi.fn().mockReturnValue(null) } as Partial<TodoService>);
expect(thrown(() => new TodoController(svc).update(user, '5', '9', { name: 'X' }))).toEqual({
status: 404, body: { error: 'Item not found' },
});
});
it('updates, forwards changed keys, normalizes checked to 0/1, broadcasts', () => {
const updateItem = vi.fn().mockReturnValue({ id: 9 });
const broadcast = vi.fn();
const svc = makeService({ updateItem, broadcast } as Partial<TodoService>);
new TodoController(svc).update(user, '5', '9', { checked: true }, 'sock');
expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ checked: 1 }), ['checked']);
expect(broadcast).toHaveBeenCalledWith('5', 'todo:updated', { item: { id: 9 } }, 'sock');
});
it('normalizes checked false to 0', () => {
const updateItem = vi.fn().mockReturnValue({ id: 9 });
const svc = makeService({ updateItem, broadcast: vi.fn() } as Partial<TodoService>);
new TodoController(svc).update(user, '5', '9', { checked: false });
expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ checked: 0 }), ['checked']);
});
});
describe('DELETE /:id', () => {
it('404 when item missing', () => {
const svc = makeService({ deleteItem: vi.fn().mockReturnValue(false) } as Partial<TodoService>);
expect(thrown(() => new TodoController(svc).remove(user, '5', '9'))).toEqual({
status: 404, body: { error: 'Item not found' },
});
});
it('deletes and broadcasts', () => {
const deleteItem = vi.fn().mockReturnValue(true);
const broadcast = vi.fn();
const svc = makeService({ deleteItem, broadcast } as Partial<TodoService>);
expect(new TodoController(svc).remove(user, '5', '9', 'sock')).toEqual({ success: true });
expect(broadcast).toHaveBeenCalledWith('5', 'todo:deleted', { itemId: 9 }, 'sock');
});
});
it('PUT /reorder succeeds with permission', () => {
const reorderItems = vi.fn();
const svc = makeService({ reorderItems } as Partial<TodoService>);
expect(new TodoController(svc).reorder(user, '5', { orderedIds: [3, 1, 2] })).toEqual({ success: true });
expect(reorderItems).toHaveBeenCalledWith('5', [3, 1, 2]);
});
describe('category assignees', () => {
it('GET returns assignees', () => {
const svc = makeService({ getCategoryAssignees: vi.fn().mockReturnValue([{ user_id: 2 }]) } as Partial<TodoService>);
expect(new TodoController(svc).categoryAssignees(user, '5')).toEqual({ assignees: [{ user_id: 2 }] });
});
it('PUT updates, decodes the category and broadcasts', () => {
const updateCategoryAssignees = vi.fn().mockReturnValue([{ user_id: 2 }]);
const broadcast = vi.fn();
const svc = makeService({ updateCategoryAssignees, broadcast } as Partial<TodoService>);
new TodoController(svc).updateCategoryAssignees(user, '5', 'To%20Buy', { user_ids: [2] }, 'sock');
expect(updateCategoryAssignees).toHaveBeenCalledWith('5', 'To Buy', [2]);
expect(broadcast).toHaveBeenCalledWith('5', 'todo:assignees', { category: 'To Buy', assignees: [{ user_id: 2 }] }, 'sock');
});
});
});