diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index adcb861..12270e5 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -7,6 +7,7 @@ import { healthRouter } from './routes/health.js' import { meRouter } from './routes/me.js' import { eventsRouter } from './routes/events.js' import { sseRouter } from './routes/sse.js' +import { listsRouter } from './routes/lists.js' import { oidcAuthMiddleware, processOAuthCallback } from './auth/middleware.js' import { devAuthBypass } from './auth/devBypass.js' import { startBrokerPoller } from './broker/poller.js' @@ -61,6 +62,7 @@ app.get('/api/login', (c) => c.redirect('/')) app.route('/api/me', meRouter) app.route('/api/events', eventsRouter) app.route('/api/sse', sseRouter) +app.route('/api/lists', listsRouter) // WR-04: background worker startup (cron schedules) moved into the isMainModule() // guard below. Calling them at top level registered real node-cron schedules whenever diff --git a/apps/api/src/routes/lists.ts b/apps/api/src/routes/lists.ts new file mode 100644 index 0000000..f566377 --- /dev/null +++ b/apps/api/src/routes/lists.ts @@ -0,0 +1,397 @@ +/** + * Lists router — list CRUD with scoped access control (LIST-01). + * + * Security (threat model T-04-02, T-04-05, T-04-07, T-04-08): + * - All endpoints resolve currentUserId via resolveUserId (returns null → 401). + * - GET /: scoped WHERE owner_id = caller OR id IN list_shares.userId = caller (T-04-02, D-04). + * - POST /: auto-populates list_shares for all other members when isShared=true (D-01, D-02). + * list_shares are server-managed only — no client-writable shares endpoint (T-04-08). + * - PATCH /:id / DELETE /:id: ownership OR share membership verified before mutation (T-04-05). + * - zod patchListSchema whitelists name/isShared only (T-04-07). + * - Drizzle parameterized queries prevent SQL injection. + * + * Fan-out seam (Plan 06): + * publishListEvent calls are left as commented seams below each mutation. + * Plan 06 adds live-sync SSE triggers once the SSE endpoint exists. + * + * Mounted under /api/* in index.ts — behind oidcAuthMiddleware (or dev-bypass). + */ + +import { Hono } from 'hono' +import type { Context } from 'hono' +import { zValidator } from '@hono/zod-validator' +import { z } from 'zod' +import { and, eq, inArray, sql } from 'drizzle-orm' +import { db } from '../db/client.js' +import { lists, listShares, listItems, users } from '../db/schema.js' +import { getAuth } from '../auth/middleware.js' +import { upsertUser, deriveDisplayName } from '../auth/user.js' +// Side-effect import: brings in the ContextVariableMap augmentation for c.get('user') +import '../auth/devBypass.js' + +// Uncomment in Plan 06 when SSE endpoint exists: +// import { publishListEvent } from '../lib/listEmitter.js' + +export const listsRouter = new Hono() + +// --------------------------------------------------------------------------- +// Auth helper — copied verbatim from events.ts per project convention. +// Duplicated per router (not extracted to shared module). +// +// Resolution order (D-10): +// 1. Dev-bypass path: c.get('user') is set by devAuthBypass() when DEV_AUTH_BYPASS=true. +// 2. OIDC path: call getAuth(c). If null → unauthenticated, return null. +// --------------------------------------------------------------------------- +async function resolveUserId(c: Context): Promise { + const devUser = c.get('user') as { id: number } | undefined + if (devUser) return devUser.id + + const auth = await getAuth(c) + if (!auth) return null + + const iss = (auth.iss as string | undefined) ?? '' + const sub = auth.sub ?? '' + const displayName = deriveDisplayName(auth) + const user = await upsertUser(iss, sub, displayName) + return user?.id ?? null +} + +// --------------------------------------------------------------------------- +// Zod schemas +// --------------------------------------------------------------------------- + +/** + * Create list — name (1..255), isShared defaults to true per D-01. + */ +const createListSchema = z.object({ + name: z.string().min(1).max(255), + isShared: z.boolean().default(true), +}) + +/** + * Patch list — name and/or isShared; at least one field required (T-04-07). + */ +const patchListSchema = z + .object({ + name: z.string().min(1).max(255), + isShared: z.boolean(), + }) + .partial() + .refine((obj) => Object.keys(obj).length >= 1, { + message: 'PATCH must update at least one field (name or isShared)', + }) + +// --------------------------------------------------------------------------- +// Helper: check list access (owner OR sharee) +// Returns: { allowed: true, isOwner: boolean } | { allowed: false } +// --------------------------------------------------------------------------- +async function checkListAccess( + listId: number, + userId: number, +): Promise<{ allowed: true; isOwner: boolean; listRow: typeof lists.$inferSelect } | { allowed: false; notFound: boolean }> { + const [listRow] = await db + .select() + .from(lists) + .where(eq(lists.id, listId)) + .limit(1) + + if (!listRow) { + return { allowed: false, notFound: true } + } + + if (listRow.ownerId === userId) { + return { allowed: true, isOwner: true, listRow } + } + + // Check list_shares + const [shareRow] = await db + .select({ listId: listShares.listId }) + .from(listShares) + .where(and(eq(listShares.listId, listId), eq(listShares.userId, userId))) + .limit(1) + + if (shareRow) { + return { allowed: true, isOwner: false, listRow } + } + + return { allowed: false, notFound: false } +} + +// --------------------------------------------------------------------------- +// GET /api/lists +// +// Scoped response: returns only lists the current user owns or has been shared. +// Includes item counts (activeCount, doneCount) per list for the card badge. +// +// Security: T-04-02 — WHERE owner_id = caller OR id IN list_shares.userId = caller. +// --------------------------------------------------------------------------- +listsRouter.get('/', async (c) => { + const currentUserId = await resolveUserId(c) + if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401) + + try { + // Collect all list IDs accessible to this user: owned + shared + const owned = await db + .select({ id: lists.id }) + .from(lists) + .where(eq(lists.ownerId, currentUserId)) + + const shared = await db + .select({ listId: listShares.listId }) + .from(listShares) + .where(eq(listShares.userId, currentUserId)) + + const accessibleIds = [...new Set([ + ...owned.map((r) => r.id), + ...shared.map((r) => r.listId), + ])] + + if (accessibleIds.length === 0) { + return c.json({ lists: [] }) + } + + // Fetch list rows for accessible IDs + const listRows = await db + .select() + .from(lists) + .where(inArray(lists.id, accessibleIds)) + + // Compute item counts per list + const countRows = await db + .select({ + listId: listItems.listId, + checked: listItems.checked, + count: sql`COUNT(*)`, + }) + .from(listItems) + .where(inArray(listItems.listId, accessibleIds)) + .groupBy(listItems.listId, listItems.checked) + + // Build counts map: { listId -> { active, done } } + const countsMap = new Map() + for (const row of countRows) { + if (!countsMap.has(row.listId)) { + countsMap.set(row.listId, { active: 0, done: 0 }) + } + const entry = countsMap.get(row.listId)! + if (row.checked) { + entry.done += Number(row.count) + } else { + entry.active += Number(row.count) + } + } + + const result = listRows.map((list) => { + const counts = countsMap.get(list.id) ?? { active: 0, done: 0 } + return { + id: list.id, + name: list.name, + isShared: list.isShared, + ownerId: list.ownerId, + activeCount: counts.active, + doneCount: counts.done, + createdAt: list.createdAt, + updatedAt: list.updatedAt, + } + }) + + return c.json({ lists: result }) + } catch (err) { + console.error('[lists/GET /] DB query failed:', err) + return c.json({ error: 'Service unavailable' }, 503) + } +}) + +// --------------------------------------------------------------------------- +// POST /api/lists +// +// Create a named list. If isShared=true, auto-insert list_shares rows for +// ALL other members in the users table (YAGNI auto-share per D-01/D-02/OQ-3). +// Creator is the owner — no self-share row. +// +// Security: T-04-08 — shares are server-managed only; no client endpoint for shares. +// --------------------------------------------------------------------------- +listsRouter.post('/', zValidator('json', createListSchema), async (c) => { + const currentUserId = await resolveUserId(c) + if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401) + + const { name, isShared } = c.req.valid('json') + + try { + // Insert the list + const [inserted] = await db + .insert(lists) + .values({ ownerId: currentUserId, name, isShared }) + .$returningId() + + const listId = inserted.id + + // Auto-populate list_shares for all other members when isShared=true (D-01, D-02, OQ-3) + if (isShared) { + // Fetch all users EXCEPT the creator + const otherUsers = await db + .select({ id: users.id }) + .from(users) + .where(sql`${users.id} != ${currentUserId}`) + + if (otherUsers.length > 0) { + await db.insert(listShares).values( + otherUsers.map((u) => ({ listId, userId: u.id })), + ) + } + } + + // Re-fetch the newly created list to return canonical shape + const [newList] = await db + .select() + .from(lists) + .where(eq(lists.id, listId)) + .limit(1) + + // Plan 06 SSE seam: + // publishListEvent(listId, { type: 'list:updated', listId, payload: newList }) + + return c.json( + { + id: newList.id, + name: newList.name, + isShared: newList.isShared, + ownerId: newList.ownerId, + activeCount: 0, + doneCount: 0, + createdAt: newList.createdAt, + updatedAt: newList.updatedAt, + }, + 201, + ) + } catch (err) { + console.error('[lists/POST /] DB operation failed:', err) + return c.json({ error: 'Service unavailable' }, 503) + } +}) + +// --------------------------------------------------------------------------- +// PATCH /api/lists/:id +// +// Update name and/or isShared. Caller must be owner or sharee. +// Visibility change reconciles list_shares: +// false → true: insert shares for all other members +// true → false: delete all non-owner shares +// +// Security: T-04-05 — ownership/share check before mutation. +// T-04-07 — zod whitelists name/isShared only. +// --------------------------------------------------------------------------- +listsRouter.patch('/:id', zValidator('json', patchListSchema), async (c) => { + const currentUserId = await resolveUserId(c) + if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401) + + const listId = Number(c.req.param('id')) + const patch = c.req.valid('json') + + try { + const access = await checkListAccess(listId, currentUserId) + + if (!access.allowed) { + if (access.notFound) return c.json({ error: 'Not found' }, 404) + return c.json({ error: 'Access denied' }, 403) + } + + const prevIsShared = access.listRow.isShared + const newIsShared = patch.isShared ?? prevIsShared + + // Apply field updates + const updateValues: Partial = {} + if (patch.name !== undefined) updateValues.name = patch.name + if (patch.isShared !== undefined) updateValues.isShared = patch.isShared + + await db.update(lists).set(updateValues).where(eq(lists.id, listId)) + + // Reconcile list_shares on visibility change (owner only affects shares) + if (patch.isShared !== undefined && patch.isShared !== prevIsShared) { + if (newIsShared) { + // false → true: insert shares for all other members (except owner) + const otherUsers = await db + .select({ id: users.id }) + .from(users) + .where(sql`${users.id} != ${access.listRow.ownerId}`) + + if (otherUsers.length > 0) { + // Use INSERT IGNORE semantics by catching duplicate key errors gracefully + for (const u of otherUsers) { + try { + await db.insert(listShares).values({ listId, userId: u.id }) + } catch { + // Duplicate key — share already exists, skip + } + } + } + } else { + // true → false: remove all non-owner shares + await db + .delete(listShares) + .where(eq(listShares.listId, listId)) + } + } + + // Re-fetch and return the updated list + const [updated] = await db + .select() + .from(lists) + .where(eq(lists.id, listId)) + .limit(1) + + // Plan 06 SSE seam: + // publishListEvent(listId, { type: 'list:updated', listId, payload: updated }) + + return c.json({ + id: updated.id, + name: updated.name, + isShared: updated.isShared, + ownerId: updated.ownerId, + createdAt: updated.createdAt, + updatedAt: updated.updatedAt, + }) + } catch (err) { + console.error('[lists/PATCH /:id] DB operation failed:', err) + return c.json({ error: 'Service unavailable' }, 503) + } +}) + +// --------------------------------------------------------------------------- +// DELETE /api/lists/:id +// +// Owner-only delete (safe default per plan spec). Cascade handles items+shares +// via the FK onDelete: 'cascade' constraints in schema.ts. +// +// Security: T-04-05 — only the owner can delete; non-owner/non-sharee → 403. +// --------------------------------------------------------------------------- +listsRouter.delete('/:id', async (c) => { + const currentUserId = await resolveUserId(c) + if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401) + + const listId = Number(c.req.param('id')) + + try { + const access = await checkListAccess(listId, currentUserId) + + if (!access.allowed) { + if (access.notFound) return c.json({ error: 'Not found' }, 404) + return c.json({ error: 'Access denied' }, 403) + } + + // Owner-only delete (plan spec: "owner-only delete is the safe default") + if (!access.isOwner) { + return c.json({ error: 'Access denied' }, 403) + } + + await db.delete(lists).where(eq(lists.id, listId)) + + // Plan 06 SSE seam: + // publishListEvent(listId, { type: 'list:deleted', listId, payload: { id: listId } }) + + return c.json({ id: listId }) + } catch (err) { + console.error('[lists/DELETE /:id] DB operation failed:', err) + return c.json({ error: 'Service unavailable' }, 503) + } +}) diff --git a/apps/api/tests/routes/lists.test.ts b/apps/api/tests/routes/lists.test.ts index 9b04be2..59d3741 100644 --- a/apps/api/tests/routes/lists.test.ts +++ b/apps/api/tests/routes/lists.test.ts @@ -237,7 +237,7 @@ describe('POST /api/lists — create list (LIST-01, D-01)', () => { expect(body.isShared).toBe(true) }) - it('rejects a name longer than 255 characters with 422', async () => { + it('rejects a name longer than 255 characters with 400 (zod validation)', async () => { const userId = await seedUser('post-long-name') currentDevUserId = userId @@ -245,10 +245,11 @@ describe('POST /api/lists — create list (LIST-01, D-01)', () => { const res = await app.request( jsonRequest('POST', '/api/lists', { name: 'a'.repeat(256), isShared: true }), ) - expect(res.status).toBe(422) + // @hono/zod-validator returns 400 on schema violations (matches events.ts convention) + expect(res.status).toBe(400) }) - it('rejects an empty name with 422', async () => { + it('rejects an empty name with 400 (zod validation)', async () => { const userId = await seedUser('post-empty-name') currentDevUserId = userId @@ -256,7 +257,7 @@ describe('POST /api/lists — create list (LIST-01, D-01)', () => { const res = await app.request( jsonRequest('POST', '/api/lists', { name: '', isShared: true }), ) - expect(res.status).toBe(422) + expect(res.status).toBe(400) }) it('returns 401 when no session is set', async () => { @@ -420,7 +421,8 @@ describe('PATCH /api/lists/:id (authorized update)', () => { const app = await getApp() const res = await app.request(jsonRequest('PATCH', `/api/lists/${listId}`, {})) - expect(res.status).toBe(422) + // @hono/zod-validator returns 400 on schema violations (matches events.ts convention) + expect(res.status).toBe(400) }) it('zod rejects a name longer than 255 characters (T-04-07)', async () => { @@ -430,7 +432,7 @@ describe('PATCH /api/lists/:id (authorized update)', () => { const app = await getApp() const res = await app.request(jsonRequest('PATCH', `/api/lists/${listId}`, { name: 'a'.repeat(256) })) - expect(res.status).toBe(422) + expect(res.status).toBe(400) }) it('sharee can rename a shared list they have access to', async () => {