From 6d1d338a4576c76dab9116b9d23e2356ffc32cb2 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Fri, 5 Jun 2026 20:40:09 -0400 Subject: [PATCH] =?UTF-8?q?test(03-09):=20add=20RED=20OIDC=20path=20tests?= =?UTF-8?q?=20=E2=80=94=20resolveUserId=20must=20call=20upsertUser=20(CR-0?= =?UTF-8?q?6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - POST /create with valid OIDC session (devBypassInjectUser.active=false, getAuth returns valid iss/sub) must return 202 not 401 - POST /create with no session (getAuth=null) must return 401 - Refactor getAuth/devBypass mocks to use vi.hoisted configurable flags for per-test OIDC path isolation - Mock upsertUser from auth/user.js so OIDC resolution can be verified --- apps/api/tests/routes/events.test.ts | 112 +++++++++++++++++++++++++-- 1 file changed, 107 insertions(+), 5 deletions(-) diff --git a/apps/api/tests/routes/events.test.ts b/apps/api/tests/routes/events.test.ts index 67d3c8a..1a2a082 100644 --- a/apps/api/tests/routes/events.test.ts +++ b/apps/api/tests/routes/events.test.ts @@ -25,12 +25,33 @@ import { SAMPLE_VEVENT_RECURRING_ALLDAY, } from '../helpers/db.js' +// --------------------------------------------------------------------------- +// Configurable getAuth and devBypass so individual tests can control the auth +// path without reloading the module. Uses vi.hoisted() to avoid TDZ issues +// (D-03-04-hoisting: vi.hoisted() required when test file has static imports of +// modules that vi.mock() references in their factory callbacks). +// --------------------------------------------------------------------------- + +// getAuthImpl: default returns null (unauthenticated); CR-06 OIDC tests override it. +const { getAuthImpl, devBypassInjectUser } = vi.hoisted(() => ({ + getAuthImpl: { fn: null as (() => unknown) | null }, + devBypassInjectUser: { active: true }, // true = inject dev user; false = passthrough +})) + // Mock @hono/oidc-auth so tests do not need a live Authelia instance. // The mock makes oidcAuthMiddleware a no-op passthrough. +// getAuth delegates to getAuthImpl.fn so per-test overrides work at call time. vi.mock('@hono/oidc-auth', () => ({ oidcAuthMiddleware: () => async (_c: unknown, next: () => Promise) => next(), processOAuthCallback: () => async (c: { json: (v: unknown) => unknown }) => c.json({ ok: true }), - getAuth: () => null, + getAuth: () => (getAuthImpl.fn ? getAuthImpl.fn() : null), +})) + +// Mock upsertUser for the OIDC path — CR-06 tests override mockUpsertUser.fn. +const mockUpsertUserFn = vi.fn() +vi.mock('../../src/auth/user.js', () => ({ + upsertUser: (...args: unknown[]) => mockUpsertUserFn(...args), + COLOR_PALETTE: ['#4A90D9', '#E8734A', '#5BA85A', '#9B6DC5', '#E8A840', '#3AAFA9'], })) // --------------------------------------------------------------------------- @@ -81,6 +102,11 @@ beforeEach(() => { mockDbRows = [] vi.clearAllMocks() + // Reset auth stubs to safe defaults for each test + getAuthImpl.fn = null // getAuth returns null (unauthenticated) + devBypassInjectUser.active = true // inject dev user (most tests use dev bypass) + mockUpsertUserFn.mockResolvedValue({ id: 42, oidcIss: 'https://auth.example.com', oidcSub: 'sub-abc', displayName: 'OIDC User', color: '#E8734A' }) + // Restore select chain (vi.clearAllMocks wipes mockImplementation) mockWhereFn.mockImplementation(() => Promise.resolve(mockDbRows)) mockInnerJoin2Fn.mockReturnValue({ where: mockWhereFn }) @@ -243,15 +269,16 @@ describe('GET /api/events', () => { // - We mock '../auth/devBypass.js' to inject a fixed user into context. // --------------------------------------------------------------------------- -// Mock devAuthBypass to inject a fixed dev user in ALL test requests. -// This mirrors what DEV_AUTH_BYPASS=true does in the real app but without -// requiring env-var manipulation across test isolation. +// Mock devAuthBypass: delegates to devBypassInjectUser.active flag so CR-06 tests +// can simulate the OIDC path by setting devBypassInjectUser.active = false. vi.mock('../../src/auth/devBypass.js', async (importOriginal) => { const original = await importOriginal() return { ...original, devAuthBypass: () => async (c: { set: (k: string, v: unknown) => void }, next: () => Promise) => { - c.set('user', { id: 1, oidcIss: 'dev', oidcSub: 'dev-user', displayName: 'Dev User', color: '#4A90D9' }) + if (devBypassInjectUser.active) { + c.set('user', { id: 1, oidcIss: 'dev', oidcSub: 'dev-user', displayName: 'Dev User', color: '#4A90D9' }) + } await next() }, } @@ -525,6 +552,81 @@ describe('CR-01: canonical client payload (title/start/end) accepted by server', }) }) +// --------------------------------------------------------------------------- +// CR-06 contract tests — real OIDC iss/sub → users.id resolution on write handlers +// +// The current handler returns 401 unconditionally when c.get('user') is null, +// even if getAuth() returns a valid OIDC session. These tests assert: +// - OIDC session (no dev bypass) → resolveUserId calls upsertUser → 202 (not 401) +// - No session at all → 401 (still unauthenticated) +// +// RED before Task 2 resolveUserId async rewrite; GREEN after. +// --------------------------------------------------------------------------- +describe('CR-06: OIDC iss/sub → users.id resolution on write handlers', () => { + beforeEach(() => { + // Disable dev bypass — simulate production OIDC path + devBypassInjectUser.active = false + + // Seed a calendar row for the create endpoint's ownership check + mockDbRows = [ + { + id: 1, + url: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/', + displayName: 'Default', + color: '#4A90D9', + userId: 42, // matches the upserted user id + isShared: false, + }, + ] + const mockSimpleWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows)) + mockFromFn.mockReturnValue({ where: mockSimpleWhere }) + mockSelectFn.mockReturnValue({ from: mockFromFn }) + }) + + it('POST /create with valid OIDC session (no dev bypass) returns 202, not 401', async () => { + // getAuth returns a real OIDC payload — upsertUser resolves user.id = 42 + getAuthImpl.fn = () => ({ iss: 'https://auth.example.com', sub: 'sub-abc', email: 'user@example.com' }) + mockUpsertUserFn.mockResolvedValue({ id: 42, oidcIss: 'https://auth.example.com', oidcSub: 'sub-abc', displayName: 'OIDC User', color: '#E8734A' }) + + const { app } = await import('../../src/index.js') + const res = await app.request('/api/events/create', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + title: 'OIDC event', + allDay: false, + start: '2026-06-15T10:00:00Z', + end: '2026-06-15T10:30:00Z', + recurrence: 'none', + calendarUrl: 'https://caldav.fastmail.com/dav/calendars/user/test@fm.com/Default/', + }), + }) + // Handler must NOT return 401 — the OIDC session is valid and resolves a user + expect(res.status).toBe(202) + // upsertUser was called with the OIDC iss/sub + expect(mockUpsertUserFn).toHaveBeenCalledWith('https://auth.example.com', 'sub-abc', 'user@example.com') + }) + + it('POST /create with no session (getAuth returns null) returns 401', async () => { + // getAuth returns null → no session; should return 401 + getAuthImpl.fn = () => null + + const { app } = await import('../../src/index.js') + const res = await app.request('/api/events/create', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + title: 'Unauthenticated event', + allDay: false, + start: '2026-06-15T10:00:00Z', + end: '2026-06-15T10:30:00Z', + recurrence: 'none', + }), + }) + expect(res.status).toBe(401) + }) +}) + // --------------------------------------------------------------------------- // GET /api/events/writable-calendars // ---------------------------------------------------------------------------