diff --git a/.claude/settings.json b/.claude/settings.json index 5b77917c..d705eebb 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,4 +1,7 @@ { + "env": { + "CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS": "1" + }, "hooks": { "PostToolUse": [ { diff --git a/.gitignore b/.gitignore index 63c03913..b7bf7b89 100644 --- a/.gitignore +++ b/.gitignore @@ -15,5 +15,6 @@ yarn-error.log yarn.lock playwright-report +.playwright-mcp test-results .vscode \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index d1b98bfa..b038a574 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,6 +116,14 @@ src/ └── app.html # HTML template ``` +## Browser Testing (Playwright MCP) +- Always use **Firefox** as the browser — Chrome is not installed +- Dev environment runs on `http://localhost:8080` (Caddy proxy) with Go backend on :8081 and Vite on :5173 +- Start with `make run-web` from the Coves backend repo + +## Sub-Agent Pattern +When using the Task tool to launch multiple agents, prefer **foreground** calls (no `run_in_background`). Multiple foreground Task calls in a single message run concurrently while keeping the main agent active to report results automatically. Background agents go idle and require manual check-ins. + ## Success Metrics Your code is ready when: - [ ] `pnpm check` passes diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e5736ce4..78e00bb6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -655,66 +655,79 @@ packages: resolution: {integrity: sha512-F8sWbhZ7tyuEfsmOxwc2giKDQzN3+kuBLPwwZGyVkLlKGdV1nvnNwYD0fKQ8+XS6hp9nY7B+ZeK01EBUE7aHaw==} cpu: [arm] os: [linux] + libc: [glibc] '@rollup/rollup-linux-arm-musleabihf@4.57.1': resolution: {integrity: sha512-rGfNUfn0GIeXtBP1wL5MnzSj98+PZe/AXaGBCRmT0ts80lU5CATYGxXukeTX39XBKsxzFpEeK+Mrp9faXOlmrw==} cpu: [arm] os: [linux] + libc: [musl] '@rollup/rollup-linux-arm64-gnu@4.57.1': resolution: {integrity: sha512-MMtej3YHWeg/0klK2Qodf3yrNzz6CGjo2UntLvk2RSPlhzgLvYEB3frRvbEF2wRKh1Z2fDIg9KRPe1fawv7C+g==} cpu: [arm64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-arm64-musl@4.57.1': resolution: {integrity: sha512-1a/qhaaOXhqXGpMFMET9VqwZakkljWHLmZOX48R0I/YLbhdxr1m4gtG1Hq7++VhVUmf+L3sTAf9op4JlhQ5u1Q==} cpu: [arm64] os: [linux] + libc: [musl] '@rollup/rollup-linux-loong64-gnu@4.57.1': resolution: {integrity: sha512-QWO6RQTZ/cqYtJMtxhkRkidoNGXc7ERPbZN7dVW5SdURuLeVU7lwKMpo18XdcmpWYd0qsP1bwKPf7DNSUinhvA==} cpu: [loong64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-loong64-musl@4.57.1': resolution: {integrity: sha512-xpObYIf+8gprgWaPP32xiN5RVTi/s5FCR+XMXSKmhfoJjrpRAjCuuqQXyxUa/eJTdAE6eJ+KDKaoEqjZQxh3Gw==} cpu: [loong64] os: [linux] + libc: [musl] '@rollup/rollup-linux-ppc64-gnu@4.57.1': resolution: {integrity: sha512-4BrCgrpZo4hvzMDKRqEaW1zeecScDCR+2nZ86ATLhAoJ5FQ+lbHVD3ttKe74/c7tNT9c6F2viwB3ufwp01Oh2w==} cpu: [ppc64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-ppc64-musl@4.57.1': resolution: {integrity: sha512-NOlUuzesGauESAyEYFSe3QTUguL+lvrN1HtwEEsU2rOwdUDeTMJdO5dUYl/2hKf9jWydJrO9OL/XSSf65R5+Xw==} cpu: [ppc64] os: [linux] + libc: [musl] '@rollup/rollup-linux-riscv64-gnu@4.57.1': resolution: {integrity: sha512-ptA88htVp0AwUUqhVghwDIKlvJMD/fmL/wrQj99PRHFRAG6Z5nbWoWG4o81Nt9FT+IuqUQi+L31ZKAFeJ5Is+A==} cpu: [riscv64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-riscv64-musl@4.57.1': resolution: {integrity: sha512-S51t7aMMTNdmAMPpBg7OOsTdn4tySRQvklmL3RpDRyknk87+Sp3xaumlatU+ppQ+5raY7sSTcC2beGgvhENfuw==} cpu: [riscv64] os: [linux] + libc: [musl] '@rollup/rollup-linux-s390x-gnu@4.57.1': resolution: {integrity: sha512-Bl00OFnVFkL82FHbEqy3k5CUCKH6OEJL54KCyx2oqsmZnFTR8IoNqBF+mjQVcRCT5sB6yOvK8A37LNm/kPJiZg==} cpu: [s390x] os: [linux] + libc: [glibc] '@rollup/rollup-linux-x64-gnu@4.57.1': resolution: {integrity: sha512-ABca4ceT4N+Tv/GtotnWAeXZUZuM/9AQyCyKYyKnpk4yoA7QIAuBt6Hkgpw8kActYlew2mvckXkvx0FfoInnLg==} cpu: [x64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-x64-musl@4.57.1': resolution: {integrity: sha512-HFps0JeGtuOR2convgRRkHCekD7j+gdAuXM+/i6kGzQtFhlCtQkpwtNzkNj6QhCDp7DRJ7+qC/1Vg2jt5iSOFw==} cpu: [x64] os: [linux] + libc: [musl] '@rollup/rollup-openbsd-x64@4.57.1': resolution: {integrity: sha512-H+hXEv9gdVQuDTgnqD+SQffoWoc0Of59AStSzTEj/feWTBAnSfSD3+Dql1ZruJQxmykT/JVY0dE8Ka7z0DH1hw==} @@ -873,24 +886,28 @@ packages: engines: {node: '>= 10'} cpu: [arm64] os: [linux] + libc: [glibc] '@tailwindcss/oxide-linux-arm64-musl@4.1.18': resolution: {integrity: sha512-1px92582HkPQlaaCkdRcio71p8bc8i/ap5807tPRDK/uw953cauQBT8c5tVGkOwrHMfc2Yh6UuxaH4vtTjGvHg==} engines: {node: '>= 10'} cpu: [arm64] os: [linux] + libc: [musl] '@tailwindcss/oxide-linux-x64-gnu@4.1.18': resolution: {integrity: sha512-v3gyT0ivkfBLoZGF9LyHmts0Isc8jHZyVcbzio6Wpzifg/+5ZJpDiRiUhDLkcr7f/r38SWNe7ucxmGW3j3Kb/g==} engines: {node: '>= 10'} cpu: [x64] os: [linux] + libc: [glibc] '@tailwindcss/oxide-linux-x64-musl@4.1.18': resolution: {integrity: sha512-bhJ2y2OQNlcRwwgOAGMY0xTFStt4/wyU6pvI6LSuZpRgKQwxTec0/3Scu91O8ir7qCR3AuepQKLU/kX99FouqQ==} engines: {node: '>= 10'} cpu: [x64] os: [linux] + libc: [musl] '@tailwindcss/oxide-wasm32-wasi@4.1.18': resolution: {integrity: sha512-LffYTvPjODiP6PT16oNeUQJzNVyJl1cjIebq/rWWBF+3eDst5JGEFSc5cWxyRCJ0Mxl+KyIkqRxk1XPEs9x8TA==} @@ -1593,24 +1610,28 @@ packages: engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] + libc: [glibc] lightningcss-linux-arm64-musl@1.30.2: resolution: {integrity: sha512-5Vh9dGeblpTxWHpOx8iauV02popZDsCYMPIgiuw97OJ5uaDsL86cnqSFs5LZkG3ghHoX5isLgWzMs+eD1YzrnA==} engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] + libc: [musl] lightningcss-linux-x64-gnu@1.30.2: resolution: {integrity: sha512-Cfd46gdmj1vQ+lR6VRTTadNHu6ALuw2pKR9lYq4FnhvgBc4zWY1EtZcAc6EffShbb1MFrIPfLDXD6Xprbnni4w==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] + libc: [glibc] lightningcss-linux-x64-musl@1.30.2: resolution: {integrity: sha512-XJaLUUFXb6/QG2lGIW6aIk6jKdtjtcffUT0NKvIqhSBY3hh9Ch+1LCeH80dR9q9LBjG3ewbDjnumefsLsP6aiA==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] + libc: [musl] lightningcss-win32-arm64-msvc@1.30.2: resolution: {integrity: sha512-FZn+vaj7zLv//D/192WFFVA0RgHawIcHqLX9xuWiQt7P0PtdFEVaxgF9rjM/IRYHQXNnk61/H/gb2Ei+kUQ4xQ==} diff --git a/src/app.d.ts b/src/app.d.ts index 9c1c427a..3756baa9 100644 --- a/src/app.d.ts +++ b/src/app.d.ts @@ -1,7 +1,7 @@ // See https://kit.svelte.dev/docs/types#app import type { Component } from 'svelte' -import type { AccountSession, AppSession, SealedToken } from '$lib/server/session' +import type { AccountSession, SealedToken } from '$lib/server/session' // for information about these interfaces declare global { @@ -21,11 +21,14 @@ declare global { */ interface AuthenticatedAuth { readonly authenticated: true - /** The complete session with all accounts */ - readonly session: AppSession - /** The currently active account */ - readonly activeAccount: AccountSession - /** The sealed token for API requests */ + /** The authenticated account */ + readonly account: AccountSession + /** + * Convenience alias for `account.sealedToken`. + * Duplicated at the top level so the proxy layer (`/api/proxy/[...path]`) + * can read the token directly from `locals.auth.authToken` without + * reaching into the nested account object on every proxied request. + */ readonly authToken: SealedToken } @@ -36,13 +39,24 @@ declare global { * @example * ```typescript * if (locals.auth.authenticated) { - * // TypeScript knows session, activeAccount, and authToken exist - * console.log(locals.auth.session.activeAccountId) + * // TypeScript knows account and authToken exist + * console.log(locals.auth.account.did) * } * ``` */ type AuthState = UnauthenticatedAuth | AuthenticatedAuth + /** + * Categories of authentication errors that can occur during session validation. + * These allow downstream code (layouts, pages) to show appropriate user feedback. + * + * - 'network_error': Infrastructure failure (DNS, TLS, timeout, connection refused). + * The session cookie is preserved because the error may be temporary. + * - 'validation_error': The /api/me response was received but contained invalid data. + * Indicates a server-side bug or protocol mismatch. + */ + type AuthErrorKind = 'network_error' | 'validation_error' + /** * Server-side request-local state populated by hooks.server.ts. * @@ -52,6 +66,14 @@ declare global { */ interface Locals { auth: AuthState + /** + * Set when authentication failed due to an infrastructure or validation error + * (as opposed to simply not having a session cookie). + * The layout can use this to show a warning banner to the user. + */ + authError?: AuthErrorKind + /** Set to true when a 401 from /api/me indicates the session has expired or been revoked */ + sessionExpired?: boolean } interface PageData { slots?: { diff --git a/src/hooks.server.test.ts b/src/hooks.server.test.ts index 72dfcd95..10b757f9 100644 --- a/src/hooks.server.test.ts +++ b/src/hooks.server.test.ts @@ -1,55 +1,38 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import type { Cookies, RequestEvent } from '@sveltejs/kit' -import { - asDID, - asHandle, - asInstanceURL, - asSealedToken, - asSessionId, - type AppSession, - type AccountId, -} from '$lib/server/session' - -// 32-byte hex key (64 characters) for testing -const TEST_SECRET = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2' - -// Valid AccountIds (32 hex characters) for testing -const TEST_ACCOUNT_ID_1 = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' as AccountId -const TEST_ACCOUNT_ID_2 = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' as AccountId - -// Variable to control the mocked SESSION_SECRET -let mockSessionSecret: string | undefined = TEST_SECRET + +// Variable to control the mocked instance URL +let mockPublicInternalInstance: string | undefined = 'http://localhost:4000' +let mockPublicInstanceUrl: string | undefined = undefined // Mock environment variables -vi.mock('$env/dynamic/private', () => ({ +vi.mock('$env/dynamic/public', () => ({ env: { - get SESSION_SECRET() { - return mockSessionSecret + get PUBLIC_INTERNAL_INSTANCE() { + return mockPublicInternalInstance + }, + get PUBLIC_INSTANCE_URL() { + return mockPublicInstanceUrl }, }, })) -// Mock decryptSession to control its behavior in tests -const mockDecryptSession = vi.fn() - -vi.mock('$lib/server/session', async () => { - const actual = await vi.importActual('$lib/server/session') - return { - ...actual, - decryptSession: (...args: unknown[]) => mockDecryptSession(...args), - } -}) - // Import handle and handleError after mocking const { handle, handleError } = await import('./hooks.server') +// Mock global fetch +const mockFetch = vi.fn() +vi.stubGlobal('fetch', mockFetch) + // Helper to create mock cookies -function createMockCookies(initialCookies: Record = {}): Cookies { +function createMockCookies( + initialCookies: Record = {}, +): Cookies { const store = new Map(Object.entries(initialCookies)) return { get: vi.fn((name: string) => store.get(name)), getAll: vi.fn(() => - Array.from(store.entries()).map(([name, value]) => ({ name, value })) + Array.from(store.entries()).map(([name, value]) => ({ name, value })), ), set: vi.fn((name: string, value: string) => { store.set(name, value) @@ -69,7 +52,6 @@ function createMockEvent(options: { locals?: App.Locals }): RequestEvent { const url = new URL('http://localhost:5173/') - // Default to unauthenticated state const defaultLocals: App.Locals = { auth: { authenticated: false } } return { request: new Request(url), @@ -97,314 +79,353 @@ function createMockResolve() { describe('hooks.server handle', () => { beforeEach(() => { vi.clearAllMocks() - mockSessionSecret = TEST_SECRET + mockPublicInternalInstance = 'http://localhost:4000' + mockPublicInstanceUrl = undefined }) - describe('valid session cookie', () => { - it('populates event.locals with session data when valid session cookie exists', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('sealed-token-123'), - sessionId: asSessionId('session-1'), - avatar: 'https://example.com/avatar.png', - }, - ], - } + describe('no coves_session cookie', () => { + it('results in unauthenticated state and no fetch called', async () => { + const cookies = createMockCookies({}) + const event = createMockEvent({ cookies }) + const resolve = createMockResolve() - mockDecryptSession.mockReturnValue(session) + await handle({ event, resolve }) - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', - }) + expect(mockFetch).not.toHaveBeenCalled() + expect(event.locals.auth.authenticated).toBe(false) + expect(event.locals.authError).toBeUndefined() + expect(resolve).toHaveBeenCalledWith(event) + }) + }) + describe('empty string coves_session cookie', () => { + it('treats empty string as no cookie and returns unauthenticated', async () => { + const cookies = createMockCookies({ coves_session: '' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - expect(mockDecryptSession).toHaveBeenCalledWith('encrypted-session-cookie', TEST_SECRET) - expect(event.locals.auth.authenticated).toBe(true) - if (event.locals.auth.authenticated) { - expect(event.locals.auth.session).toEqual(session) - expect(event.locals.auth.activeAccount).toEqual(session.accounts[0]) - expect(event.locals.auth.authToken).toBe('sealed-token-123') - } + // Empty string is falsy, so it's treated the same as no cookie + expect(event.locals.auth.authenticated).toBe(false) + expect(mockFetch).not.toHaveBeenCalled() expect(resolve).toHaveBeenCalledWith(event) }) + }) - it('populates event.locals with correct account when multiple accounts exist', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_2, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - { - id: TEST_ACCOUNT_ID_2, - did: asDID('did:plc:user2'), - handle: asHandle('user2.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }, - ], - } - - mockDecryptSession.mockReturnValue(session) - - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', - }) + describe('valid cookie and /api/me returns 200', () => { + it('populates authenticated state with correct account and authToken', async () => { + mockFetch.mockResolvedValue( + new Response( + JSON.stringify({ + did: 'did:plc:user1', + handle: 'user1.example.com', + avatar: 'https://example.com/avatar.png', + }), + { status: 200 }, + ), + ) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) + expect(mockFetch).toHaveBeenCalledWith('http://localhost:4000/api/me', { + headers: { Cookie: 'coves_session=sealed-token-value' }, + }) expect(event.locals.auth.authenticated).toBe(true) if (event.locals.auth.authenticated) { - expect(event.locals.auth.activeAccount.id).toBe(TEST_ACCOUNT_ID_2) - expect(event.locals.auth.authToken).toBe('token-2') + expect(event.locals.auth.account.did).toBe('did:plc:user1') + expect(event.locals.auth.account.handle).toBe('user1.example.com') + expect(event.locals.auth.account.instance).toBe('http://localhost:4000') + expect(event.locals.auth.account.sealedToken).toBe('sealed-token-value') + expect(event.locals.auth.account.avatar).toBe( + 'https://example.com/avatar.png', + ) + expect(event.locals.auth.authToken).toBe('sealed-token-value') } + expect(resolve).toHaveBeenCalledWith(event) }) }) - describe('missing SESSION_SECRET', () => { - it('throws fatal error when SESSION_SECRET is undefined and session cookie exists', async () => { - mockSessionSecret = undefined + describe('valid cookie and /api/me returns 401', () => { + it('results in unauthenticated state without console.warn', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', - }) + mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 })) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() - await expect(handle({ event, resolve })).rejects.toThrow( - '[FATAL] SESSION_SECRET environment variable is not set' - ) + await handle({ event, resolve }) - expect(mockDecryptSession).not.toHaveBeenCalled() + expect(event.locals.auth.authenticated).toBe(false) + // Should NOT log a warning for 401 (expected case) + expect(warnSpy).not.toHaveBeenCalled() + expect(resolve).toHaveBeenCalledWith(event) + + warnSpy.mockRestore() }) - it('throws fatal error when SESSION_SECRET is empty string and session cookie exists', async () => { - mockSessionSecret = '' + it('deletes the stale coves_session cookie on 401', async () => { + mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 })) + + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) + const event = createMockEvent({ cookies }) + const resolve = createMockResolve() + + await handle({ event, resolve }) - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', + expect(cookies.delete).toHaveBeenCalledWith('coves_session', { + path: '/', }) + }) + it('sets sessionExpired flag on 401', async () => { + mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 })) + + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() - await expect(handle({ event, resolve })).rejects.toThrow( - '[FATAL] SESSION_SECRET environment variable is not set' - ) + await handle({ event, resolve }) - expect(mockDecryptSession).not.toHaveBeenCalled() + expect(event.locals.sessionExpired).toBe(true) }) - it('allows unauthenticated requests without session cookie when SESSION_SECRET is not set', async () => { - mockSessionSecret = undefined + it('does not set authError on 401 (session expiry is expected)', async () => { + mockFetch.mockResolvedValue(new Response('Unauthorized', { status: 401 })) - const cookies = createMockCookies({}) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) + const event = createMockEvent({ cookies }) + const resolve = createMockResolve() + + await handle({ event, resolve }) + expect(event.locals.authError).toBeUndefined() + }) + }) + + describe('valid cookie and /api/me returns 500', () => { + it('results in unauthenticated state and logs warning', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + mockFetch.mockResolvedValue( + new Response('Internal Server Error', { status: 500 }), + ) + + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() - // Should not throw because no session cookie exists await handle({ event, resolve }) - expect(mockDecryptSession).not.toHaveBeenCalled() expect(event.locals.auth.authenticated).toBe(false) + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('/api/me returned 500'), + ) expect(resolve).toHaveBeenCalledWith(event) + + warnSpy.mockRestore() }) }) - describe('invalid/malformed session cookie', () => { - it('results in unauthenticated request when decryption returns null', async () => { - mockDecryptSession.mockReturnValue(null) + describe('valid cookie and fetch throws network error', () => { + it('sets authError to network_error for connection refused', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const cookies = createMockCookies({ - kelp_session: 'invalid-encrypted-data', - }) + mockFetch.mockRejectedValue( + new Error('Network error: connection refused'), + ) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - expect(mockDecryptSession).toHaveBeenCalledWith('invalid-encrypted-data', TEST_SECRET) expect(event.locals.auth.authenticated).toBe(false) + expect(event.locals.authError).toBe('network_error') + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Network error calling /api/me'), + expect.any(Error), + ) expect(resolve).toHaveBeenCalledWith(event) + + warnSpy.mockRestore() }) - it('clears corrupted session cookie when decryption fails', async () => { - mockDecryptSession.mockReturnValue(null) + it('sets authError to network_error for TypeError (fetch failure)', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const cookies = createMockCookies({ - kelp_session: 'corrupted-session-data', - }) + mockFetch.mockRejectedValue(new TypeError('fetch failed')) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - // Verify the corrupted cookie is cleared by setting it to empty with maxAge: 0 - expect(cookies.set).toHaveBeenCalledWith('kelp_session', '', { - path: '/', - maxAge: 0, - }) + expect(event.locals.auth.authenticated).toBe(false) + expect(event.locals.authError).toBe('network_error') + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Network error calling /api/me'), + expect.any(TypeError), + ) + expect(resolve).toHaveBeenCalledWith(event) + + warnSpy.mockRestore() }) - it('results in unauthenticated request when session cookie is missing', async () => { - const cookies = createMockCookies({}) + it('does not delete the coves_session cookie on network error', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + mockFetch.mockRejectedValue(new TypeError('fetch failed')) + + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - expect(mockDecryptSession).not.toHaveBeenCalled() - expect(event.locals.auth.authenticated).toBe(false) - expect(resolve).toHaveBeenCalledWith(event) + expect(cookies.delete).not.toHaveBeenCalled() }) - it('results in unauthenticated request when session cookie is empty string', async () => { - const cookies = createMockCookies({ - kelp_session: '', - }) + it('sets authError to network_error for unexpected non-network errors', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + mockFetch.mockRejectedValue(new Error('some completely unexpected error')) + + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - // Empty string is falsy, so decryptSession should not be called - expect(mockDecryptSession).not.toHaveBeenCalled() expect(event.locals.auth.authenticated).toBe(false) + expect(event.locals.authError).toBe('network_error') + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Unexpected error calling /api/me'), + expect.any(Error), + ) + expect(resolve).toHaveBeenCalledWith(event) + + warnSpy.mockRestore() }) }) - describe('missing activeAccountId in session', () => { - it('results in unauthenticated request when activeAccountId is null', async () => { - const session: AppSession = { - activeAccountId: null, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('sealed-token-123'), - sessionId: asSessionId('session-1'), - }, - ], - } - - mockDecryptSession.mockReturnValue(session) + describe('valid cookie and /api/me returns invalid JSON', () => { + it('sets authError to validation_error and logs warning', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', - }) + mockFetch.mockResolvedValue( + new Response('not json', { + status: 200, + headers: { 'Content-Type': 'text/plain' }, + }), + ) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - expect(mockDecryptSession).toHaveBeenCalled() expect(event.locals.auth.authenticated).toBe(false) + expect(event.locals.authError).toBe('validation_error') + // Invalid JSON triggers response.json() to throw as SyntaxError, + // which is categorized as a validation error + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('/api/me returned invalid JSON'), + expect.any(SyntaxError), + ) expect(resolve).toHaveBeenCalledWith(event) - }) - it('results in unauthenticated request when activeAccountId references non-existent account', async () => { - const session: AppSession = { - activeAccountId: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaff' as AccountId, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('sealed-token-123'), - sessionId: asSessionId('session-1'), - }, - ], - } + warnSpy.mockRestore() + }) + }) - mockDecryptSession.mockReturnValue(session) + describe('valid cookie and /api/me returns incomplete data', () => { + it('sets authError to validation_error when did is missing', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', - }) + mockFetch.mockResolvedValue( + new Response(JSON.stringify({ handle: 'user1.example.com' }), { + status: 200, + }), + ) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - expect(mockDecryptSession).toHaveBeenCalled() expect(event.locals.auth.authenticated).toBe(false) + expect(event.locals.authError).toBe('validation_error') + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('/api/me response failed validation'), + ) expect(resolve).toHaveBeenCalledWith(event) - }) - it('results in unauthenticated request when accounts array is empty', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [], - } + warnSpy.mockRestore() + }) - mockDecryptSession.mockReturnValue(session) + it('sets authError to validation_error when handle is missing', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', - }) + mockFetch.mockResolvedValue( + new Response(JSON.stringify({ did: 'did:plc:user1' }), { status: 200 }), + ) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() await handle({ event, resolve }) - expect(mockDecryptSession).toHaveBeenCalled() expect(event.locals.auth.authenticated).toBe(false) + expect(event.locals.authError).toBe('validation_error') + expect(resolve).toHaveBeenCalledWith(event) + + warnSpy.mockRestore() }) }) - describe('authToken population', () => { - it('populates authToken with sealedToken from active account', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('my-special-sealed-token'), - sessionId: asSessionId('session-1'), - }, - ], - } + describe('no instance URL configured', () => { + it('throws a fatal error when instance URL is missing', async () => { + mockPublicInternalInstance = undefined + mockPublicInstanceUrl = undefined - mockDecryptSession.mockReturnValue(session) + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) + const event = createMockEvent({ cookies }) + const resolve = createMockResolve() - const cookies = createMockCookies({ - kelp_session: 'encrypted-session-cookie', - }) + await expect(handle({ event, resolve })).rejects.toThrow( + 'No instance URL configured', + ) + expect(mockFetch).not.toHaveBeenCalled() + }) + }) + + describe('authToken equals cookie value', () => { + it('authToken is the coves_session cookie value', async () => { + mockFetch.mockResolvedValue( + new Response( + JSON.stringify({ + did: 'did:plc:user1', + handle: 'user1.example.com', + }), + { status: 200 }, + ), + ) + + const cookieValue = 'my-specific-sealed-token-value' + const cookies = createMockCookies({ coves_session: cookieValue }) const event = createMockEvent({ cookies }) const resolve = createMockResolve() @@ -412,7 +433,7 @@ describe('hooks.server handle', () => { expect(event.locals.auth.authenticated).toBe(true) if (event.locals.auth.authenticated) { - expect(event.locals.auth.authToken).toBe('my-special-sealed-token') + expect(event.locals.auth.authToken).toBe(cookieValue) } }) }) @@ -440,6 +461,37 @@ describe('hooks.server handle', () => { expect(result).toBe(expectedResponse) }) }) + + describe('instance URL fallback', () => { + it('uses PUBLIC_INSTANCE_URL when PUBLIC_INTERNAL_INSTANCE is not set', async () => { + mockPublicInternalInstance = undefined + mockPublicInstanceUrl = 'https://coves.example.com' + + mockFetch.mockResolvedValue( + new Response( + JSON.stringify({ + did: 'did:plc:user1', + handle: 'user1.example.com', + }), + { status: 200 }, + ), + ) + + const cookies = createMockCookies({ coves_session: 'sealed-token-value' }) + const event = createMockEvent({ cookies }) + const resolve = createMockResolve() + + await handle({ event, resolve }) + + expect(mockFetch).toHaveBeenCalledWith( + 'https://coves.example.com/api/me', + { + headers: { Cookie: 'coves_session=sealed-token-value' }, + }, + ) + expect(event.locals.auth.authenticated).toBe(true) + }) + }) }) describe('hooks.server handleError', () => { @@ -460,7 +512,9 @@ describe('hooks.server handleError', () => { it('returns generic error message for non-404 errors', async () => { const result = await handleError({ - error: new Error('Internal database connection failed with password xyz123'), + error: new Error( + 'Internal database connection failed with password xyz123', + ), event: createMockEvent({ cookies: createMockCookies() }), status: 500, message: 'Internal Server Error', @@ -470,7 +524,9 @@ describe('hooks.server handleError', () => { }) it('does not expose internal error details in response', async () => { - const sensitiveError = new Error('Database password: secret123, API key: abc-def-ghi') + const sensitiveError = new Error( + 'Database password: secret123, API key: abc-def-ghi', + ) const result = await handleError({ error: sensitiveError, event: createMockEvent({ cookies: createMockCookies() }), @@ -478,7 +534,6 @@ describe('hooks.server handleError', () => { message: 'Internal Server Error', }) - // The result should not contain any sensitive information const appError = result as App.Error expect(appError.message).not.toContain('secret123') expect(appError.message).not.toContain('abc-def-ghi') diff --git a/src/hooks.server.ts b/src/hooks.server.ts index 12fec230..12307c94 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -1,93 +1,123 @@ import type { Handle, HandleServerError } from '@sveltejs/kit' -import { env } from '$env/dynamic/private' -import { decryptSession } from '$lib/server/session' +import { env } from '$env/dynamic/public' +import { + parseApiMeResponse, + asInstanceURL, + asSealedToken, +} from '$lib/server/session' + +function getInstanceUrl(): string { + return env.PUBLIC_INTERNAL_INSTANCE || env.PUBLIC_INSTANCE_URL || '' +} /** - * Validates that SESSION_SECRET is configured. - * Throws a fatal error at startup if not set, preventing the app from running - * in an insecure state where all users appear logged out. + * Checks whether an error is a network-level failure (DNS, TLS, connection refused, etc.). + * `fetch` throws `TypeError` for network failures in most runtimes, but some runtimes + * wrap the cause in a generic `Error`. This helper inspects the message as a fallback. */ -function requireSessionSecret(): string { - const secret = env.SESSION_SECRET - if (!secret) { - throw new Error( - '[FATAL] SESSION_SECRET environment variable is not set. ' + - 'Authentication cannot function without this. ' + - 'Please set SESSION_SECRET to a 64-character hex string (32 bytes).' +function isNetworkError(error: unknown): boolean { + if (error instanceof TypeError) return true + if (error instanceof Error) { + const msg = error.message.toLowerCase() + return ( + msg.includes('fetch') || + msg.includes('network') || + msg.includes('econnrefused') || + msg.includes('enotfound') || + msg.includes('etimedout') || + msg.includes('tls') || + msg.includes('ssl') || + msg.includes('dns') ) } - return secret + return false } -/** - * Handle hook - runs for every request - * Loads the user session from the encrypted cookie - */ export const handle: Handle = async ({ event, resolve }) => { - // Default to unauthenticated state event.locals.auth = { authenticated: false } - const sessionCookie = event.cookies.get('kelp_session') - - if (!sessionCookie) { - // No session cookie present - user is not logged in (this is normal) + const covesSession = event.cookies.get('coves_session') + if (!covesSession) { return resolve(event) } - // This will throw a fatal error if SESSION_SECRET is not configured, - // preventing the app from silently treating all users as logged out. - const sessionSecret = requireSessionSecret() + const instanceUrl = getInstanceUrl() + if (!instanceUrl) { + throw new Error( + '[hooks] No instance URL configured. Set PUBLIC_INTERNAL_INSTANCE or PUBLIC_INSTANCE_URL.', + ) + } - const session = decryptSession(sessionCookie, sessionSecret) + // Validate configuration eagerly — these throw on invalid input and must + // NOT be caught so that misconfiguration surfaces immediately on the first request. + const instance = asInstanceURL(instanceUrl) + const sealedToken = asSealedToken(covesSession) - if (!session) { - // Session decryption failed - this could be due to: - // - Corrupt cookie data - // - Key rotation (SESSION_SECRET changed) - // - Tampering attempt - // Log at ERROR level and clear the bad cookie to prevent repeated failures - console.error( - '[hooks] Session decryption failed - clearing corrupt cookie and proceeding as unauthenticated' - ) - event.cookies.set('kelp_session', '', { - path: '/', - maxAge: 0, - }) - // Set a flash message cookie to inform the user they were logged out - // This cookie is NOT httpOnly so the client can read and display it - event.cookies.set('kelp_flash', JSON.stringify({ - type: 'session_expired', - message: 'Your session has expired. Please log in again.', - }), { - path: '/', - maxAge: 60, // Short-lived - just needs to survive until the page loads - httpOnly: false, // Client needs to read this to display the message - secure: process.env.NODE_ENV === 'production', - sameSite: 'lax', + // TODO: Consider caching /api/me responses or skipping validation for proxy + // requests to reduce latency. Currently /api/me is called on every request. + try { + const response = await fetch(`${instance}/api/me`, { + headers: { + Cookie: `coves_session=${covesSession}`, + }, }) - return resolve(event) - } - if (!session.activeAccountId) { - console.warn('[hooks] Session has no active account ID - proceeding as unauthenticated') - return resolve(event) - } + if (!response.ok) { + if (response.status === 401) { + // Session expired or revoked — clear the stale cookie so we don't + // make a wasted /api/me round-trip on every subsequent request. + event.cookies.delete('coves_session', { path: '/' }) + // Flag so the layout can show "Your session has expired" to the user + event.locals.sessionExpired = true + } else { + console.warn( + `[hooks] /api/me returned ${response.status} - treating as unauthenticated`, + ) + } + return resolve(event) + } - const activeAccount = session.accounts.find((a) => a.id === session.activeAccountId) + const data: unknown = await response.json() + const account = parseApiMeResponse(data, instance, sealedToken) - if (!activeAccount) { - console.warn( - `[hooks] Active account ID "${session.activeAccountId}" not found in session accounts - proceeding as unauthenticated` - ) - return resolve(event) - } + if (!account) { + console.warn( + '[hooks] /api/me response failed validation - treating as unauthenticated', + ) + event.locals.authError = 'validation_error' + return resolve(event) + } - // Set authenticated state with all required fields - event.locals.auth = { - authenticated: true, - session, - activeAccount, - authToken: activeAccount.sealedToken, + event.locals.auth = { + authenticated: true, + account, + authToken: sealedToken, + } + } catch (error) { + // Distinguish network/infrastructure errors from validation errors. + // Network errors (DNS, TLS, timeouts, connection refused) are likely + // temporary — preserve the cookie so the user can retry. + if (isNetworkError(error)) { + console.warn( + '[hooks] Network error calling /api/me - backend may be unreachable:', + error, + ) + event.locals.authError = 'network_error' + } else if (error instanceof SyntaxError) { + // JSON parse error from response.json() — the server returned + // non-JSON content (e.g. HTML error page, empty body) + console.warn( + '[hooks] /api/me returned invalid JSON - treating as unauthenticated:', + error, + ) + event.locals.authError = 'validation_error' + } else { + console.warn( + '[hooks] Unexpected error calling /api/me - treating as unauthenticated:', + error, + ) + event.locals.authError = 'network_error' + } } return resolve(event) @@ -109,6 +139,5 @@ export const handleError: HandleServerError = async ({ console.error(`Status:`, status) console.error(`Message:`, message) - // Return a generic error message to the client (don't expose internal details) return { message: 'An unexpected error occurred' } } diff --git a/src/lib/app/auth.svelte.ts b/src/lib/app/auth.svelte.ts index 5e19dda7..1e3d5dcb 100644 --- a/src/lib/app/auth.svelte.ts +++ b/src/lib/app/auth.svelte.ts @@ -1,9 +1,17 @@ import { browser } from '$app/environment' import { DEFAULT_INSTANCE_URL } from './instance.svelte' import { moveItem } from './util.svelte' -import type { ClientSession, DID, Handle, InstanceURL } from '$lib/server/session' - -function getFromStorage(key: string, validator?: (data: unknown) => data is T): T | undefined { +import type { + ClientSession, + DID, + Handle, + InstanceURL, +} from '$lib/server/session' + +function getFromStorage( + key: string, + validator?: (data: unknown) => data is T, +): T | undefined { if (!browser) return const lc = localStorage.getItem(key) if (!lc) return undefined @@ -15,7 +23,7 @@ function getFromStorage(key: string, validator?: (data: unknown) => data is T if (validator) { if (!validator(parsed)) { console.warn( - `localStorage key "${key}" contains invalid data structure - clearing corrupted data` + `localStorage key "${key}" contains invalid data structure - clearing corrupted data`, ) localStorage.removeItem(key) return undefined @@ -32,7 +40,10 @@ function getFromStorage(key: string, validator?: (data: unknown) => data is T function setFromStorage(key: string, item: unknown, stringify: boolean = true) { if (!browser) return - return localStorage.setItem(key, stringify ? JSON.stringify(item) : String(item)) + return localStorage.setItem( + key, + stringify ? JSON.stringify(item) : String(item), + ) } // ============================================================================ @@ -127,7 +138,9 @@ export type ProfileInfo = GuestProfile | AuthenticatedProfile /** * Type guard to check if a profile is authenticated. */ -export function isAuthenticated(profile: ProfileInfo): profile is AuthenticatedProfile { +export function isAuthenticated( + profile: ProfileInfo, +): profile is AuthenticatedProfile { return profile.type === 'authenticated' } @@ -228,7 +241,8 @@ class Profile { ) #current = $derived( - this.meta.profiles.find((i) => i.id == this.meta.profile) ?? createGuestProfile(), + this.meta.profiles.find((i) => i.id == this.meta.profile) ?? + createGuestProfile(), ) getDefaultProfile(): ProfileInfo { @@ -252,30 +266,22 @@ class Profile { * @param serverSession - The session data from the server (passed via page data) */ syncFromServer(serverSession: ServerSession | undefined): void { - if (!serverSession) return - - // Convert server accounts to client ProfileInfo format - // All accounts from the server are authenticated (they have DIDs) - const serverProfiles: ProfileInfo[] = serverSession.accounts.map( - (account): AuthenticatedProfile => ({ - type: 'authenticated', - id: account.id, - instance: account.instance, - jwt: 'authenticated', - did: account.did, - handle: account.handle, - avatar: account.avatar, - }) - ) - - // Find current active profile ID (already a string) - const activeId = serverSession.activeAccountId + if (!serverSession || !serverSession.authenticated) return + + // Convert server account to client ProfileInfo format + const serverProfile: AuthenticatedProfile = { + type: 'authenticated', + id: serverSession.account.id, + instance: serverSession.account.instance, + jwt: 'authenticated', + did: serverSession.account.did, + handle: serverSession.account.handle, + avatar: serverSession.account.avatar, + } // Update local state - if (serverProfiles.length > 0) { - this.meta.profiles = serverProfiles - this.meta.profile = activeId ?? serverProfiles[0].id - } + this.meta.profiles = [serverProfile] + this.meta.profile = serverSession.activeAccountId } /** @@ -296,15 +302,16 @@ class Profile { try { response = await fetch('/api/auth/logout', { method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ accountId: id }), credentials: 'include', }) } catch (err) { // Network error - don't clear local state const errorMsg = err instanceof Error ? err.message : 'Network error' console.error('Logout request failed:', err) - return { success: false, error: `Logout failed: ${errorMsg}. Please try again.` } + return { + success: false, + error: `Logout failed: ${errorMsg}. Please try again.`, + } } if (!response.ok) { @@ -319,7 +326,10 @@ class Profile { console.warn('[auth] Failed to parse error response JSON:', err) } console.error('Server logout failed:', errorMsg) - return { success: false, error: `Logout failed: ${errorMsg}. Please try again.` } + return { + success: false, + error: `Logout failed: ${errorMsg}. Please try again.`, + } } // Server logout succeeded - now safe to clear local state @@ -344,60 +354,21 @@ class Profile { } if (id === this.meta.profile) { - this.meta.profile = this.meta.profiles.length > 0 ? this.meta.profiles[0].id : 'guest' + this.meta.profile = + this.meta.profiles.length > 0 ? this.meta.profiles[0].id : 'guest' } return result } - /** - * Switch to a different account. - * Calls the server to update the active session. - * - * @returns Object with success status and optional error message - */ - async switchTo(id: string): Promise<{ success: boolean; error?: string }> { - const targetProfile = this.meta.profiles.find((p) => p.id === id) - if (!targetProfile) { - return { success: false, error: 'Account not found in local profiles' } - } - - try { - const response = await fetch('/api/auth/switch', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ accountId: id }), - credentials: 'include', - }) - - if (!response.ok) { - let errorMsg = `Server returned status ${response.status}` - try { - const errorData = await response.json() - if (errorData.error) { - errorMsg = errorData.error - } - } catch (err) { - console.warn('[auth] Failed to parse switch account response JSON:', err) - } - console.warn('Account switch failed:', errorMsg) - return { success: false, error: errorMsg } - } - - // Update local state - this.meta.profile = id - return { success: true } - } catch (err) { - const errorMsg = err instanceof Error ? err.message : 'Network error' - console.warn('Account switch request failed:', err) - return { success: false, error: `Network error: ${errorMsg}` } - } - } - move(id: string, up: boolean) { try { const index = this.meta.profiles.findIndex((i) => i.id === id) - this.meta.profiles = moveItem(this.meta.profiles, index, index + (up ? -1 : 1)) + this.meta.profiles = moveItem( + this.meta.profiles, + index, + index + (up ? -1 : 1), + ) } catch (err) { console.warn('Failed to move profile:', err) } @@ -405,7 +376,10 @@ class Profile { get isDefaultProfile(): boolean { // A default/guest profile has type 'guest' - return this.#current.type === 'guest' && this.#current.instance == DEFAULT_INSTANCE_URL + return ( + this.#current.type === 'guest' && + this.#current.instance == DEFAULT_INSTANCE_URL + ) } /** @@ -433,7 +407,9 @@ class Profile { // eslint-disable-next-line @typescript-eslint/no-unused-vars isMod(_community?: unknown): boolean { if (!this.#warnedIsMod) { - console.warn('isMod() is a stub - implement when Coves roles API is available') + console.warn( + 'isMod() is a stub - implement when Coves roles API is available', + ) this.#warnedIsMod = true } return false @@ -444,7 +420,9 @@ class Profile { */ get isAdmin(): boolean { if (!this.#warnedIsAdmin) { - console.warn('isAdmin is a stub - implement when Coves roles API is available') + console.warn( + 'isAdmin is a stub - implement when Coves roles API is available', + ) this.#warnedIsAdmin = true } return false diff --git a/src/lib/feature/post/actions/PostActions.svelte b/src/lib/feature/post/actions/PostActions.svelte index b7692a4e..8b1d8df1 100644 --- a/src/lib/feature/post/actions/PostActions.svelte +++ b/src/lib/feature/post/actions/PostActions.svelte @@ -33,7 +33,7 @@ ShieldCheck, } from 'svelte-hero-icons/dist' import { PostVote } from '..' - import { PostFormState } from '../form/postform.svelte' + import { PostFormState } from '../form/post-form.svelte' import { postLink } from '../helpers' let saving = $state(false) diff --git a/src/lib/feature/post/actions/PostActionsMenu.svelte b/src/lib/feature/post/actions/PostActionsMenu.svelte index fc6faf4a..4b76e8de 100644 --- a/src/lib/feature/post/actions/PostActionsMenu.svelte +++ b/src/lib/feature/post/actions/PostActionsMenu.svelte @@ -15,7 +15,7 @@ Trash, XMark, } from 'svelte-hero-icons/dist' - import { type PostFormInit } from '../form/postform.svelte' + import { type PostFormInit } from '../form/post-form.svelte' import { hidePost } from '../helpers' interface Props { diff --git a/src/lib/feature/post/form/PostForm.svelte b/src/lib/feature/post/form/PostForm.svelte index fbc4dba8..9a5a28f7 100644 --- a/src/lib/feature/post/form/PostForm.svelte +++ b/src/lib/feature/post/form/PostForm.svelte @@ -41,7 +41,7 @@ Trash, XMark, } from 'svelte-hero-icons/dist' - import { autofillPost, PostFormState } from './postform.svelte' + import { autofillPost, PostFormState } from './post-form.svelte' interface Props { editPost?: number @@ -58,19 +58,21 @@ // TODO: Re-enable extended community data when Coves API supports it // eslint-disable-next-line @typescript-eslint/no-unused-vars - let _extendedCommunity: Promise<{ - community_view: { - flair_list?: Array<{ - id: number - flair_title: string - background_color: string - text_color: string - community_id: number - blur_images: boolean - ap_id: string - }> - } - } | null> | undefined = $derived.by(() => { + let _extendedCommunity: + | Promise<{ + community_view: { + flair_list?: Array<{ + id: number + flair_title: string + background_color: string + text_color: string + community_id: number + blur_images: boolean + ap_id: string + }> + } + } | null> + | undefined = $derived.by(() => { // Legacy PieFed code - disabled for Coves migration return undefined }) @@ -194,7 +196,7 @@ form .submit(editPost) .then(onsubmit) - .catch((err) => + .catch((err: unknown) => pushError({ message: errorMessage(err as string), scope: 'post-form' }), ) .then(() => (loading = false)) @@ -316,7 +318,10 @@ onclick={() => form.poll?.choices.push({ choice_text: `Option ${form.poll?.choices.length + 1}`, - id: Math.max(...form.poll.choices.map((i) => i.id)) + 1, + id: + Math.max( + ...form.poll.choices.map((i: { id: number }) => i.id), + ) + 1, num_votes: 0, sort_order: 2, })} diff --git a/src/lib/feature/post/form/postform.svelte.ts b/src/lib/feature/post/form/post-form.svelte.ts similarity index 95% rename from src/lib/feature/post/form/postform.svelte.ts rename to src/lib/feature/post/form/post-form.svelte.ts index 8190c7df..41b96677 100644 --- a/src/lib/feature/post/form/postform.svelte.ts +++ b/src/lib/feature/post/form/post-form.svelte.ts @@ -102,9 +102,12 @@ export class PostFormState { }) ).post_view } else { + if (!this.community) { + throw new Error('Community is required when creating a new post') + } res = ( await api.createPost({ - community_id: this.community!.id, + community_id: this.community.id, name: this.title, body: this.body, url: this.url, diff --git a/src/lib/feature/user/ProfileSelection.svelte b/src/lib/feature/user/ProfileSelection.svelte index 966f99ea..9bb4fb1a 100644 --- a/src/lib/feature/user/ProfileSelection.svelte +++ b/src/lib/feature/user/ProfileSelection.svelte @@ -1,11 +1,9 @@ @@ -77,7 +60,6 @@ {#each profiles as p} {@const selected = profile.meta.profile == p.id} switchTo(p.id)} class={[selected && 'bg-slate-100! dark:bg-zinc-800!', 'gap-2!']} > {#snippet prefix()} diff --git a/src/lib/server/cookies.test.ts b/src/lib/server/cookies.test.ts index 71f586c2..eae46f83 100644 --- a/src/lib/server/cookies.test.ts +++ b/src/lib/server/cookies.test.ts @@ -10,30 +10,6 @@ vi.stubGlobal('import', { }) describe('cookies configuration', () => { - describe('SESSION_COOKIE_OPTIONS', () => { - it('should have httpOnly enabled for security', async () => { - // Re-import to get fresh module with mocked env - const { SESSION_COOKIE_OPTIONS } = await import('./cookies') - expect(SESSION_COOKIE_OPTIONS.httpOnly).toBe(true) - }) - - it('should use lax sameSite for OAuth redirect compatibility', async () => { - const { SESSION_COOKIE_OPTIONS } = await import('./cookies') - expect(SESSION_COOKIE_OPTIONS.sameSite).toBe('lax') - }) - - it('should set path to root', async () => { - const { SESSION_COOKIE_OPTIONS } = await import('./cookies') - expect(SESSION_COOKIE_OPTIONS.path).toBe('/') - }) - - it('should have a 30-day maxAge', async () => { - const { SESSION_COOKIE_OPTIONS } = await import('./cookies') - const thirtyDaysInSeconds = 60 * 60 * 24 * 30 - expect(SESSION_COOKIE_OPTIONS.maxAge).toBe(thirtyDaysInSeconds) - }) - }) - describe('PENDING_AUTH_COOKIE_OPTIONS', () => { it('should have httpOnly enabled for security', async () => { const { PENDING_AUTH_COOKIE_OPTIONS } = await import('./cookies') @@ -56,22 +32,4 @@ describe('cookies configuration', () => { expect(PENDING_AUTH_COOKIE_OPTIONS.maxAge).toBe(tenMinutesInSeconds) }) }) - - describe('security considerations', () => { - it('session cookie should have longer TTL than pending auth cookie', async () => { - const { SESSION_COOKIE_OPTIONS, PENDING_AUTH_COOKIE_OPTIONS } = await import('./cookies') - expect(SESSION_COOKIE_OPTIONS.maxAge).toBeGreaterThan(PENDING_AUTH_COOKIE_OPTIONS.maxAge) - }) - - it('both cookies should have httpOnly to prevent XSS access', async () => { - const { SESSION_COOKIE_OPTIONS, PENDING_AUTH_COOKIE_OPTIONS } = await import('./cookies') - expect(SESSION_COOKIE_OPTIONS.httpOnly).toBe(true) - expect(PENDING_AUTH_COOKIE_OPTIONS.httpOnly).toBe(true) - }) - - it('both cookies should have same sameSite policy', async () => { - const { SESSION_COOKIE_OPTIONS, PENDING_AUTH_COOKIE_OPTIONS } = await import('./cookies') - expect(SESSION_COOKIE_OPTIONS.sameSite).toBe(PENDING_AUTH_COOKIE_OPTIONS.sameSite) - }) - }) }) diff --git a/src/lib/server/cookies.ts b/src/lib/server/cookies.ts index b51003c6..a7b213de 100644 --- a/src/lib/server/cookies.ts +++ b/src/lib/server/cookies.ts @@ -5,23 +5,6 @@ * and reduce the risk of configuration drift. */ -/** - * Cookie options for the session cookie (kelp_session). - * - * NOTE: sameSite is set to 'lax' (not 'strict') because the OAuth callback endpoint - * is reached via a cross-site redirect from the Coves OAuth server. With 'strict', - * the browser would not send the pending_auth cookie on the redirect, breaking - * the OAuth flow. 'lax' allows cookies on top-level navigations (like OAuth redirects) - * while still protecting against CSRF on cross-site POST requests. - */ -export const SESSION_COOKIE_OPTIONS = { - httpOnly: true, - secure: import.meta.env.PROD, - sameSite: 'lax' as const, - path: '/', - maxAge: 60 * 60 * 24 * 30, // 30 days -} - /** * Cookie options for pending auth state (kelp_pending_auth). * diff --git a/src/lib/server/session.test.ts b/src/lib/server/session.test.ts index c0c0e291..9f9053fc 100644 --- a/src/lib/server/session.test.ts +++ b/src/lib/server/session.test.ts @@ -1,16 +1,9 @@ -import { describe, it, expect } from 'vitest' +import { describe, it, expect, vi } from 'vitest' import { - createSession, - addAccount, - removeAccount, - switchAccount, - encryptSession, - decryptSession, asDID, asHandle, asInstanceURL, asSealedToken, - asSessionId, isValidDID, isValidHandle, isValidInstanceURL, @@ -19,312 +12,10 @@ import { tryAsInstanceURL, toClientAccount, toClientSession, - type AppSession, + parseApiMeResponse, type AccountSession, - type AccountId, } from './session' -// Valid AccountId for testing (32 hex characters) -const TEST_ACCOUNT_ID = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' as AccountId - -describe('createSession', () => { - it('returns empty session with null activeAccountId', () => { - const session = createSession() - expect(session.activeAccountId).toBeNull() - }) - - it('returns empty accounts array', () => { - const session = createSession() - expect(session.accounts).toEqual([]) - }) -}) - -describe('addAccount', () => { - const mockAccount: Omit = { - did: asDID('did:plc:abc123'), - handle: asHandle('alice.bsky.social'), - instance: asInstanceURL('https://bsky.social'), - sealedToken: asSealedToken('encrypted-token-data'), - sessionId: asSessionId('session-123'), - avatar: 'https://example.com/avatar.png', - } - - it('adds account to empty session', () => { - const session = createSession() - const updated = addAccount(session, mockAccount) - - expect(updated.accounts).toHaveLength(1) - expect(updated.accounts[0].did).toBe('did:plc:abc123') - expect(updated.accounts[0].handle).toBe('alice.bsky.social') - expect(updated.accounts[0].instance).toBe('https://bsky.social') - expect(updated.accounts[0].sealedToken).toBe('encrypted-token-data') - expect(updated.accounts[0].sessionId).toBe('session-123') - expect(updated.accounts[0].avatar).toBe('https://example.com/avatar.png') - }) - - it('sets new account as active', () => { - const session = createSession() - const updated = addAccount(session, mockAccount) - - expect(updated.activeAccountId).toBe(updated.accounts[0].id) - }) - - it('generates unique id for account', () => { - const session = createSession() - const updated = addAccount(session, mockAccount) - - expect(updated.accounts[0].id).toBeDefined() - expect(typeof updated.accounts[0].id).toBe('string') - expect(updated.accounts[0].id.length).toBeGreaterThan(0) - }) - - it('preserves existing accounts when adding', () => { - const session = createSession() - const firstAccount: Omit = { - did: asDID('did:plc:first'), - handle: asHandle('first.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - } - const secondAccount: Omit = { - did: asDID('did:plc:second'), - handle: asHandle('second.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - } - - const withFirst = addAccount(session, firstAccount) - const withBoth = addAccount(withFirst, secondAccount) - - expect(withBoth.accounts).toHaveLength(2) - expect(withBoth.accounts[0].did).toBe('did:plc:first') - expect(withBoth.accounts[1].did).toBe('did:plc:second') - }) - - it('generates different ids for different accounts', () => { - const session = createSession() - const firstAccount: Omit = { - did: asDID('did:plc:first'), - handle: asHandle('first.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - } - const secondAccount: Omit = { - did: asDID('did:plc:second'), - handle: asHandle('second.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - } - - const withFirst = addAccount(session, firstAccount) - const withBoth = addAccount(withFirst, secondAccount) - - expect(withBoth.accounts[0].id).not.toBe(withBoth.accounts[1].id) - }) -}) - -describe('removeAccount', () => { - const createSessionWithAccounts = (): AppSession => { - let session = createSession() - session = addAccount(session, { - did: asDID('did:plc:first'), - handle: asHandle('first.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }) - session = addAccount(session, { - did: asDID('did:plc:second'), - handle: asHandle('second.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }) - return session - } - - it('removes account by id', () => { - const session = createSessionWithAccounts() - const accountToRemove = session.accounts[0] - const updated = removeAccount(session, accountToRemove.id) - - expect(updated.accounts).toHaveLength(1) - expect(updated.accounts[0].did).toBe('did:plc:second') - }) - - it('sets activeAccountId to null if removed account was active', () => { - const session = createSessionWithAccounts() - // The active account is the last added one (second) - const activeId = session.activeAccountId! - const updated = removeAccount(session, activeId) - - expect(updated.activeAccountId).toBeNull() - }) - - it('keeps activeAccountId if different account removed', () => { - const session = createSessionWithAccounts() - const activeId = session.activeAccountId! - // Remove the first account (not active) - const firstAccountId = session.accounts[0].id - const updated = removeAccount(session, firstAccountId) - - expect(updated.activeAccountId).toBe(activeId) - }) - - it('handles removing non-existent account gracefully', () => { - const session = createSessionWithAccounts() - // Use a valid AccountId format that doesn't exist in the session - const nonExistentId = 'ffffffffffffffffffffffffffffffff' as AccountId - const updated = removeAccount(session, nonExistentId) - - expect(updated.accounts).toHaveLength(2) - expect(updated.activeAccountId).toBe(session.activeAccountId) - }) -}) - -describe('switchAccount', () => { - const createSessionWithAccounts = (): AppSession => { - let session = createSession() - session = addAccount(session, { - did: asDID('did:plc:first'), - handle: asHandle('first.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }) - session = addAccount(session, { - did: asDID('did:plc:second'), - handle: asHandle('second.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }) - return session - } - - it('changes activeAccountId to specified account', () => { - const session = createSessionWithAccounts() - const firstAccountId = session.accounts[0].id - const updated = switchAccount(session, firstAccountId) - - expect(updated.activeAccountId).toBe(firstAccountId) - }) - - it('throws error for non-existent account id', () => { - const session = createSessionWithAccounts() - // Use a valid AccountId format that doesn't exist in the session - const nonExistentId = 'ffffffffffffffffffffffffffffffff' as AccountId - - expect(() => switchAccount(session, nonExistentId)).toThrow( - 'Account not found' - ) - }) -}) - -describe('encryptSession / decryptSession', () => { - // 32-byte hex string (64 characters) for AES-256 - const validSecret = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef' - const differentSecret = 'fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210' - - const createTestSession = (): AppSession => { - let session = createSession() - session = addAccount(session, { - did: asDID('did:plc:test'), - handle: asHandle('test.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('test-token'), - sessionId: asSessionId('test-session'), - avatar: 'https://example.com/avatar.png', - }) - return session - } - - it('roundtrips session data correctly', () => { - const session = createTestSession() - const encrypted = encryptSession(session, validSecret) - const decrypted = decryptSession(encrypted, validSecret) - - expect(decrypted).not.toBeNull() - expect(decrypted!.activeAccountId).toBe(session.activeAccountId) - expect(decrypted!.accounts).toHaveLength(1) - expect(decrypted!.accounts[0].did).toBe('did:plc:test') - expect(decrypted!.accounts[0].handle).toBe('test.example.com') - expect(decrypted!.accounts[0].instance).toBe('https://example.com') - expect(decrypted!.accounts[0].sealedToken).toBe('test-token') - expect(decrypted!.accounts[0].sessionId).toBe('test-session') - expect(decrypted!.accounts[0].avatar).toBe('https://example.com/avatar.png') - }) - - it('produces different ciphertext each time (random IV)', () => { - const session = createTestSession() - const encrypted1 = encryptSession(session, validSecret) - const encrypted2 = encryptSession(session, validSecret) - - expect(encrypted1).not.toBe(encrypted2) - }) - - it('decryptSession returns null for invalid data', () => { - const result = decryptSession('invalid-data', validSecret) - expect(result).toBeNull() - }) - - it('decryptSession returns null for tampered data', () => { - const session = createTestSession() - const encrypted = encryptSession(session, validSecret) - // Tamper with the encrypted data - const tampered = encrypted.slice(0, -4) + 'xxxx' - const result = decryptSession(tampered, validSecret) - expect(result).toBeNull() - }) - - it('decryptSession returns null for wrong secret', () => { - const session = createTestSession() - const encrypted = encryptSession(session, validSecret) - const result = decryptSession(encrypted, differentSecret) - expect(result).toBeNull() - }) - - it('handles empty session correctly', () => { - const session = createSession() - const encrypted = encryptSession(session, validSecret) - const decrypted = decryptSession(encrypted, validSecret) - - expect(decrypted).not.toBeNull() - expect(decrypted!.activeAccountId).toBeNull() - expect(decrypted!.accounts).toEqual([]) - }) - - it('handles session with multiple accounts', () => { - let session = createSession() - session = addAccount(session, { - did: asDID('did:plc:first'), - handle: asHandle('first.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }) - session = addAccount(session, { - did: asDID('did:plc:second'), - handle: asHandle('second.example.com'), - instance: asInstanceURL('https://example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }) - - const encrypted = encryptSession(session, validSecret) - const decrypted = decryptSession(encrypted, validSecret) - - expect(decrypted).not.toBeNull() - expect(decrypted!.accounts).toHaveLength(2) - expect(decrypted!.accounts[0].did).toBe('did:plc:first') - expect(decrypted!.accounts[1].did).toBe('did:plc:second') - }) -}) - // ============================================================================ // Branded Types Tests // ============================================================================ @@ -333,7 +24,9 @@ describe('DID validation', () => { it('validates correct DID format', () => { expect(isValidDID('did:plc:abc123')).toBe(true) expect(isValidDID('did:web:example.com')).toBe(true) - expect(isValidDID('did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK')).toBe(true) + expect( + isValidDID('did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK'), + ).toBe(true) }) it('rejects invalid DID format', () => { @@ -408,7 +101,9 @@ describe('InstanceURL validation', () => { }) it('asInstanceURL throws for invalid URL', () => { - expect(() => asInstanceURL('invalid')).toThrow('Invalid Instance URL format') + expect(() => asInstanceURL('invalid')).toThrow( + 'Invalid Instance URL format', + ) }) it('asInstanceURL returns branded type for valid URL', () => { @@ -426,44 +121,274 @@ describe('InstanceURL validation', () => { }) }) -describe('toClientAccount / toClientSession', () => { - it('removes sensitive data from AccountSession', () => { +// ============================================================================ +// parseApiMeResponse Tests +// ============================================================================ + +describe('parseApiMeResponse', () => { + const testInstance = asInstanceURL('https://coves.example.com') + const testToken = asSealedToken('sealed-token-123') + + it('returns AccountSession for valid response', () => { + const data = { + did: 'did:plc:user1', + handle: 'user1.example.com', + } + + const result = parseApiMeResponse(data, testInstance, testToken) + + expect(result).not.toBeNull() + expect(result!.did).toBe('did:plc:user1') + expect(result!.handle).toBe('user1.example.com') + expect(result!.instance).toBe('https://coves.example.com') + expect(result!.sealedToken).toBe('sealed-token-123') + expect(result!.avatar).toBeUndefined() + }) + + it('returns null when did is missing', () => { + const data = { handle: 'user1.example.com' } + const result = parseApiMeResponse(data, testInstance, testToken) + expect(result).toBeNull() + }) + + it('returns null when DID format is invalid', () => { + const data = { did: 'not-a-did', handle: 'user1.example.com' } + const result = parseApiMeResponse(data, testInstance, testToken) + expect(result).toBeNull() + }) + + it('returns null when handle is missing', () => { + const data = { did: 'did:plc:user1' } + const result = parseApiMeResponse(data, testInstance, testToken) + expect(result).toBeNull() + }) + + it('returns null when handle format is invalid', () => { + const data = { did: 'did:plc:user1', handle: 'invalid' } + const result = parseApiMeResponse(data, testInstance, testToken) + expect(result).toBeNull() + }) + + it('includes avatar when present', () => { + const data = { + did: 'did:plc:user1', + handle: 'user1.example.com', + avatar: 'https://example.com/avatar.png', + } + + const result = parseApiMeResponse(data, testInstance, testToken) + + expect(result).not.toBeNull() + expect(result!.avatar).toBe('https://example.com/avatar.png') + }) + + it('sets avatar to undefined when not present', () => { + const data = { + did: 'did:plc:user1', + handle: 'user1.example.com', + } + + const result = parseApiMeResponse(data, testInstance, testToken) + + expect(result).not.toBeNull() + expect(result!.avatar).toBeUndefined() + }) + + it('returns null for null input', () => { + const result = parseApiMeResponse(null, testInstance, testToken) + expect(result).toBeNull() + }) + + it('returns null for non-object input', () => { + expect(parseApiMeResponse('string', testInstance, testToken)).toBeNull() + expect(parseApiMeResponse(42, testInstance, testToken)).toBeNull() + expect(parseApiMeResponse(true, testInstance, testToken)).toBeNull() + expect(parseApiMeResponse(undefined, testInstance, testToken)).toBeNull() + }) + + it('logs warning when did is missing', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + parseApiMeResponse({ handle: 'user1.example.com' }, testInstance, testToken) + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Missing or non-string "did"'), + ) + warnSpy.mockRestore() + }) + + it('logs warning when DID format is invalid', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + parseApiMeResponse( + { did: 'not-a-did', handle: 'user1.example.com' }, + testInstance, + testToken, + ) + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Invalid DID format'), + 'not-a-did', + ) + warnSpy.mockRestore() + }) + + it('logs warning when handle is missing', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + parseApiMeResponse({ did: 'did:plc:user1' }, testInstance, testToken) + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Missing or non-string "handle"'), + ) + warnSpy.mockRestore() + }) + + it('logs warning when handle format is invalid', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + parseApiMeResponse( + { did: 'did:plc:user1', handle: 'invalid' }, + testInstance, + testToken, + ) + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Invalid handle format'), + 'invalid', + ) + warnSpy.mockRestore() + }) + + it('logs warning for non-object input', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + parseApiMeResponse('string', testInstance, testToken) + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Invalid input: expected object'), + 'string', + ) + warnSpy.mockRestore() + }) + + it('rejects javascript: avatar URLs and sets avatar to undefined', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const data = { + did: 'did:plc:user1', + handle: 'user1.example.com', + avatar: 'javascript:alert(1)', + } + + const result = parseApiMeResponse(data, testInstance, testToken) + + expect(result).not.toBeNull() + expect(result!.avatar).toBeUndefined() + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Avatar URL rejected'), + 'javascript:alert(1)', + ) + warnSpy.mockRestore() + }) + + it('rejects data: avatar URLs and sets avatar to undefined', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const data = { + did: 'did:plc:user1', + handle: 'user1.example.com', + avatar: 'data:text/html,', + } + + const result = parseApiMeResponse(data, testInstance, testToken) + + expect(result).not.toBeNull() + expect(result!.avatar).toBeUndefined() + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Avatar URL rejected'), + 'data:text/html,', + ) + warnSpy.mockRestore() + }) + + it('accepts https avatar URLs', () => { + const data = { + did: 'did:plc:user1', + handle: 'user1.example.com', + avatar: 'https://cdn.example.com/avatar.png', + } + + const result = parseApiMeResponse(data, testInstance, testToken) + + expect(result).not.toBeNull() + expect(result!.avatar).toBe('https://cdn.example.com/avatar.png') + }) + + it('accepts http avatar URLs', () => { + const data = { + did: 'did:plc:user1', + handle: 'user1.example.com', + avatar: 'http://localhost:3000/avatar.png', + } + + const result = parseApiMeResponse(data, testInstance, testToken) + + expect(result).not.toBeNull() + expect(result!.avatar).toBe('http://localhost:3000/avatar.png') + }) +}) + +// ============================================================================ +// toClientAccount / toClientSession Tests +// ============================================================================ + +describe('toClientAccount', () => { + it('removes sealedToken and uses DID as id', () => { const account: AccountSession = { - id: TEST_ACCOUNT_ID, did: asDID('did:plc:test'), handle: asHandle('test.example.com'), instance: asInstanceURL('https://example.com'), sealedToken: asSealedToken('secret-token'), - sessionId: asSessionId('session-123'), avatar: 'https://example.com/avatar.png', } const clientAccount = toClientAccount(account) - expect(clientAccount.id).toBe(TEST_ACCOUNT_ID) + expect(clientAccount.id).toBe('did:plc:test') expect(clientAccount.did).toBe('did:plc:test') expect(clientAccount.handle).toBe('test.example.com') expect(clientAccount.instance).toBe('https://example.com') expect(clientAccount.avatar).toBe('https://example.com/avatar.png') expect('sealedToken' in clientAccount).toBe(false) - expect('sessionId' in clientAccount).toBe(false) }) +}) + +describe('toClientSession', () => { + it('returns unauthenticated session for null account', () => { + const clientSession = toClientSession(null) - it('converts full session to client-safe session', () => { - let session = createSession() - session = addAccount(session, { + expect(clientSession.authenticated).toBe(false) + expect(clientSession.activeAccountId).toBeNull() + expect(clientSession.account).toBeNull() + }) + + it('returns authenticated session with account for valid AccountSession', () => { + const account: AccountSession = { did: asDID('did:plc:test'), handle: asHandle('test.example.com'), instance: asInstanceURL('https://example.com'), sealedToken: asSealedToken('secret-token'), - sessionId: asSessionId('session-123'), - }) + } - const clientSession = toClientSession(session) + const clientSession = toClientSession(account) - expect(clientSession.activeAccountId).toBe(session.activeAccountId) - expect(clientSession.accounts).toHaveLength(1) - expect('sealedToken' in clientSession.accounts[0]).toBe(false) - expect('sessionId' in clientSession.accounts[0]).toBe(false) + expect(clientSession.authenticated).toBe(true) + expect(clientSession.activeAccountId).toBe('did:plc:test') + expect(clientSession.account).not.toBeNull() + if (clientSession.authenticated) { + expect(clientSession.account.did).toBe('did:plc:test') + expect('sealedToken' in clientSession.account).toBe(false) + } }) }) diff --git a/src/lib/server/session.ts b/src/lib/server/session.ts index e1e8a41b..e37ee456 100644 --- a/src/lib/server/session.ts +++ b/src/lib/server/session.ts @@ -1,5 +1,3 @@ -import { createCipheriv, createDecipheriv, randomBytes } from 'crypto' - // ============================================================================ // Branded Types for Type-Safe String Identifiers // ============================================================================ @@ -22,13 +20,6 @@ export type Handle = string & { readonly __brand: 'Handle' } */ export type InstanceURL = string & { readonly __brand: 'InstanceURL' } -/** - * Branded type for account IDs within a session. - * These are cryptographically random hex strings used to identify accounts. - * Example: "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4" - */ -export type AccountId = string & { readonly __brand: 'AccountId' } - /** * Branded type for sealed (encrypted) authentication tokens. * These tokens are encrypted by the Coves backend and should be treated as opaque. @@ -36,42 +27,6 @@ export type AccountId = string & { readonly __brand: 'AccountId' } */ export type SealedToken = string & { readonly __brand: 'SealedToken' } -/** - * Branded type for server-side session identifiers. - * These are used to identify sessions on the Coves backend for revocation. - */ -export type SessionId = string & { readonly __brand: 'SessionId' } - -/** - * Type guard to validate AccountId format and narrow type. - * Account IDs are 32-character hexadecimal strings (16 bytes). - * - * @param value - The string to validate - * @returns True if the value matches the AccountId format (also narrows type to AccountId) - */ -export function isValidAccountId(value: string): value is AccountId { - return /^[a-f0-9]{32}$/.test(value) -} - -/** - * Creates a branded AccountId from a string. - * @throws Error if the value is not a valid AccountId format - */ -export function asAccountId(value: string): AccountId { - if (!isValidAccountId(value)) { - throw new Error(`Invalid AccountId format: ${value}`) - } - return value -} - -/** - * Safely attempts to create a branded AccountId from a string. - * @returns The branded AccountId or null if invalid - */ -export function tryAsAccountId(value: string): AccountId | null { - return isValidAccountId(value) ? value : null -} - /** * Creates a branded SealedToken from a string. * Sealed tokens are opaque encrypted strings from the Coves backend, @@ -84,17 +39,6 @@ export function asSealedToken(value: string): SealedToken { return value as SealedToken } -/** - * Creates a branded SessionId from a string. - * Session IDs are opaque identifiers from the Coves backend. - */ -export function asSessionId(value: string): SessionId { - if (!value || value.trim().length === 0) { - throw new Error('Invalid SessionId: cannot be empty') - } - return value as SessionId -} - /** * Type guard to validate DID format and narrow type. * Validates DID format: did:{method}:{identifier} where method is lowercase letters @@ -118,7 +62,7 @@ export function isValidDID(value: string): value is DID { export function isValidHandle(value: string): value is Handle { // Basic validation: at least one dot, alphanumeric with hyphens return /^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)+$/.test( - value + value, ) } @@ -203,20 +147,16 @@ export function tryAsInstanceURL(value: string): InstanceURL | null { * Represents a single authenticated account in the session. */ export interface AccountSession { - /** Unique identifier for this account entry in the session */ - id: AccountId /** The DID (Decentralized Identifier) of the account */ - did: DID + readonly did: DID /** The handle/username of the account */ - handle: Handle + readonly handle: Handle /** The instance/server the account belongs to */ - instance: InstanceURL + readonly instance: InstanceURL /** Sealed access token for API calls (sealed = encrypted by Coves backend) */ - sealedToken: SealedToken - /** Server-side session identifier */ - sessionId: SessionId + readonly sealedToken: SealedToken /** Optional avatar URL */ - avatar?: string + readonly avatar?: string } /** @@ -224,341 +164,146 @@ export interface AccountSession { * This is what gets passed to the client via page data. * Derived from AccountSession to ensure types stay in sync. */ -export type ClientAccount = Omit +export type ClientAccount = Omit & { id: string } /** - * Client-safe session data (excludes sensitive tokens). - * This is what gets passed to the client via page data. + * Unauthenticated client session -- no valid account. */ -export interface ClientSession { - /** The ID of the currently active account, or null if none */ - activeAccountId: AccountId | null - /** All authenticated accounts (without sensitive data) */ - accounts: ClientAccount[] +interface UnauthenticatedClientSession { + readonly authenticated: false + readonly activeAccountId: null + readonly account: null } /** - * Represents the complete application session state. + * Authenticated client session -- valid account present. */ -export interface AppSession { - /** The ID of the currently active account, or null if none */ - activeAccountId: AccountId | null - /** All authenticated accounts in this session */ - accounts: AccountSession[] +interface AuthenticatedClientSession { + readonly authenticated: true + readonly activeAccountId: string + readonly account: ClientAccount } /** - * Converts an AccountSession to a ClientAccount by removing sensitive data. - */ -export function toClientAccount(account: AccountSession): ClientAccount { - return { - id: account.id, - did: account.did, - handle: account.handle, - instance: account.instance, - avatar: account.avatar, - } -} - -/** - * Converts an AppSession to a ClientSession by removing sensitive data. - */ -export function toClientSession(session: AppSession): ClientSession { - return { - activeAccountId: session.activeAccountId, - accounts: session.accounts.map(toClientAccount), - } -} - -/** - * Creates a new empty session with no active account and no accounts. - */ -export function createSession(): AppSession { - return { - activeAccountId: null, - accounts: [], - } -} - -/** - * Generates a cryptographically secure unique ID for an account. - * Returns a branded AccountId type. + * Client-safe session data (excludes sensitive tokens). + * This is what gets passed to the client via page data. + * + * Uses a discriminated union so that `authenticated: true` guarantees + * both `activeAccountId` and `account` are non-null, and vice-versa. */ -function generateAccountId(): AccountId { - // randomBytes(16).toString('hex') produces a 32-char hex string - // which matches the AccountId format - return randomBytes(16).toString('hex') as AccountId -} +export type ClientSession = + | UnauthenticatedClientSession + | AuthenticatedClientSession /** - * Adds a new account to the session. The new account becomes the active account. - * Returns a new session object (immutable pattern). - * - * @param session - The current session state - * @param account - The account data without an ID (ID will be generated) - * @returns A new session with the account added and set as active + * Response from Go backend's /api/me endpoint. + * Returns profile data from the database after validating the session. */ -export function addAccount( - session: AppSession, - account: Omit -): AppSession { - const newAccount: AccountSession = { - ...account, - id: generateAccountId(), - } - - return { - activeAccountId: newAccount.id, - accounts: [...session.accounts, newAccount], - } +interface ApiMeResponse { + did: string + handle: string + avatar?: string } /** - * Removes an account from the session by its ID. - * If the removed account was the active account, activeAccountId is set to null. - * Returns a new session object (immutable pattern). - * - * @param session - The current session state - * @param accountId - The ID of the account to remove (must be a valid AccountId) - * @returns A new session with the account removed + * Converts an AccountSession to a ClientAccount by removing sensitive data. */ -export function removeAccount(session: AppSession, accountId: AccountId): AppSession { - const accountExists = session.accounts.some((acc) => acc.id === accountId) - - if (!accountExists) { - return session - } - - const newAccounts = session.accounts.filter((acc) => acc.id !== accountId) - const wasActive = session.activeAccountId === accountId - +export function toClientAccount(account: AccountSession): ClientAccount { return { - activeAccountId: wasActive ? null : session.activeAccountId, - accounts: newAccounts, + // Use DID as the client-facing ID because the UI components (ProfileSelection, + // accounts page, etc.) identify accounts by an `id` field rather than `did`. + id: account.did, + did: account.did, + handle: account.handle, + instance: account.instance, + avatar: account.avatar, } } /** - * Switches the active account to the specified account ID. - * Throws an error if the account does not exist. - * - * @param session - The current session state - * @param accountId - The ID of the account to switch to - * @returns A new session with the specified account as active - * @throws Error if the account ID does not exist in the session + * Converts an AccountSession (or null) to a ClientSession. */ -export function switchAccount(session: AppSession, accountId: AccountId): AppSession { - const accountExists = session.accounts.some((acc) => acc.id === accountId) - - if (!accountExists) { - throw new Error('Account not found') +export function toClientSession(account: AccountSession | null): ClientSession { + if (!account) { + return { authenticated: false, activeAccountId: null, account: null } } - + const clientAccount = toClientAccount(account) return { - ...session, - activeAccountId: accountId, + authenticated: true, + activeAccountId: clientAccount.id, + account: clientAccount, } } /** - * Updates an existing account in the session by DID. - * If the account exists, updates its data and sets it as active. - * Returns a new session object (immutable pattern). - * - * @param session - The current session state - * @param did - The DID of the account to update - * @param updates - Partial account data to update (excluding id and did) - * @returns Object with updated session and the account ID if found, or null if not found + * Validates that a URL string uses a safe protocol (http: or https:). + * Rejects javascript:, data:, and other potentially dangerous URI schemes. */ -export function updateAccountByDid( - session: AppSession, - did: DID, - updates: Partial> -): { session: AppSession; accountId: AccountId } | null { - const accountIndex = session.accounts.findIndex((acc) => acc.did === did) - - if (accountIndex === -1) { - return null - } - - const existingAccount = session.accounts[accountIndex] - const updatedAccount: AccountSession = { - ...existingAccount, - ...updates, - } - - const newAccounts = [...session.accounts] - newAccounts[accountIndex] = updatedAccount - - return { - session: { - activeAccountId: existingAccount.id, - accounts: newAccounts, - }, - accountId: existingAccount.id, +function isSafeAvatarUrl(url: string): boolean { + try { + const parsed = new URL(url) + return parsed.protocol === 'https:' || parsed.protocol === 'http:' + } catch { + return false } } /** - * Validates that the session secret is a valid 32-byte hex string. - * AES-256 requires exactly 32 bytes (256 bits) as the key. - * - * @param secret - The secret to validate - * @throws Error if the secret is not a valid 64-character hex string - */ -function validateSessionSecret(secret: string): void { - if (typeof secret !== 'string') { - throw new Error('Session secret must be a string') - } - if (secret.length !== 64) { - throw new Error( - `Session secret must be exactly 64 hex characters (32 bytes), got ${secret.length} characters` + * Parses and validates a /api/me response into an AccountSession. + * Combines the API response with the instance URL and sealed token (from cookie). + * Returns null if validation fails. Logs warnings for each specific validation failure + * to aid debugging. + */ +export function parseApiMeResponse( + data: unknown, + instance: InstanceURL, + sealedToken: SealedToken, +): AccountSession | null { + if (typeof data !== 'object' || data === null) { + console.warn( + '[parseApiMeResponse] Invalid input: expected object, got', + typeof data, ) + return null } - if (!/^[a-fA-F0-9]+$/.test(secret)) { - throw new Error('Session secret must contain only hexadecimal characters (0-9, a-f, A-F)') - } -} - -/** - * Encrypts a session using AES-256-GCM. - * Uses a random IV for each encryption to ensure different ciphertext each time. - * - * @param session - The session to encrypt - * @param secret - A 32-byte hex string (64 characters) used as the encryption key - * @returns Base64-encoded encrypted session data (IV + authTag + ciphertext) - * @throws Error if the secret is not a valid 64-character hex string - */ -export function encryptSession(session: AppSession, secret: string): string { - validateSessionSecret(secret) - const key = Buffer.from(secret, 'hex') - const iv = randomBytes(12) // 96-bit IV for GCM - const cipher = createCipheriv('aes-256-gcm', key, iv) + const obj = data as Record - const plaintext = JSON.stringify(session) - const encrypted = Buffer.concat([ - cipher.update(plaintext, 'utf8'), - cipher.final(), - ]) - const authTag = cipher.getAuthTag() - - // Concatenate IV (12 bytes) + authTag (16 bytes) + ciphertext - const combined = Buffer.concat([iv, authTag, encrypted]) - return combined.toString('base64') -} - -/** - * Type guard to validate if a parsed object is a valid AccountSession. - * Note: For deserialization, we validate the format of branded types but - * cast them since the data was previously validated when stored. - */ -function isValidAccountSession(obj: unknown): obj is AccountSession { - if (typeof obj !== 'object' || obj === null) return false - const account = obj as Record - - // Check basic string types - if ( - typeof account.id !== 'string' || - typeof account.did !== 'string' || - typeof account.handle !== 'string' || - typeof account.instance !== 'string' || - typeof account.sealedToken !== 'string' || - typeof account.sessionId !== 'string' - ) { - return false + if (typeof obj.did !== 'string') { + console.warn('[parseApiMeResponse] Missing or non-string "did" field') + return null } - - // Validate avatar is optional string - if (account.avatar !== undefined && typeof account.avatar !== 'string') { - return false + if (!isValidDID(obj.did)) { + console.warn('[parseApiMeResponse] Invalid DID format:', obj.did) + return null } - // Validate branded type formats - if (!isValidAccountId(account.id)) { - return false - } - if (!isValidDID(account.did)) { - return false - } - if (!isValidHandle(account.handle)) { - return false + if (typeof obj.handle !== 'string') { + console.warn('[parseApiMeResponse] Missing or non-string "handle" field') + return null } - if (!isValidInstanceURL(account.instance)) { - return false + if (!isValidHandle(obj.handle)) { + console.warn('[parseApiMeResponse] Invalid handle format:', obj.handle) + return null } - return true -} - -/** - * Type guard to validate if a parsed object is a valid AppSession. - */ -function isValidAppSession(obj: unknown): obj is AppSession { - if (typeof obj !== 'object' || obj === null) return false - const session = obj as Record - - // Validate activeAccountId is null or a valid AccountId - if (session.activeAccountId !== null) { - if (typeof session.activeAccountId !== 'string' || !isValidAccountId(session.activeAccountId)) { - return false + let avatar: string | undefined + if (typeof obj.avatar === 'string') { + if (isSafeAvatarUrl(obj.avatar)) { + avatar = obj.avatar + } else { + console.warn( + '[parseApiMeResponse] Avatar URL rejected (unsafe protocol or invalid URL):', + obj.avatar, + ) + avatar = undefined } } - // Validate accounts array - if (!Array.isArray(session.accounts)) { - return false - } - - return session.accounts.every(isValidAccountSession) -} - -/** - * Decrypts an encrypted session using AES-256-GCM. - * Returns null if decryption fails for any reason (invalid data, wrong key, tampering). - * - * @param encrypted - Base64-encoded encrypted session data - * @param secret - A 32-byte hex string (64 characters) used as the decryption key - * @returns The decrypted session, or null if decryption fails - */ -export function decryptSession(encrypted: string, secret: string): AppSession | null { - try { - const key = Buffer.from(secret, 'hex') - const combined = Buffer.from(encrypted, 'base64') - - // Validate minimum length: IV (12) + authTag (16) + at least some ciphertext - if (combined.length < 28) { - console.error('[session] Decryption failed: encrypted data too short (expected >= 28 bytes)') - return null - } - - const iv = combined.subarray(0, 12) - const authTag = combined.subarray(12, 28) - const ciphertext = combined.subarray(28) - - const decipher = createDecipheriv('aes-256-gcm', key, iv) - decipher.setAuthTag(authTag) - - const decrypted = Buffer.concat([ - decipher.update(ciphertext), - decipher.final(), - ]) - - let parsed: unknown - try { - parsed = JSON.parse(decrypted.toString('utf8')) - } catch (parseError) { - console.error('[session] Decryption failed: invalid JSON in decrypted data', parseError) - return null - } - - if (!isValidAppSession(parsed)) { - console.error('[session] Decryption failed: parsed data does not match AppSession schema') - return null - } - - return parsed - } catch (error) { - console.error('[session] Decryption failed: cryptographic error', error) - return null + return { + did: obj.did as DID, + handle: obj.handle as Handle, + instance, + sealedToken, + avatar, } } diff --git a/src/routes/+layout.server.ts b/src/routes/+layout.server.ts index 88cfa7f1..c1c6a99e 100644 --- a/src/routes/+layout.server.ts +++ b/src/routes/+layout.server.ts @@ -21,11 +21,12 @@ export const load = async ({ request, locals }) => { // Build client-safe session (without sensitive tokens) const session: ClientSession | null = locals.auth.authenticated - ? toClientSession(locals.auth.session) + ? toClientSession(locals.auth.account) : null return { lang: preferredLanguage, session, + sessionExpired: locals.sessionExpired ?? false, } } diff --git a/src/routes/+layout.svelte b/src/routes/+layout.svelte index a283a1c8..27786e46 100644 --- a/src/routes/+layout.svelte +++ b/src/routes/+layout.svelte @@ -2,6 +2,7 @@ import { browser } from '$app/environment' import { navigating, page } from '$app/state' import { site } from '$lib/api/client.svelte' + import { profile } from '$lib/app/auth.svelte' import { locale } from '$lib/app/i18n' import { LINKED_INSTANCE_URL } from '$lib/app/instance.svelte' import { settings } from '$lib/app/settings.svelte' @@ -17,7 +18,13 @@ import { Shell } from '$lib/ui/layout' import Navbar from '$lib/ui/navbar/Navbar.svelte' import Sidebar from '$lib/ui/sidebar/Sidebar.svelte' - import { Button, ModalContainer, Spinner, toast, ToastContainer } from 'mono-svelte' + import { + Button, + ModalContainer, + Spinner, + toast, + ToastContainer, + } from 'mono-svelte' import { t } from '$lib/app/i18n' import nProgress from 'nprogress' import 'nprogress/nprogress.css' @@ -107,6 +114,13 @@ }) } + // Sync server-validated session into client-side profile state. + // hooks.server.ts validates the coves_session cookie and returns the user + // via +layout.server.ts; this effect hydrates the client profile from it. + $effect(() => { + profile.syncFromServer(page.data.session ?? undefined) + }) + let nprogressTimeout = -1 $effect(() => { if (navigating.to) { diff --git a/src/routes/api/auth/auth.test.ts b/src/routes/api/auth/auth.test.ts index f9ac223c..c2f1ea72 100644 --- a/src/routes/api/auth/auth.test.ts +++ b/src/routes/api/auth/auth.test.ts @@ -4,24 +4,11 @@ import type { Redirect } from '@sveltejs/kit' import { POST as loginHandler } from './login/+server' import { GET as callbackHandler } from './callback/+server' import { POST as logoutHandler } from './logout/+server' -import { POST as switchHandler } from './switch/+server' -import { - encryptSession, - decryptSession, - updateAccountByDid, - asDID, - asHandle, - asInstanceURL, - asSealedToken, - asSessionId, - type AppSession, - type AccountId, -} from '$lib/server/session' import { generateOAuthState } from '$lib/server/csrf' // Type alias for any RequestEvent to simplify testing - -type AnyRequestEvent = RequestEvent +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type AnyRequestEvent = RequestEvent, any> /** * Helper to check if an error is a SvelteKit Redirect @@ -37,30 +24,20 @@ function isRedirect(error: unknown): error is Redirect { ) } -// 32-byte hex key (64 characters) for testing -const TEST_SECRET = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2' - -// Valid AccountIds (32 hex characters) for testing -const TEST_ACCOUNT_ID_1 = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1' as AccountId -const TEST_ACCOUNT_ID_2 = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2' as AccountId -const TEST_ACCOUNT_EXISTING = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb01' as AccountId -const TEST_ACCOUNT_TARGET = 'cccccccccccccccccccccccccccccc01' as AccountId -const TEST_ACCOUNT_OTHER = 'dddddddddddddddddddddddddddddd01' as AccountId - -// Mock environment variables +// Mock environment variables (needed by login endpoint) vi.mock('$env/dynamic/private', () => ({ - env: { - SESSION_SECRET: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', - }, + env: {}, })) // Helper to create mock cookies -function createMockCookies(initialCookies: Record = {}): Cookies { +function createMockCookies( + initialCookies: Record = {}, +): Cookies { const store = new Map(Object.entries(initialCookies)) return { get: vi.fn((name: string) => store.get(name)), getAll: vi.fn(() => - Array.from(store.entries()).map(([name, value]) => ({ name, value })) + Array.from(store.entries()).map(([name, value]) => ({ name, value })), ), set: vi.fn((name: string, value: string) => { store.set(name, value) @@ -74,7 +51,6 @@ function createMockCookies(initialCookies: Record = {}): Cookies /** * Creates a mock request event for testing. - * Uses AnyRequestEvent to avoid strict route typing issues in tests. */ function createMockEvent(options: { method?: string @@ -111,34 +87,28 @@ function createMockEvent(options: { } /** - * Helper to create authenticated App.Locals from a session. - * Finds the active account from the session and constructs the proper auth state. + * Helper to create authenticated App.Locals with the new shape. */ -function createAuthenticatedLocals(session: AppSession): App.Locals { - const activeAccount = session.accounts.find((a) => a.id === session.activeAccountId) - if (!activeAccount) { - // Fallback to first account if activeAccountId doesn't match - const fallbackAccount = session.accounts[0] - if (!fallbackAccount) { - return { auth: { authenticated: false } } - } - return { - auth: { - authenticated: true, - session, - activeAccount: fallbackAccount, - authToken: fallbackAccount.sealedToken, - }, - } - } +function createAuthenticatedLocals(account: { + did: string + handle: string + instance: string + sealedToken: string + avatar?: string +}): App.Locals { return { auth: { authenticated: true, - session, - activeAccount, - authToken: activeAccount.sealedToken, + account: { + did: account.did, + handle: account.handle, + instance: account.instance, + sealedToken: account.sealedToken, + avatar: account.avatar, + }, + authToken: account.sealedToken, }, - } + } as App.Locals } // Mock fetch for Coves API calls @@ -195,7 +165,7 @@ describe('POST /api/auth/login', () => { secure: false, sameSite: 'lax', path: '/', - }) + }), ) }) @@ -245,10 +215,9 @@ describe('POST /api/auth/login', () => { await loginHandler(event) - // Verify the redirect was stored in pending auth cookie const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { @@ -272,10 +241,9 @@ describe('POST /api/auth/login', () => { await loginHandler(event) - // Verify the redirect was sanitized to '/' const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { @@ -299,10 +267,9 @@ describe('POST /api/auth/login', () => { await loginHandler(event) - // Verify the redirect was sanitized to '/' const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { @@ -326,10 +293,9 @@ describe('POST /api/auth/login', () => { await loginHandler(event) - // Verify the redirect was sanitized to '/' const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { @@ -353,10 +319,9 @@ describe('POST /api/auth/login', () => { await loginHandler(event) - // Verify the redirect was sanitized to '/' const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { @@ -380,10 +345,9 @@ describe('POST /api/auth/login', () => { await loginHandler(event) - // Verify the redirect was accepted (path extracted) const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { @@ -393,7 +357,7 @@ describe('POST /api/auth/login', () => { }) }) - describe('CSRF state parameter (RFC 9700)', () => { + describe('CSRF state parameter (RFC 6749)', () => { it('generates and stores state in pending auth cookie', async () => { const cookies = createMockCookies() const event = createMockEvent({ @@ -408,17 +372,15 @@ describe('POST /api/auth/login', () => { await loginHandler(event) - // Verify state was stored in pending auth cookie const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { const pendingAuth = JSON.parse(pendingAuthCall[1] as string) expect(pendingAuth.state).toBeDefined() expect(typeof pendingAuth.state).toBe('string') - // State should be 64-character hex string (32 bytes) expect(pendingAuth.state).toMatch(/^[a-f0-9]{64}$/) } }) @@ -438,7 +400,6 @@ describe('POST /api/auth/login', () => { const response = await loginHandler(event) const data = await response.json() - // Verify state is included in OAuth URL expect(data.redirectUrl).toContain('state=') const redirectUrl = new URL(data.redirectUrl) const state = redirectUrl.searchParams.get('state') @@ -461,14 +422,12 @@ describe('POST /api/auth/login', () => { const response = await loginHandler(event) const data = await response.json() - // Get state from OAuth URL const redirectUrl = new URL(data.redirectUrl) const urlState = redirectUrl.searchParams.get('state') - // Get state from pending auth cookie const setCalls = (cookies.set as ReturnType).mock.calls const pendingAuthCall = setCalls.find( - (call) => call[0] === 'kelp_pending_auth' + (call) => call[0] === 'kelp_pending_auth', ) expect(pendingAuthCall).toBeDefined() if (pendingAuthCall) { @@ -502,17 +461,19 @@ describe('POST /api/auth/login', () => { await loginHandler(event1) await loginHandler(event2) - // Get states from both requests const setCalls1 = (cookies1.set as ReturnType).mock.calls const setCalls2 = (cookies2.set as ReturnType).mock.calls const pendingAuth1 = JSON.parse( - setCalls1.find((call) => call[0] === 'kelp_pending_auth')?.[1] as string + setCalls1.find( + (call) => call[0] === 'kelp_pending_auth', + )?.[1] as string, ) const pendingAuth2 = JSON.parse( - setCalls2.find((call) => call[0] === 'kelp_pending_auth')?.[1] as string + setCalls2.find( + (call) => call[0] === 'kelp_pending_auth', + )?.[1] as string, ) - // States should be different expect(pendingAuth1.state).not.toBe(pendingAuth2.state) }) }) @@ -523,31 +484,16 @@ describe('GET /api/auth/callback', () => { vi.clearAllMocks() }) - it('reads coves_session cookie and creates kelp session', async () => { + it('redirects to stored redirect URL on valid state', async () => { const testState = generateOAuthState() const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', + redirect: '/community/test', state: testState, }), + coves_session: 'valid-session-cookie', }) - // Mock the Coves /api/me response - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - handle: 'user.example.com', - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - avatar: 'https://cdn.example.com/avatar.jpg', - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, @@ -561,44 +507,21 @@ describe('GET /api/auth/callback', () => { expect(isRedirect(error)).toBe(true) if (isRedirect(error)) { expect(error.status).toBe(302) - expect(error.location).toBe('/') + expect(error.location).toBe('/community/test') } } - - expect(cookies.set).toHaveBeenCalledWith( - 'kelp_session', - expect.any(String), - expect.objectContaining({ - httpOnly: true, - // secure is false in test environment (import.meta.env.PROD is false) - secure: false, - }) - ) }) - it('redirects to stored redirect URL on success', async () => { + it('redirects to / when no redirect URL stored', async () => { const testState = generateOAuthState() const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/community/test', + redirect: '', state: testState, }), + coves_session: 'valid-session-cookie', }) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - handle: 'user.example.com', - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, @@ -612,77 +535,18 @@ describe('GET /api/auth/callback', () => { expect(isRedirect(error)).toBe(true) if (isRedirect(error)) { expect(error.status).toBe(302) - expect(error.location).toBe('/community/test') - } - } - }) - - it('redirects to /login on missing coves_session', async () => { - const cookies = createMockCookies({ - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - }), - }) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: 'http://localhost:5173/api/auth/callback', - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=no_session') - } - } - }) - - it('redirects to /login with error when pending auth has empty instance', async () => { - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: '', // Empty instance - redirect: '/community/test', - }), - }) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: 'http://localhost:5173/api/auth/callback', - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=no_pending_auth') + expect(error.location).toBe('/') } } }) - it('redirects to /login with error when pending auth is missing instance field', async () => { - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - // Missing instance field entirely - redirect: '/community/test', - }), - }) + it('redirects to /login on missing pending auth cookie', async () => { + const cookies = createMockCookies({}) const event = createMockEvent({ method: 'GET', cookies, - url: 'http://localhost:5173/api/auth/callback', + url: 'http://localhost:5173/api/auth/callback?state=some-state', }) try { @@ -697,46 +561,16 @@ describe('GET /api/auth/callback', () => { } }) - it('handles multi-account (adds to existing session)', async () => { + it('cleans up pending auth cookie after use', async () => { const testState = generateOAuthState() - const existingSession: AppSession = { - activeAccountId: TEST_ACCOUNT_EXISTING, - accounts: [ - { - id: TEST_ACCOUNT_EXISTING, - did: asDID('did:plc:existing'), - handle: asHandle('existing.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('existing-token'), - sessionId: asSessionId('existing-session'), - }, - ], - } - - const encryptedSession = encryptSession(existingSession, TEST_SECRET) - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_session: encryptedSession, kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', state: testState, }), + coves_session: 'valid-session-cookie', }) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:newuser', - handle: 'newuser.example.com', - sessionId: asSessionId('new-session-123'), - sealedToken: asSealedToken('new-sealed-token'), - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, @@ -745,76 +579,29 @@ describe('GET /api/auth/callback', () => { try { await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - } + } catch { + // Expected redirect } - // Verify new session was set with multiple accounts - const setCalls = (cookies.set as ReturnType).mock.calls - const sessionSetCall = setCalls.find( - (call) => call[0] === 'kelp_session' - ) - expect(sessionSetCall).toBeDefined() - - if (sessionSetCall) { - const newEncryptedSession = sessionSetCall[1] as string - const newSession = decryptSession(newEncryptedSession, TEST_SECRET) - expect(newSession?.accounts).toHaveLength(2) - } + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) }) - describe('re-authentication with existing DID', () => { - it('updates existing account instead of creating duplicate when re-authenticating', async () => { + describe('CSRF state validation', () => { + it('rejects callback when state parameter is missing from URL', async () => { const testState = generateOAuthState() - const existingSession: AppSession = { - activeAccountId: TEST_ACCOUNT_EXISTING, - accounts: [ - { - id: TEST_ACCOUNT_EXISTING, - did: asDID('did:plc:sameuser'), - handle: asHandle('oldhandle.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('old-token'), - sessionId: asSessionId('old-session'), - avatar: 'https://cdn.example.com/old-avatar.jpg', - }, - ], - } - - const encryptedSession = encryptSession(existingSession, TEST_SECRET) - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_session: encryptedSession, kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', state: testState, }), }) - // Re-authenticate with same DID but updated info - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:sameuser', // Same DID as existing account - handle: 'newhandle.example.com', // Updated handle - sessionId: asSessionId('new-session-123'), // New session - sealedToken: asSealedToken('new-sealed-token'), // New token - avatar: 'https://cdn.example.com/new-avatar.jpg', // Updated avatar - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, + url: 'http://localhost:5173/api/auth/callback', }) try { @@ -824,288 +611,23 @@ describe('GET /api/auth/callback', () => { expect(isRedirect(error)).toBe(true) if (isRedirect(error)) { expect(error.status).toBe(302) + expect(error.location).toBe('/login?error=invalid_state') } } - - // Verify session was updated (not duplicated) - const setCalls = (cookies.set as ReturnType).mock.calls - const sessionSetCall = setCalls.find( - (call) => call[0] === 'kelp_session' - ) - expect(sessionSetCall).toBeDefined() - - if (sessionSetCall) { - const newEncryptedSession = sessionSetCall[1] as string - const newSession = decryptSession(newEncryptedSession, TEST_SECRET) - - // Should still have only one account (no duplicates) - expect(newSession?.accounts).toHaveLength(1) - - // Account should have same ID (preserved) - expect(newSession?.accounts[0].id).toBe(TEST_ACCOUNT_EXISTING) - - // Account should have same DID - expect(newSession?.accounts[0].did).toBe('did:plc:sameuser') - - // Account should have updated fields - expect(newSession?.accounts[0].handle).toBe('newhandle.example.com') - expect(newSession?.accounts[0].sealedToken).toBe('new-sealed-token') - expect(newSession?.accounts[0].sessionId).toBe('new-session-123') - expect(newSession?.accounts[0].avatar).toBe('https://cdn.example.com/new-avatar.jpg') - - // Should be the active account - expect(newSession?.activeAccountId).toBe(TEST_ACCOUNT_EXISTING) - } - }) - - it('preserves instance when re-authenticating existing account', async () => { - const testState = generateOAuthState() - const existingSession: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:reauth'), - handle: asHandle('user.example.com'), - instance: asInstanceURL('https://original-instance.example.com'), - sealedToken: asSealedToken('old-token'), - sessionId: asSessionId('old-session'), - }, - ], - } - - const encryptedSession = encryptSession(existingSession, TEST_SECRET) - - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_session: encryptedSession, - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', // Different instance in pending auth - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:reauth', - handle: 'user.example.com', - sessionId: asSessionId('new-session'), - sealedToken: asSealedToken('new-token'), - }), - { status: 200 } - ) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - } - - const setCalls = (cookies.set as ReturnType).mock.calls - const sessionSetCall = setCalls.find( - (call) => call[0] === 'kelp_session' - ) - - if (sessionSetCall) { - const newSession = decryptSession(sessionSetCall[1] as string, TEST_SECRET) - - // Instance should be preserved from original account - expect(newSession?.accounts[0].instance).toBe('https://original-instance.example.com') - } - }) - - it('sets re-authenticated account as active even if different account was active', async () => { - const testState = generateOAuthState() - const existingSession: AppSession = { - activeAccountId: TEST_ACCOUNT_OTHER, // Different account is active - accounts: [ - { - id: TEST_ACCOUNT_TARGET, - did: asDID('did:plc:target'), - handle: asHandle('target.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('old-token'), - sessionId: asSessionId('old-session'), - }, - { - id: TEST_ACCOUNT_OTHER, - did: asDID('did:plc:other'), - handle: asHandle('other.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('other-token'), - sessionId: asSessionId('other-session'), - }, - ], - } - - const encryptedSession = encryptSession(existingSession, TEST_SECRET) - - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_session: encryptedSession, - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - // Re-authenticate with the first account (not currently active) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:target', - handle: 'target.example.com', - sessionId: asSessionId('refreshed-session'), - sealedToken: asSealedToken('refreshed-token'), - }), - { status: 200 } - ) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - } - - const setCalls = (cookies.set as ReturnType).mock.calls - const sessionSetCall = setCalls.find( - (call) => call[0] === 'kelp_session' - ) - - if (sessionSetCall) { - const newSession = decryptSession(sessionSetCall[1] as string, TEST_SECRET) - - // Still have both accounts - expect(newSession?.accounts).toHaveLength(2) - - // Re-authenticated account should now be active - expect(newSession?.activeAccountId).toBe(TEST_ACCOUNT_TARGET) - } - }) - }) - - describe('updateAccountByDid helper function', () => { - it('returns null when DID does not exist in session', () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:existing'), - handle: asHandle('existing.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token'), - sessionId: asSessionId('session'), - }, - ], - } - - const result = updateAccountByDid(session, asDID('did:plc:nonexistent'), { - handle: asHandle('new.handle.com'), - }) - - expect(result).toBeNull() - }) - - it('returns updated session and accountId when DID exists', () => { - const session: AppSession = { - activeAccountId: null, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:existing'), - handle: asHandle('old.handle.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('old-token'), - sessionId: asSessionId('old-session'), - }, - ], - } - - const result = updateAccountByDid(session, asDID('did:plc:existing'), { - handle: asHandle('new.handle.com'), - sealedToken: asSealedToken('new-token'), - }) - - expect(result).not.toBeNull() - expect(result?.accountId).toBe(TEST_ACCOUNT_ID_1) - expect(result?.session.accounts[0].handle).toBe('new.handle.com') - expect(result?.session.accounts[0].sealedToken).toBe('new-token') - expect(result?.session.activeAccountId).toBe(TEST_ACCOUNT_ID_1) - }) - - it('does not mutate original session', () => { - const originalSession: AppSession = { - activeAccountId: null, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:existing'), - handle: asHandle('old.handle.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('old-token'), - sessionId: asSessionId('old-session'), - }, - ], - } - - updateAccountByDid(originalSession, asDID('did:plc:existing'), { - handle: asHandle('new.handle.com'), - }) - - // Original session should be unchanged - expect(originalSession.accounts[0].handle).toBe('old.handle.com') - expect(originalSession.activeAccountId).toBeNull() }) - }) - describe('malformed user info response', () => { - it('redirects to /login with error when did is missing', async () => { - const testState = generateOAuthState() + it('rejects callback when state is missing from pending auth cookie', async () => { const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', - state: testState, + // No state field - runtime validation rejects this shape }), }) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - // Missing 'did' - handle: 'user.example.com', - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, + url: 'http://localhost:5173/api/auth/callback?state=somestate123', }) try { @@ -1115,38 +637,25 @@ describe('GET /api/auth/callback', () => { expect(isRedirect(error)).toBe(true) if (isRedirect(error)) { expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_user_info') + expect(error.location).toBe('/login?error=invalid_pending_auth') } } }) - it('redirects to /login with error when handle is missing', async () => { - const testState = generateOAuthState() + it('rejects callback when state values do not match', async () => { + const cookieState = generateOAuthState() + const differentState = generateOAuthState() const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', - state: testState, + state: cookieState, }), }) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - // Missing 'handle' - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, + url: `http://localhost:5173/api/auth/callback?state=${differentState}`, }) try { @@ -1156,38 +665,24 @@ describe('GET /api/auth/callback', () => { expect(isRedirect(error)).toBe(true) if (isRedirect(error)) { expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_user_info') + expect(error.location).toBe('/login?error=invalid_state') } } }) - it('redirects to /login with error when sealedToken is missing', async () => { + it('rejects callback with empty state in URL', async () => { const testState = generateOAuthState() const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', state: testState, }), }) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - handle: 'user.example.com', - sessionId: asSessionId('session-123'), - // Missing 'sealedToken' - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, + url: 'http://localhost:5173/api/auth/callback?state=', }) try { @@ -1197,38 +692,24 @@ describe('GET /api/auth/callback', () => { expect(isRedirect(error)).toBe(true) if (isRedirect(error)) { expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_user_info') + expect(error.location).toBe('/login?error=invalid_state') } } }) - it('redirects to /login with error when sessionId is missing', async () => { - const testState = generateOAuthState() + it('rejects callback with empty state in pending auth cookie', async () => { + const urlState = generateOAuthState() const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', - state: testState, + state: '', }), }) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - handle: 'user.example.com', - // Missing 'sessionId' - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - const event = createMockEvent({ method: 'GET', cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, + url: `http://localhost:5173/api/auth/callback?state=${urlState}`, }) try { @@ -1238,672 +719,62 @@ describe('GET /api/auth/callback', () => { expect(isRedirect(error)).toBe(true) if (isRedirect(error)) { expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_user_info') + expect(error.location).toBe('/login?error=invalid_state') } } }) - - it('handles partial user info with only some required fields', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - // Only did is present, missing handle, sessionId, sealedToken - }), - { status: 200 } - ) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_user_info') - } - } - }) - - it('handles empty object response', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response(JSON.stringify({}), { status: 200 }) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_user_info') - } - } - }) - }) - - describe('invalid credential format handling', () => { - it('redirects to /login with error when DID format is invalid', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - // DID has invalid format (not starting with did:) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'invalid-did-format', // Invalid: should be "did:plc:xxx" - handle: 'user.example.com', - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_credential_format') - } - } - }) - - it('redirects to /login with error when handle format is invalid', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - // Handle has invalid format (no dots, not a domain-like identifier) - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - handle: 'invalid', // Invalid: should be "user.domain.tld" - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_credential_format') - } - } - }) - - it('redirects to /login with error when instance URL format is invalid', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'not-a-valid-url', // Invalid URL format - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - handle: 'user.example.com', - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_credential_format') - } - } - }) - }) - - describe('CSRF state validation (RFC 9700)', () => { - it('rejects callback when state parameter is missing from URL', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: 'http://localhost:5173/api/auth/callback', // No state parameter - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_state') - } - } - }) - - it('rejects callback when state is missing from pending auth cookie', async () => { - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - // No state field - }), - }) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: 'http://localhost:5173/api/auth/callback?state=somestate123', - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_state') - } - } - }) - - it('rejects callback when state values do not match', async () => { - const cookieState = generateOAuthState() - const differentState = generateOAuthState() // Different state - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: cookieState, - }), - }) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${differentState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_state') - } - } - }) - - it('accepts callback when state values match exactly', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response( - JSON.stringify({ - did: 'did:plc:abc123', - handle: 'user.example.com', - sessionId: asSessionId('session-123'), - sealedToken: asSealedToken('sealed-token-xyz'), - }), - { status: 200 } - ) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/') // Success - redirected to stored redirect URL - } - } - }) - - it('rejects callback with empty state in URL', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: 'http://localhost:5173/api/auth/callback?state=', // Empty state - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_state') - } - } - }) - - it('rejects callback with empty state in pending auth cookie', async () => { - const urlState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: '', // Empty state in cookie - }), - }) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${urlState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_state') - } - } - }) - }) - - describe('fetch error handling', () => { - it('redirects to /login with error when fetch throws network error', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockRejectedValueOnce(new Error('Network error: Failed to fetch')) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') - } - } - }) - - it('redirects to /login with error when /api/me returns non-OK status (401)', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response(JSON.stringify({ error: 'Unauthorized' }), { status: 401 }) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') - } - } - }) - - it('redirects to /login with error when /api/me returns non-OK status (500)', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response(JSON.stringify({ error: 'Internal Server Error' }), { status: 500 }) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') - } - } - }) - - it('redirects to /login with error when /api/me returns non-OK status (404)', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockResolvedValueOnce( - new Response(JSON.stringify({ error: 'Not Found' }), { status: 404 }) - ) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') - } - } - }) - - it('redirects to /login with error when fetch times out (AbortError)', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - const abortError = new DOMException('The operation was aborted', 'AbortError') - mockFetch.mockRejectedValueOnce(abortError) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') - } - } - }) - - it('redirects to /login with error when fetch throws TypeError (invalid URL)', async () => { - const testState = generateOAuthState() - const cookies = createMockCookies({ - coves_session: 'mock-coves-session-token', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), - }) - - mockFetch.mockRejectedValueOnce(new TypeError('Failed to parse URL')) - - const event = createMockEvent({ - method: 'GET', - cookies, - url: `http://localhost:5173/api/auth/callback?state=${testState}`, - }) - - try { - await callbackHandler(event) - expect.fail('Expected redirect to be thrown') - } catch (error) { - expect(isRedirect(error)).toBe(true) - if (isRedirect(error)) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') - } - } - }) - }) -}) + }) +}) describe('POST /api/auth/logout', () => { beforeEach(() => { vi.clearAllMocks() }) - it('removes account from session', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - { - id: TEST_ACCOUNT_ID_2, - did: asDID('did:plc:user2'), - handle: asHandle('user2.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }, - ], - } + it('returns 401 if not authenticated', async () => { + const cookies = createMockCookies() - const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), + const event = createMockEvent({ + method: 'POST', + body: {}, + cookies, }) - // Mock Coves logout endpoint - mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) + const response = await logoutHandler(event) + expect(response.status).toBe(401) + }) + + it('returns 403 for cross-origin requests', async () => { + const cookies = createMockCookies({ + coves_session: 'some-session', + }) const event = createMockEvent({ method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_1 }, + body: {}, cookies, - locals: createAuthenticatedLocals(session), + locals: createAuthenticatedLocals({ + did: 'did:plc:user1', + handle: 'user1.example.com', + instance: 'https://coves.example.com', + sealedToken: 'token-1', + }), + url: 'http://localhost:5173/api/auth/logout', + headers: { + Origin: 'https://evil.com', + }, }) const response = await logoutHandler(event) + const data = await response.json() - expect(response.status).toBe(200) - - // Verify session was updated (account removed) - const setCalls = (cookies.set as ReturnType).mock.calls - const sessionSetCall = setCalls.find( - (call) => call[0] === 'kelp_session' - ) - expect(sessionSetCall).toBeDefined() - - if (sessionSetCall) { - const newSession = decryptSession(sessionSetCall[1] as string, TEST_SECRET) - expect(newSession?.accounts).toHaveLength(1) - expect(newSession?.accounts[0].id).toBe(TEST_ACCOUNT_ID_2) - } + expect(response.status).toBe(403) + expect(data.error).toBe('Cross-origin requests not allowed') }) - it('calls Coves /oauth/logout endpoint', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - ], - } - + it('calls Go /oauth/logout endpoint using authToken from locals (not cookie)', async () => { const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), + coves_session: 'my-sealed-token', }) mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) @@ -1912,11 +783,17 @@ describe('POST /api/auth/logout', () => { method: 'POST', body: {}, cookies, - locals: createAuthenticatedLocals(session), + locals: createAuthenticatedLocals({ + did: 'did:plc:user1', + handle: 'user1.example.com', + instance: 'https://coves.example.com', + sealedToken: 'token-1', + }), }) await logoutHandler(event) + // Should use locals.auth.authToken ('token-1'), not the cookie value ('my-sealed-token') expect(mockFetch).toHaveBeenCalledWith( 'https://coves.example.com/oauth/logout', expect.objectContaining({ @@ -1924,425 +801,138 @@ describe('POST /api/auth/logout', () => { headers: expect.objectContaining({ Cookie: 'coves_session=token-1', }), - }) + }), ) }) - it('clears session cookie if no accounts remain', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - ], - } - + it('clears coves_session cookie on logout', async () => { const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), + coves_session: 'my-sealed-token', }) mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) - const event = createMockEvent({ - method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_1 }, - cookies, - locals: createAuthenticatedLocals(session), - }) - - const response = await logoutHandler(event) - - expect(response.status).toBe(200) - expect(cookies.delete).toHaveBeenCalledWith('kelp_session', { path: '/' }) - }) - - it('returns 401 if not authenticated', async () => { - const cookies = createMockCookies() - const event = createMockEvent({ method: 'POST', body: {}, cookies, - // Unauthenticated - use default locals - }) - - const response = await logoutHandler(event) - - expect(response.status).toBe(401) - }) - - it('logs out non-active account while keeping active account unchanged', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, // account-1 is active - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - { - id: TEST_ACCOUNT_ID_2, - did: asDID('did:plc:user2'), - handle: asHandle('user2.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }, - ], - } - - const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), - }) - - // Mock Coves logout endpoint - mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) - - const event = createMockEvent({ - method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_2 }, // Logout account-2, NOT the active account - cookies, - locals: createAuthenticatedLocals(session), + locals: createAuthenticatedLocals({ + did: 'did:plc:user1', + handle: 'user1.example.com', + instance: 'https://coves.example.com', + sealedToken: 'token-1', + }), }) const response = await logoutHandler(event) + const data = await response.json() expect(response.status).toBe(200) - - // Verify session was updated correctly - const setCalls = (cookies.set as ReturnType).mock.calls - const sessionSetCall = setCalls.find( - (call) => call[0] === 'kelp_session' - ) - expect(sessionSetCall).toBeDefined() - - if (sessionSetCall) { - const newSession = decryptSession(sessionSetCall[1] as string, TEST_SECRET) - // account-2 should be removed - expect(newSession?.accounts).toHaveLength(1) - expect(newSession?.accounts[0].id).toBe(TEST_ACCOUNT_ID_1) - // account-1 should STILL be the active account - expect(newSession?.activeAccountId).toBe(TEST_ACCOUNT_ID_1) - } + expect(data.success).toBe(true) + expect(data.session).toBeNull() + expect(cookies.delete).toHaveBeenCalledWith('coves_session', { path: '/' }) }) - it('succeeds locally even when remote logout fails', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - ], - } - + it('succeeds locally even when remote logout fails with 500', async () => { const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), + coves_session: 'my-sealed-token', }) - // Mock Coves logout endpoint to fail with 500 mockFetch.mockResolvedValueOnce( - new Response(JSON.stringify({ error: 'Internal Server Error' }), { status: 500 }) + new Response(JSON.stringify({ error: 'Internal Server Error' }), { + status: 500, + }), ) const event = createMockEvent({ method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_1 }, + body: {}, cookies, - locals: createAuthenticatedLocals(session), + locals: createAuthenticatedLocals({ + did: 'did:plc:user1', + handle: 'user1.example.com', + instance: 'https://coves.example.com', + sealedToken: 'token-1', + }), }) const response = await logoutHandler(event) const data = await response.json() - // Should still succeed locally expect(response.status).toBe(200) expect(data.success).toBe(true) - // Should indicate remote logout failed expect(data.remoteLogoutFailed).toBe(true) - expect(data.remoteLogoutError).toContain('500') - // Session should still be cleared (cookie deleted) - expect(cookies.delete).toHaveBeenCalledWith('kelp_session', { path: '/' }) + // Error details are sanitized -- client gets a generic message + expect(data.remoteLogoutError).toBe('Remote logout failed') + expect(cookies.delete).toHaveBeenCalledWith('coves_session', { path: '/' }) }) it('succeeds locally even when remote logout throws network error', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - ], - } - const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), + coves_session: 'my-sealed-token', }) - // Mock Coves logout endpoint to throw network error - mockFetch.mockRejectedValueOnce(new Error('Network error: connection refused')) + mockFetch.mockRejectedValueOnce( + new Error('Network error: connection refused'), + ) const event = createMockEvent({ method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_1 }, + body: {}, cookies, - locals: createAuthenticatedLocals(session), + locals: createAuthenticatedLocals({ + did: 'did:plc:user1', + handle: 'user1.example.com', + instance: 'https://coves.example.com', + sealedToken: 'token-1', + }), }) const response = await logoutHandler(event) const data = await response.json() - // Should still succeed locally expect(response.status).toBe(200) expect(data.success).toBe(true) - // Should indicate remote logout failed expect(data.remoteLogoutFailed).toBe(true) - expect(data.remoteLogoutError).toContain('Network error') - // Session should still be cleared - expect(cookies.delete).toHaveBeenCalledWith('kelp_session', { path: '/' }) + // Error details are sanitized -- client gets a generic message + expect(data.remoteLogoutError).toBe('Remote logout failed') + expect(cookies.delete).toHaveBeenCalledWith('coves_session', { path: '/' }) }) - it('returns 403 for cross-origin requests', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - ], - } - + it('always calls remote logout using authToken from locals (even without cookie)', async () => { const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), + // No coves_session cookie -- but the authToken comes from locals }) - const event = createMockEvent({ - method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_1 }, - cookies, - locals: createAuthenticatedLocals(session), - url: 'http://localhost:5173/api/auth/logout', - headers: { - Origin: 'https://evil.com', - }, - }) - - const response = await logoutHandler(event) - const data = await response.json() - - expect(response.status).toBe(403) - expect(data.error).toBe('Cross-origin requests not allowed') - }) -}) - -describe('POST /api/auth/switch', () => { - beforeEach(() => { - vi.clearAllMocks() - }) - - it('switches active account', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - { - id: TEST_ACCOUNT_ID_2, - did: asDID('did:plc:user2'), - handle: asHandle('user2.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }, - ], - } - - const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), - }) + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })) const event = createMockEvent({ method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_2 }, + body: {}, cookies, - locals: createAuthenticatedLocals(session), + locals: createAuthenticatedLocals({ + did: 'did:plc:user1', + handle: 'user1.example.com', + instance: 'https://coves.example.com', + sealedToken: 'token-1', + }), }) - const response = await switchHandler(event) + const response = await logoutHandler(event) const data = await response.json() expect(response.status).toBe(200) - expect(data.activeAccountId).toBe(TEST_ACCOUNT_ID_2) - - // Verify cookie was updated - const setCalls = (cookies.set as ReturnType).mock.calls - const sessionSetCall = setCalls.find( - (call) => call[0] === 'kelp_session' + expect(data.success).toBe(true) + // Now uses locals.auth.authToken, so fetch IS called even without cookie + expect(mockFetch).toHaveBeenCalledWith( + 'https://coves.example.com/oauth/logout', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ + Cookie: 'coves_session=token-1', + }), + }), ) - expect(sessionSetCall).toBeDefined() - - if (sessionSetCall) { - const newSession = decryptSession(sessionSetCall[1] as string, TEST_SECRET) - expect(newSession?.activeAccountId).toBe(TEST_ACCOUNT_ID_2) - } - }) - - it('returns 400 for invalid account id format', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - ], - } - - const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), - }) - - const event = createMockEvent({ - method: 'POST', - body: { accountId: 'invalid-format' }, // Not a valid 32-char hex string - cookies, - locals: createAuthenticatedLocals(session), - }) - - const response = await switchHandler(event) - - expect(response.status).toBe(400) - - const data = await response.json() - expect(data.error).toContain('Invalid accountId format') - }) - - it('returns 400 for non-existent account id', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - ], - } - - const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), - }) - - // Use a valid AccountId format that doesn't exist in the session - const event = createMockEvent({ - method: 'POST', - body: { accountId: 'ffffffffffffffffffffffffffffffff' }, - cookies, - locals: createAuthenticatedLocals(session), - }) - - const response = await switchHandler(event) - - expect(response.status).toBe(400) - - const data = await response.json() - expect(data.error).toContain('not found') - }) - - it('returns 401 if not authenticated', async () => { - const cookies = createMockCookies() - - const event = createMockEvent({ - method: 'POST', - body: { accountId: 'some-account' }, - cookies, - // Unauthenticated - use default locals - }) - - const response = await switchHandler(event) - - expect(response.status).toBe(401) - }) - - it('returns 403 for cross-origin requests', async () => { - const session: AppSession = { - activeAccountId: TEST_ACCOUNT_ID_1, - accounts: [ - { - id: TEST_ACCOUNT_ID_1, - did: asDID('did:plc:user1'), - handle: asHandle('user1.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-1'), - sessionId: asSessionId('session-1'), - }, - { - id: TEST_ACCOUNT_ID_2, - did: asDID('did:plc:user2'), - handle: asHandle('user2.example.com'), - instance: asInstanceURL('https://coves.example.com'), - sealedToken: asSealedToken('token-2'), - sessionId: asSessionId('session-2'), - }, - ], - } - - const cookies = createMockCookies({ - kelp_session: encryptSession(session, TEST_SECRET), - }) - - const event = createMockEvent({ - method: 'POST', - body: { accountId: TEST_ACCOUNT_ID_2 }, - cookies, - locals: createAuthenticatedLocals(session), - url: 'http://localhost:5173/api/auth/switch', - headers: { - Origin: 'https://evil.com', - }, - }) - - const response = await switchHandler(event) - const data = await response.json() - - expect(response.status).toBe(403) - expect(data.error).toBe('Cross-origin requests not allowed') + expect(cookies.delete).toHaveBeenCalledWith('coves_session', { path: '/' }) }) }) diff --git a/src/routes/api/auth/callback/+server.ts b/src/routes/api/auth/callback/+server.ts index 97b74474..f6b8b2e7 100644 --- a/src/routes/api/auth/callback/+server.ts +++ b/src/routes/api/auth/callback/+server.ts @@ -1,56 +1,37 @@ import { redirect } from '@sveltejs/kit' import type { RequestHandler } from './$types' -import { env } from '$env/dynamic/private' -import { - createSession, - addAccount, - updateAccountByDid, - encryptSession, - decryptSession, - asDID, - asHandle, - asInstanceURL, - asSealedToken, - asSessionId, - type AppSession, -} from '$lib/server/session' -import { SESSION_COOKIE_OPTIONS } from '$lib/server/cookies' import { validateOAuthState } from '$lib/server/csrf' interface PendingAuth { - instance: string redirect: string state: string } -interface CovesMeResponse { - did: string - handle: string - sessionId: string - sealedToken: string - avatar?: string +/** + * Runtime validation for parsed PendingAuth cookie values. + * Returns the validated PendingAuth if the shape is correct, or null if invalid. + * This guards against cookie values like "null", "42", "[]", or objects + * missing required fields. + */ +function validatePendingAuth(parsed: unknown): PendingAuth | null { + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + return null + } + const obj = parsed as Record + if (typeof obj.state !== 'string' || typeof obj.redirect !== 'string') { + return null + } + return { state: obj.state, redirect: obj.redirect } } /** * GET /api/auth/callback * - * OAuth callback handler. Called by Coves backend after successful authentication. - * - * Flow: - * 1. Read coves_session cookie (set by Coves backend during OAuth) - * 2. Use coves_session to call Coves /api/me to get user info - * 3. Create or update kelp_session with the new account - * 4. Redirect to stored redirect URL + * OAuth callback handler. Called after Coves backend completes authentication. + * The Go backend has already set the coves_session cookie during OAuth. + * This endpoint just validates the CSRF state and redirects. */ export const GET: RequestHandler = async ({ cookies, url }) => { - // Read coves_session cookie set by Coves backend - const covesSession = cookies.get('coves_session') - - if (!covesSession) { - throw redirect(302, '/login?error=no_session') - } - - // Read pending auth state const pendingAuthCookie = cookies.get('kelp_pending_auth') if (!pendingAuthCookie) { @@ -59,130 +40,59 @@ export const GET: RequestHandler = async ({ cookies, url }) => { let pendingAuth: PendingAuth try { - pendingAuth = JSON.parse(pendingAuthCookie) as PendingAuth + const parsed: unknown = JSON.parse(pendingAuthCookie) + const validated = validatePendingAuth(parsed) + if (!validated) { + console.warn( + '[auth/callback] Pending auth cookie has invalid shape:', + typeof parsed, + ) + cookies.delete('kelp_pending_auth', { path: '/' }) + throw redirect(302, '/login?error=invalid_pending_auth') + } + pendingAuth = validated } catch (error) { - console.error('[auth/callback] Failed to parse kelp_pending_auth cookie:', error) - // Clean up corrupted cookie + // Re-throw SvelteKit redirects (they use throw for control flow) + if ( + error && + typeof error === 'object' && + 'status' in error && + 'location' in error + ) { + throw error + } + console.warn('[auth/callback] Failed to parse pending auth cookie', error) cookies.delete('kelp_pending_auth', { path: '/' }) throw redirect(302, '/login?error=no_pending_auth') } // Clean up pending auth cookie - cookies.delete('kelp_pending_auth', { path: '/' }) - - // If no instance in pending auth, we can't proceed - if (!pendingAuth.instance) { - throw redirect(302, '/login?error=no_pending_auth') - } + cookies.delete('kelp_pending_auth', { path: '/' }) // Validate CSRF state parameter (RFC 6749 section 10.12) const callbackState = url.searchParams.get('state') if (!callbackState || !pendingAuth.state) { - console.warn('[auth/callback] Missing state parameter - possible CSRF attack') + console.warn( + '[auth/callback] Missing state parameter - possible CSRF attack', + ) throw redirect(302, '/login?error=invalid_state') } if (!validateOAuthState(pendingAuth.state, callbackState)) { console.warn('[auth/callback] State mismatch - possible CSRF attack', { - expected: pendingAuth.state.substring(0, 8) + '...', - received: callbackState.substring(0, 8) + '...', + expected: `${pendingAuth.state.substring(0, 8)}...`, + received: `${callbackState.substring(0, 8)}...`, }) throw redirect(302, '/login?error=invalid_state') } - // Call Coves /api/me to get user info - let userInfo: CovesMeResponse - try { - const response = await fetch(`${pendingAuth.instance}/api/me`, { - method: 'GET', - headers: { - Cookie: `coves_session=${covesSession}`, - }, - }) - - if (!response.ok) { - throw new Error(`Failed to fetch user info: ${response.status}`) - } - - userInfo = (await response.json()) as CovesMeResponse - } catch (error) { - console.error('Failed to fetch user info from Coves:', error) - throw redirect(302, '/login?error=fetch_failed') - } - - // Validate user info - if (!userInfo.did || !userInfo.handle || !userInfo.sealedToken || !userInfo.sessionId) { - console.error('[auth/callback] Invalid user info from /api/me:', { - instance: pendingAuth.instance, - hasDid: !!userInfo.did, - hasHandle: !!userInfo.handle, - hasSealedToken: !!userInfo.sealedToken, - hasSessionId: !!userInfo.sessionId, - }) - throw redirect(302, '/login?error=invalid_user_info') - } - - // Validate and convert to branded types - let did, handle, instance - try { - did = asDID(userInfo.did) - handle = asHandle(userInfo.handle) - instance = asInstanceURL(pendingAuth.instance) - } catch (error) { - console.error('Invalid credential format:', error) - throw redirect(302, '/login?error=invalid_credential_format') - } - - // Get session secret - const sessionSecret = env.SESSION_SECRET - if (!sessionSecret) { - console.error('SESSION_SECRET environment variable not set') - throw redirect(302, '/login?error=server_config') - } - - // Get existing session or create new one - let session: AppSession - const existingSessionCookie = cookies.get('kelp_session') - - if (existingSessionCookie) { - const existingSession = decryptSession(existingSessionCookie, sessionSecret) - if (existingSession) { - session = existingSession - } else { - session = createSession() - } - } else { - session = createSession() - } - - // Check if this account already exists (by DID) and update or add accordingly - const updateResult = updateAccountByDid(session, did, { - handle, - sealedToken: asSealedToken(userInfo.sealedToken), - sessionId: asSessionId(userInfo.sessionId), - avatar: userInfo.avatar, - }) - - if (updateResult) { - // Existing account was updated - session = updateResult.session - } else { - // Add new account - session = addAccount(session, { - did, - handle, - instance, - sealedToken: asSealedToken(userInfo.sealedToken), - sessionId: asSessionId(userInfo.sessionId), - avatar: userInfo.avatar, - }) + // Verify the Go backend actually set the coves_session cookie during OAuth. + // If it's missing, the user would appear silently logged out with no feedback. + if (!cookies.get('coves_session')) { + throw redirect(302, '/login?error=no_session') } - // Encrypt and store session - const encryptedSession = encryptSession(session, sessionSecret) - cookies.set('kelp_session', encryptedSession, SESSION_COOKIE_OPTIONS) - // Redirect to stored URL or home - const redirectUrl = pendingAuth.redirect || '/' - throw redirect(302, redirectUrl) + // The hook will handle authentication on the next request via /api/me + throw redirect(302, pendingAuth.redirect || '/') } diff --git a/src/routes/api/auth/callback/callback.test.ts b/src/routes/api/auth/callback/callback.test.ts index 10007f63..0b6e8baf 100644 --- a/src/routes/api/auth/callback/callback.test.ts +++ b/src/routes/api/auth/callback/callback.test.ts @@ -1,49 +1,10 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest' - -// Variable to control the mocked SESSION_SECRET -let mockSessionSecret: string | undefined = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2' - -// Mock environment variables -vi.mock('$env/dynamic/private', () => ({ - env: { - get SESSION_SECRET() { - return mockSessionSecret - }, - }, -})) +import { describe, it, expect, vi, type Mock, beforeEach } from 'vitest' -// Mock session functions -const mockCreateSession = vi.fn() -const mockAddAccount = vi.fn() -const mockUpdateAccountByDid = vi.fn() -const mockEncryptSession = vi.fn() -const mockDecryptSession = vi.fn() - -vi.mock('$lib/server/session', () => ({ - createSession: () => mockCreateSession(), - addAccount: (...args: unknown[]) => mockAddAccount(...args), - updateAccountByDid: (...args: unknown[]) => mockUpdateAccountByDid(...args), - encryptSession: (...args: unknown[]) => mockEncryptSession(...args), - decryptSession: (...args: unknown[]) => mockDecryptSession(...args), - asDID: (value: string) => value, - asHandle: (value: string) => value, - asInstanceURL: (value: string) => value, -})) +// Mock CSRF validation - control per test +let mockValidateOAuthState: Mock -// Mock cookies module -vi.mock('$lib/server/cookies', () => ({ - SESSION_COOKIE_OPTIONS: { - httpOnly: true, - secure: false, - sameSite: 'lax' as const, - path: '/', - maxAge: 60 * 60 * 24 * 30, - }, -})) - -// Mock CSRF validation to always return true (we test CSRF separately in auth.test.ts) vi.mock('$lib/server/csrf', () => ({ - validateOAuthState: () => true, + validateOAuthState: (...args: unknown[]) => mockValidateOAuthState(...args), })) // Helper to create mock cookies @@ -60,265 +21,455 @@ function createMockCookies(initialCookies: Record = {}) { } } -// Helper to create mock URL with state parameter -function createMockUrl(state: string = 'test-state-1234567890abcdef1234567890abcdef1234567890abcdef12345678') { - return new URL(`https://kelp.example.com/api/auth/callback?state=${state}`) +// Helper to create mock URL with optional state parameter +function createMockUrl(state?: string): URL { + const base = 'https://kelp.example.com/api/auth/callback' + if (state !== undefined) { + return new URL(`${base}?state=${state}`) + } + return new URL(base) } -describe('auth callback endpoint', () => { +describe('GET /api/auth/callback', () => { beforeEach(() => { vi.clearAllMocks() - mockSessionSecret = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2' - global.fetch = vi.fn() + mockValidateOAuthState = vi.fn(() => true) }) - describe('missing SESSION_SECRET', () => { - const testState = 'test-state-1234567890abcdef1234567890abcdef1234567890abcdef12345678' + describe('missing pending auth cookie', () => { + it('redirects to /login?error=no_pending_auth when kelp_pending_auth is missing', async () => { + const { GET } = await import('./+server') + + const cookies = createMockCookies({}) + + try { + await GET({ + cookies: cookies as never, + url: createMockUrl('some-state'), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=no_pending_auth') + } + }) + }) - it('redirects to login with server_config error when SESSION_SECRET is not set', async () => { - // Import the module after mocks are set up + describe('invalid JSON in pending auth cookie', () => { + it('deletes cookie and redirects to error', async () => { const { GET } = await import('./+server') - mockSessionSecret = undefined + const cookies = createMockCookies({ + kelp_pending_auth: '{invalid-json', + }) + + try { + await GET({ + cookies: cookies as never, + url: createMockUrl('some-state'), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=no_pending_auth') + } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) + }) + }) + + describe('cookie with invalid shape (runtime validation)', () => { + it('redirects to /login?error=invalid_pending_auth when cookie is "null"', async () => { + const { GET } = await import('./+server') const cookies = createMockCookies({ - coves_session: 'valid-coves-session', - kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', - state: testState, - }), + kelp_pending_auth: 'null', }) - // Mock the /api/me response - ;(global.fetch as ReturnType).mockResolvedValue({ - ok: true, - json: () => - Promise.resolve({ - did: 'did:plc:user123', - handle: 'user.example.com', - sessionId: 'session-123', - sealedToken: 'sealed-token-123', - }), + try { + await GET({ + cookies: cookies as never, + url: createMockUrl('some-state'), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_pending_auth') + } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) + }) + + it('redirects to /login?error=invalid_pending_auth when cookie is "42"', async () => { + const { GET } = await import('./+server') + + const cookies = createMockCookies({ + kelp_pending_auth: '42', }) try { await GET({ - cookies: cookies as any, - url: createMockUrl(testState), - } as any) - // Should not reach here - expecting redirect to be thrown + cookies: cookies as never, + url: createMockUrl('some-state'), + } as never) expect.fail('Expected redirect to be thrown') - } catch (error: any) { - // SvelteKit redirect throws an object with status and location - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=server_config') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_pending_auth') } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) + }) + + it('redirects to /login?error=invalid_pending_auth when cookie is "[]"', async () => { + const { GET } = await import('./+server') + + const cookies = createMockCookies({ + kelp_pending_auth: '[]', + }) + + try { + await GET({ + cookies: cookies as never, + url: createMockUrl('some-state'), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_pending_auth') + } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) }) - it('redirects to login with server_config error when SESSION_SECRET is empty', async () => { + it('redirects to /login?error=invalid_pending_auth when cookie is "{}" (empty object)', async () => { const { GET } = await import('./+server') - mockSessionSecret = '' + const cookies = createMockCookies({ + kelp_pending_auth: '{}', + }) + + try { + await GET({ + cookies: cookies as never, + url: createMockUrl('some-state'), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_pending_auth') + } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) + }) + + it('redirects to /login?error=invalid_pending_auth when state is a number instead of string', async () => { + const { GET } = await import('./+server') const cookies = createMockCookies({ - coves_session: 'valid-coves-session', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', - state: testState, + state: 12345, }), }) - ;(global.fetch as ReturnType).mockResolvedValue({ - ok: true, - json: () => - Promise.resolve({ - did: 'did:plc:user123', - handle: 'user.example.com', - sessionId: 'session-123', - sealedToken: 'sealed-token-123', - }), - }) - try { await GET({ - cookies: cookies as any, - url: createMockUrl(testState), - } as any) + cookies: cookies as never, + url: createMockUrl('some-state'), + } as never) expect.fail('Expected redirect to be thrown') - } catch (error: any) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=server_config') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_pending_auth') } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) }) }) - describe('missing coves_session cookie', () => { - it('redirects to login with no_session error when coves_session is missing', async () => { + describe('missing state parameter', () => { + it('redirects to /login?error=invalid_state when state param is missing from URL', async () => { const { GET } = await import('./+server') const cookies = createMockCookies({ - // No coves_session cookie kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', + state: 'stored-state-value', }), }) try { await GET({ - cookies: cookies as any, - } as any) + cookies: cookies as never, + url: createMockUrl(), // No state parameter + } as never) expect.fail('Expected redirect to be thrown') - } catch (error: any) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=no_session') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_state') } }) - }) - describe('missing pending auth state', () => { - it('redirects to login with no_pending_auth error when pending auth cookie is missing or invalid', async () => { + it('redirects to /login?error=invalid_pending_auth when state is missing from pending auth', async () => { const { GET } = await import('./+server') const cookies = createMockCookies({ - coves_session: 'valid-coves-session', - // No kelp_pending_auth cookie + kelp_pending_auth: JSON.stringify({ + redirect: '/', + // No state field - runtime validation rejects this shape + }), }) try { await GET({ - cookies: cookies as any, - } as any) + cookies: cookies as never, + url: createMockUrl('url-state-value'), + } as never) expect.fail('Expected redirect to be thrown') - } catch (error: any) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=no_pending_auth') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_pending_auth') } }) - it('redirects to login with no_pending_auth error when pending auth has no instance', async () => { + it('redirects to /login?error=invalid_state when state is empty in pending auth', async () => { const { GET } = await import('./+server') const cookies = createMockCookies({ - coves_session: 'valid-coves-session', kelp_pending_auth: JSON.stringify({ - instance: '', redirect: '/', + state: '', }), }) try { await GET({ - cookies: cookies as any, - } as any) + cookies: cookies as never, + url: createMockUrl('url-state-value'), + } as never) expect.fail('Expected redirect to be thrown') - } catch (error: any) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=no_pending_auth') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_state') } }) }) - describe('fetch user info failure', () => { - const testState = 'test-state-1234567890abcdef1234567890abcdef1234567890abcdef12345678' - - it('redirects to login with fetch_failed error when /api/me request fails', async () => { + describe('state mismatch', () => { + it('redirects to /login?error=invalid_state when states do not match', async () => { const { GET } = await import('./+server') + mockValidateOAuthState.mockReturnValue(false) + const cookies = createMockCookies({ - coves_session: 'valid-coves-session', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', + state: 'stored-state-aaaa', + }), + }) + + try { + await GET({ + cookies: cookies as never, + url: createMockUrl('different-state-bbbb'), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=invalid_state') + } + + expect(mockValidateOAuthState).toHaveBeenCalledWith( + 'stored-state-aaaa', + 'different-state-bbbb', + ) + }) + }) + + describe('valid state', () => { + it('redirects to stored redirect URL on success', async () => { + const { GET } = await import('./+server') + + const testState = 'abc123def456' + + const cookies = createMockCookies({ + kelp_pending_auth: JSON.stringify({ + redirect: '/community/test', state: testState, }), + coves_session: 'valid-session-cookie', }) - ;(global.fetch as ReturnType).mockResolvedValue({ - ok: false, - status: 500, + try { + await GET({ + cookies: cookies as never, + url: createMockUrl(testState), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/community/test') + } + }) + + it('redirects to / when no redirect in pending auth', async () => { + const { GET } = await import('./+server') + + const testState = 'abc123def456' + + const cookies = createMockCookies({ + kelp_pending_auth: JSON.stringify({ + redirect: '', + state: testState, + }), + coves_session: 'valid-session-cookie', }) try { await GET({ - cookies: cookies as any, + cookies: cookies as never, url: createMockUrl(testState), - } as any) + } as never) expect.fail('Expected redirect to be thrown') - } catch (error: any) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/') } }) + }) - it('redirects to login with fetch_failed error when network error occurs', async () => { + describe('missing coves_session cookie after OAuth', () => { + it('redirects to /login?error=no_session when coves_session is not set', async () => { const { GET } = await import('./+server') + const testState = 'abc123def456' + + // kelp_pending_auth exists but coves_session does NOT const cookies = createMockCookies({ - coves_session: 'valid-coves-session', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', - redirect: '/', + redirect: '/community/test', state: testState, }), }) - ;(global.fetch as ReturnType).mockRejectedValue( - new Error('Network error') - ) + try { + await GET({ + cookies: cookies as never, + url: createMockUrl(testState), + } as never) + expect.fail('Expected redirect to be thrown') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/login?error=no_session') + } + }) + + it('redirects to stored URL when coves_session exists', async () => { + const { GET } = await import('./+server') + + const testState = 'abc123def456' + + const cookies = createMockCookies({ + kelp_pending_auth: JSON.stringify({ + redirect: '/community/test', + state: testState, + }), + coves_session: 'valid-session-cookie', + }) try { await GET({ - cookies: cookies as any, + cookies: cookies as never, url: createMockUrl(testState), - } as any) + } as never) expect.fail('Expected redirect to be thrown') - } catch (error: any) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=fetch_failed') + } catch (error: unknown) { + const redirect = error as { status: number; location: string } + expect(redirect.status).toBe(302) + expect(redirect.location).toBe('/community/test') } }) }) - describe('invalid user info', () => { - const testState = 'test-state-1234567890abcdef1234567890abcdef1234567890abcdef12345678' - - it('redirects to login with invalid_user_info error when response is missing required fields', async () => { + describe('pending auth cookie cleanup', () => { + it('deletes kelp_pending_auth cookie after successful use', async () => { const { GET } = await import('./+server') + const testState = 'abc123def456' + const cookies = createMockCookies({ - coves_session: 'valid-coves-session', kelp_pending_auth: JSON.stringify({ - instance: 'https://coves.example.com', redirect: '/', state: testState, }), + coves_session: 'valid-session-cookie', }) - ;(global.fetch as ReturnType).mockResolvedValue({ - ok: true, - json: () => - Promise.resolve({ - // Missing required fields - did: 'did:plc:user123', - // handle: missing - // sessionId: missing - // sealedToken: missing - }), + try { + await GET({ + cookies: cookies as never, + url: createMockUrl(testState), + } as never) + } catch { + // Expected redirect + } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) + }) + + it('deletes kelp_pending_auth cookie even on state validation failure', async () => { + const { GET } = await import('./+server') + + mockValidateOAuthState.mockReturnValue(false) + + const cookies = createMockCookies({ + kelp_pending_auth: JSON.stringify({ + redirect: '/', + state: 'stored-state', + }), }) try { await GET({ - cookies: cookies as any, - url: createMockUrl(testState), - } as any) - expect.fail('Expected redirect to be thrown') - } catch (error: any) { - expect(error.status).toBe(302) - expect(error.location).toBe('/login?error=invalid_user_info') + cookies: cookies as never, + url: createMockUrl('different-state'), + } as never) + } catch { + // Expected redirect } + + expect(cookies.delete).toHaveBeenCalledWith('kelp_pending_auth', { + path: '/', + }) }) }) }) diff --git a/src/routes/api/auth/login/+server.ts b/src/routes/api/auth/login/+server.ts index aef485be..88a462a8 100644 --- a/src/routes/api/auth/login/+server.ts +++ b/src/routes/api/auth/login/+server.ts @@ -40,7 +40,10 @@ export const POST: RequestHandler = async ({ request, cookies, url }) => { // Normalize and validate instance URL // If the instance doesn't have a protocol, prepend https:// let normalizedInstance = instance.trim() - if (!normalizedInstance.startsWith('http://') && !normalizedInstance.startsWith('https://')) { + if ( + !normalizedInstance.startsWith('http://') && + !normalizedInstance.startsWith('https://') + ) { normalizedInstance = `https://${normalizedInstance}` } @@ -68,21 +71,34 @@ export const POST: RequestHandler = async ({ request, cookies, url }) => { safeRedirect = trimmedRedirect } else if (trimmedRedirect.startsWith('\\')) { // Reject backslash-prefixed URLs (potential bypass attempt) - console.warn('[auth/login] Rejected redirect URL with backslash prefix:', trimmedRedirect) + console.warn( + '[auth/login] Rejected redirect URL with backslash prefix:', + trimmedRedirect, + ) } else if (trimmedRedirect.startsWith('//')) { // Reject protocol-relative URLs - console.warn('[auth/login] Rejected protocol-relative redirect URL:', trimmedRedirect) + console.warn( + '[auth/login] Rejected protocol-relative redirect URL:', + trimmedRedirect, + ) } else { // Try to parse as URL and check if same-origin try { const redirectUrl = new URL(trimmedRedirect, url.origin) if (redirectUrl.origin === url.origin) { - safeRedirect = redirectUrl.pathname + redirectUrl.search + redirectUrl.hash + safeRedirect = + redirectUrl.pathname + redirectUrl.search + redirectUrl.hash } else { - console.warn('[auth/login] Rejected external redirect URL:', trimmedRedirect) + console.warn( + '[auth/login] Rejected external redirect URL:', + trimmedRedirect, + ) } } catch { - console.warn('[auth/login] Rejected invalid redirect URL:', trimmedRedirect) + console.warn( + '[auth/login] Rejected invalid redirect URL:', + trimmedRedirect, + ) } } } @@ -92,12 +108,15 @@ export const POST: RequestHandler = async ({ request, cookies, url }) => { // Store pending auth state in cookie const pendingAuth = { - instance: instanceUrl.origin, redirect: safeRedirect, state, } - cookies.set('kelp_pending_auth', JSON.stringify(pendingAuth), PENDING_AUTH_COOKIE_OPTIONS) + cookies.set( + 'kelp_pending_auth', + JSON.stringify(pendingAuth), + PENDING_AUTH_COOKIE_OPTIONS, + ) // Build OAuth redirect URL // Coves OAuth endpoint: {instance}/oauth/login?handle={handle}&redirect_uri={callback}&state={state} diff --git a/src/routes/api/auth/logout/+server.ts b/src/routes/api/auth/logout/+server.ts index 2b432fd3..4fae5610 100644 --- a/src/routes/api/auth/logout/+server.ts +++ b/src/routes/api/auth/logout/+server.ts @@ -1,139 +1,73 @@ import { json } from '@sveltejs/kit' import type { RequestHandler } from './$types' -import { env } from '$env/dynamic/private' -import { removeAccount, encryptSession, isValidAccountId, asAccountId } from '$lib/server/session' -import { SESSION_COOKIE_OPTIONS } from '$lib/server/cookies' import { validateRequestOrigin } from '$lib/server/csrf' -interface LogoutRequest { - accountId?: string -} - /** * POST /api/auth/logout * - * Logs out an account from the session. - * - * Flow: - * 1. Parse accountId from body (defaults to active account) - * 2. Call Coves /oauth/logout endpoint (best effort) - * 3. Remove account from session - * 4. Update or clear kelp_session cookie + * Logs out the current user. + * Forwards the coves_session cookie to Go's /oauth/logout for server-side revocation, + * then clears the cookie locally. */ -export const POST: RequestHandler = async ({ request, cookies, locals, url }) => { +export const POST: RequestHandler = async ({ + request, + cookies, + locals, + url, +}) => { // Validate Origin header (defense-in-depth against CSRF) const originResult = validateRequestOrigin(request, url.origin) if (!originResult.valid) { - console.warn('[auth/logout] Cross-origin request blocked:', originResult.reason) + console.warn( + '[auth/logout] Cross-origin request blocked:', + originResult.reason, + ) return json({ error: 'Cross-origin requests not allowed' }, { status: 403 }) } - // Check authentication if (!locals.auth.authenticated) { return json({ error: 'Not authenticated' }, { status: 401 }) } - const { session } = locals.auth - - // Parse request body - let body: LogoutRequest = {} - try { - const text = await request.text() - if (text.trim()) { - body = JSON.parse(text) - } - // Empty body is valid - will logout active account - } catch (error) { - console.warn('Failed to parse logout request body as JSON, defaulting to active account logout:', error) - } - - // Determine which account to logout - const rawAccountId = body.accountId ?? session.activeAccountId - if (!rawAccountId) { - return json({ error: 'No account to logout' }, { status: 400 }) - } - - // Validate accountId format (must be valid AccountId) - if (!isValidAccountId(rawAccountId)) { - return json({ error: 'Invalid accountId format' }, { status: 400 }) - } - const accountId = asAccountId(rawAccountId) - - // Find the account to logout - const account = session.accounts.find((a) => a.id === accountId) - if (!account) { - return json({ error: 'Account not found' }, { status: 400 }) - } - - // Call Coves /oauth/logout endpoint to revoke the session on the backend. - // The Coves backend expects the sealed token in a `coves_session` cookie, - // which it unseals to extract the DID and session ID for revocation. - // See: Coves/internal/atproto/oauth/handlers.go HandleLogout() - // - // Track whether remote revocation succeeded for user notification + // Call Go /oauth/logout to revoke the session on the backend (best effort). + // Use locals.auth.authToken (already validated by hooks) instead of re-reading + // the cookie, which avoids a race condition where another request could delete + // the cookie mid-flight. let remoteLogoutFailed = false - let remoteLogoutError: string | undefined + + const authToken = locals.auth.authToken try { - const logoutResponse = await fetch(`${account.instance}/oauth/logout`, { - method: 'POST', - headers: { - Cookie: `coves_session=${account.sealedToken}`, + const logoutResponse = await fetch( + `${locals.auth.account.instance}/oauth/logout`, + { + method: 'POST', + headers: { + Cookie: `coves_session=${authToken}`, + }, }, - }) + ) if (!logoutResponse.ok) { remoteLogoutFailed = true - remoteLogoutError = `Backend returned status ${logoutResponse.status}` - console.warn('Coves logout endpoint returned non-OK status:', logoutResponse.status) + // Log full details server-side for debugging, but don't expose to client + console.warn( + '[auth/logout] Backend returned non-OK status:', + logoutResponse.status, + ) } } catch (error) { - // Log but don't fail - we still want to clear the local session remoteLogoutFailed = true - remoteLogoutError = error instanceof Error ? error.message : 'Network error' - console.warn('Failed to call Coves logout endpoint:', error) + // Log the full error server-side for debugging + console.warn('[auth/logout] Failed to call backend logout endpoint:', error) } - // Remove account from session - const updatedSession = removeAccount(session, accountId) + // Clear the coves_session cookie + cookies.delete('coves_session', { path: '/' }) - // Get session secret - const sessionSecret = env.SESSION_SECRET - if (!sessionSecret) { - console.error('SESSION_SECRET environment variable not set') - return json({ error: 'Server configuration error' }, { status: 500 }) - } - - // Update or clear session cookie - if (updatedSession.accounts.length === 0) { - // No accounts left - clear the session - cookies.delete('kelp_session', { path: '/' }) - return json({ - success: true, - session: null, - remoteLogoutFailed, - remoteLogoutError, - }) - } else { - // Update session with remaining accounts - // If active account was removed, set it to first remaining account - if (updatedSession.activeAccountId === null) { - updatedSession.activeAccountId = updatedSession.accounts[0].id - } - - const encryptedSession = encryptSession(updatedSession, sessionSecret) - cookies.set('kelp_session', encryptedSession, SESSION_COOKIE_OPTIONS) - - return json({ - success: true, - activeAccountId: updatedSession.activeAccountId, - accounts: updatedSession.accounts.map((a) => ({ - id: a.id, - did: a.did, - handle: a.handle, - instance: a.instance, - avatar: a.avatar, - })), - remoteLogoutFailed, - remoteLogoutError, - }) - } + return json({ + success: true, + session: null, + remoteLogoutFailed, + // Return a generic message instead of leaking internal error details + ...(remoteLogoutFailed && { remoteLogoutError: 'Remote logout failed' }), + }) } diff --git a/src/routes/api/auth/switch/+server.ts b/src/routes/api/auth/switch/+server.ts deleted file mode 100644 index b7f39a0f..00000000 --- a/src/routes/api/auth/switch/+server.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { json } from '@sveltejs/kit' -import type { RequestHandler } from './$types' -import { env } from '$env/dynamic/private' -import { switchAccount, encryptSession, isValidAccountId } from '$lib/server/session' -import { SESSION_COOKIE_OPTIONS } from '$lib/server/cookies' -import { validateRequestOrigin } from '$lib/server/csrf' - -interface SwitchRequest { - accountId: string -} - -/** - * POST /api/auth/switch - * - * Switches the active account in the session. - * - * Flow: - * 1. Parse accountId from body - * 2. Validate account exists in session - * 3. Update activeAccountId - * 4. Set updated kelp_session cookie - */ -export const POST: RequestHandler = async ({ request, cookies, locals, url }) => { - // Validate Origin header (defense-in-depth against CSRF) - const originResult = validateRequestOrigin(request, url.origin) - if (!originResult.valid) { - console.warn('[auth/switch] Cross-origin request blocked:', originResult.reason) - return json({ error: 'Cross-origin requests not allowed' }, { status: 403 }) - } - - // Check authentication - if (!locals.auth.authenticated) { - return json({ error: 'Not authenticated' }, { status: 401 }) - } - - const { session } = locals.auth - - // Parse request body - let body: Partial - try { - body = await request.json() - } catch { - return json({ error: 'Invalid JSON body' }, { status: 400 }) - } - - const { accountId } = body - - if (!accountId || typeof accountId !== 'string') { - return json({ error: 'Missing or invalid accountId' }, { status: 400 }) - } - - // Validate accountId format (must be valid AccountId) - if (!isValidAccountId(accountId)) { - return json({ error: 'Invalid accountId format' }, { status: 400 }) - } - - // Try to switch account - let updatedSession - try { - updatedSession = switchAccount(session, accountId) - } catch (err) { - // Log the actual error for debugging - console.error('[auth/switch] Failed to switch account:', err) - - // Return specific error message based on the error type - const errorMessage = err instanceof Error ? err.message : 'Unknown error during account switch' - return json({ error: errorMessage }, { status: 400 }) - } - - // Get session secret - const sessionSecret = env.SESSION_SECRET - if (!sessionSecret) { - console.error('SESSION_SECRET environment variable not set') - return json({ error: 'Server configuration error' }, { status: 500 }) - } - - // Update session cookie - const encryptedSession = encryptSession(updatedSession, sessionSecret) - cookies.set('kelp_session', encryptedSession, SESSION_COOKIE_OPTIONS) - - return json({ - activeAccountId: updatedSession.activeAccountId, - accounts: updatedSession.accounts.map((a) => ({ - id: a.id, - did: a.did, - handle: a.handle, - instance: a.instance, - avatar: a.avatar, - })), - }) -} diff --git a/src/routes/api/proxy/[...path]/+server.ts b/src/routes/api/proxy/[...path]/+server.ts index 0b780851..b3626980 100644 --- a/src/routes/api/proxy/[...path]/+server.ts +++ b/src/routes/api/proxy/[...path]/+server.ts @@ -8,9 +8,12 @@ import { DEFAULT_INSTANCE_URL } from '$lib/app/instance.svelte' * * PURPOSE: * This proxy exists to keep authentication tokens secure by never exposing them - * to the browser. In ATProto OAuth, access tokens are stored in encrypted - * server-side session cookies. The proxy injects the Authorization header on - * behalf of the client, so the client never needs to handle or store tokens. + * to the browser. Authentication is managed via a backend-delegated session: the + * Coves Go backend sets a sealed (encrypted) session cookie during OAuth, and + * the SvelteKit frontend forwards that cookie to the backend's /api/me endpoint + * for validation. The proxy injects the Authorization header (using the sealed + * token from the cookie) on behalf of the client, so the client never needs to + * handle or store tokens. * * TRUST MODEL: * - Client -> Proxy: Client is untrusted. All paths are validated for security @@ -51,7 +54,7 @@ import { DEFAULT_INSTANCE_URL } from '$lib/app/instance.svelte' * 4. Backslash - Windows separator that could bypass Unix-style checks * 5. Encoded separators - %2F (/), %5C (\) that could bypass validation */ -function validateProxyPath(path: string): string | null { +export function validateProxyPath(path: string): string | null { // Check for null bytes (can be used to bypass filters) if (path.includes('\x00')) { return 'Invalid path: null bytes not allowed' @@ -64,7 +67,8 @@ function validateProxyPath(path: string): string | null { // Check for path traversal patterns // This catches: ../, ..\, and URL-encoded variants like %2F, %5C - const traversalPattern = /(?:^|[\\/])\.\.(?:[\\/]|$)|%2e%2e|%252e|%c0%ae|%c1%9c/i + const traversalPattern = + /(?:^|[\\/])\.\.(?:[\\/]|$)|%2e%2e|%252e|%c0%ae|%c1%9c/i if (traversalPattern.test(path)) { return 'Invalid path: path traversal not allowed' } @@ -108,14 +112,14 @@ async function handler({ { status: 400, headers: { 'Content-Type': 'application/json' }, - } + }, ) } // Determine target instance (from session or default) // Instance may already include protocol (e.g., "https://coves.social") or be just the hostname const instance = locals.auth.authenticated - ? locals.auth.activeAccount.instance + ? locals.auth.account.instance : DEFAULT_INSTANCE_URL let baseUrl: string if (instance.startsWith('http://') || instance.startsWith('https://')) { @@ -136,7 +140,7 @@ async function handler({ { status: 400, headers: { 'Content-Type': 'application/json' }, - } + }, ) } // Remove trailing slash from baseUrl if present to avoid double slashes @@ -151,8 +155,9 @@ async function handler({ headers.delete('host') headers.delete('connection') - // Inject Authorization header from encrypted session cookie - // This is the core security benefit: tokens never reach the browser + // Inject Authorization header from the sealed session cookie. + // The sealed token is opaque to the browser (encrypted by the Go backend), + // so raw access/refresh tokens are never exposed to client-side code. if (locals.auth.authenticated) { headers.set('Authorization', `Bearer ${locals.auth.authToken}`) } @@ -186,7 +191,10 @@ async function handler({ const requestId = crypto.randomUUID().slice(0, 8) // Short ID for easier reference // Connection error to upstream - include request context for debugging - console.error(`Proxy error [${request.method} /${path}] [requestId: ${requestId}]:`, error) + console.error( + `Proxy error [${request.method} /${path}] [requestId: ${requestId}]:`, + error, + ) return new Response( JSON.stringify({ error: 'Bad Gateway', @@ -196,7 +204,7 @@ async function handler({ { status: 502, headers: { 'Content-Type': 'application/json' }, - } + }, ) } } diff --git a/src/routes/api/proxy/proxy.test.ts b/src/routes/api/proxy/proxy.test.ts index fd061418..82935a19 100644 --- a/src/routes/api/proxy/proxy.test.ts +++ b/src/routes/api/proxy/proxy.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import type { SealedToken, InstanceURL } from '$lib/server/session' +import { validateProxyPath } from './[...path]/+server' // Mock SvelteKit types for testing - mirrors App.AuthState type MockAuthState = @@ -7,7 +8,7 @@ type MockAuthState = | { authenticated: true authToken: SealedToken - activeAccount: { instance: InstanceURL } + account: { instance: InstanceURL } } interface MockLocals { @@ -19,46 +20,16 @@ interface MockParams { } // Mock fetch type for testing -type MockFetch = (input: RequestInfo | URL, init?: RequestInit) => Promise - -/** - * Validates a proxy path for security issues. - * Returns an error message if the path is invalid, or null if it's safe. - */ -function validateProxyPath(path: string): string | null { - // Check for null bytes (can be used to bypass filters) - if (path.includes('\x00')) { - return 'Invalid path: null bytes not allowed' - } - - // Check for protocol injection attempts - if (/^[a-z][a-z0-9+.-]*:/i.test(path)) { - return 'Invalid path: protocol schemes not allowed' - } - - // Check for path traversal patterns - // This catches: ../, ..\, and URL-encoded variants like %2F, %5C - const traversalPattern = /(?:^|[\\/])\.\.(?:[\\/]|$)|%2e%2e|%252e|%c0%ae|%c1%9c/i - if (traversalPattern.test(path)) { - return 'Invalid path: path traversal not allowed' - } - - // Check for backslash (Windows path separator that could bypass checks) - if (path.includes('\\')) { - return 'Invalid path: backslash not allowed' - } - - // Check for URL-encoded separators that might bypass validation - // %2F = /, %5C = \ - if (/%2f|%5c/i.test(path)) { - return 'Invalid path: encoded path separators not allowed' - } - - return null -} - -// Create the handler function we'll test -// This mirrors the implementation we'll create +type MockFetch = ( + input: RequestInfo | URL, + init?: RequestInit, +) => Promise + +// Create the handler function we'll test. +// This is duplicated from the source because the real handler function is not +// exported (it's an internal implementation detail wrapped by the exported +// RequestHandler functions), and it uses App.Locals which requires the full +// SvelteKit type context. The tests verify the same logic in isolation. async function createHandler(options: { params: MockParams request: Request @@ -76,13 +47,13 @@ async function createHandler(options: { { status: 400, headers: { 'Content-Type': 'application/json' }, - } + }, ) } // Determine target instance (from session or default) const instance = locals.auth.authenticated - ? locals.auth.activeAccount.instance + ? locals.auth.account.instance : 'coves.social' const targetUrl = `https://${instance}/${path}` @@ -122,11 +93,14 @@ async function createHandler(options: { // Connection error to upstream console.error('Proxy error:', error) return new Response( - JSON.stringify({ error: 'Bad Gateway', message: 'Failed to connect to upstream server' }), + JSON.stringify({ + error: 'Bad Gateway', + message: 'Failed to connect to upstream server', + }), { status: 502, headers: { 'Content-Type': 'application/json' }, - } + }, ) } } @@ -134,12 +108,15 @@ async function createHandler(options: { /** * Helper to create authenticated MockLocals */ -function createAuthenticatedLocals(token: string, instance: string): MockLocals { +function createAuthenticatedLocals( + token: string, + instance: string, +): MockLocals { return { auth: { authenticated: true, authToken: token as SealedToken, - activeAccount: { instance: instance as InstanceURL }, + account: { instance: instance as InstanceURL }, }, } } @@ -165,7 +142,10 @@ describe('API Proxy', () => { const calls = mockFetch.mock.calls expect(calls.length).toBeGreaterThan(0) const lastCall = calls[calls.length - 1]! - return [lastCall[0] as string, lastCall[1] as RequestInit & { headers: Headers }] + return [ + lastCall[0] as string, + lastCall[1] as RequestInit & { headers: Headers }, + ] } describe('authenticated requests', () => { @@ -184,7 +164,10 @@ describe('API Proxy', () => { const response = await createHandler({ params: { path: 'api/v1/feed' }, request, - locals: createAuthenticatedLocals('test-jwt-token', 'test.coves.social'), + locals: createAuthenticatedLocals( + 'test-jwt-token', + 'test.coves.social', + ), fetch: mockFetch, }) @@ -213,7 +196,10 @@ describe('API Proxy', () => { const response = await createHandler({ params: { path: 'api/v1/posts' }, request, - locals: createAuthenticatedLocals('test-jwt-token', 'test.coves.social'), + locals: createAuthenticatedLocals( + 'test-jwt-token', + 'test.coves.social', + ), fetch: mockFetch, }) @@ -234,7 +220,7 @@ describe('API Proxy', () => { method: 'GET', headers: { 'Content-Type': 'application/json', - 'Accept': 'application/json', + Accept: 'application/json', 'X-Custom-Header': 'custom-value', 'Accept-Language': 'en-US', }, @@ -354,13 +340,16 @@ describe('API Proxy', () => { { status: 404, headers: { 'Content-Type': 'application/json' }, - } + }, ) mockFetch.mockResolvedValue(errorResponse) - const request = new Request('http://localhost/api/proxy/api/v1/posts/999', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/v1/posts/999', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/v1/posts/999' }, @@ -377,13 +366,16 @@ describe('API Proxy', () => { it('handles 401 responses from upstream', async () => { const errorResponse = new Response( JSON.stringify({ error: 'Unauthorized' }), - { status: 401 } + { status: 401 }, ) mockFetch.mockResolvedValue(errorResponse) - const request = new Request('http://localhost/api/proxy/api/v1/protected', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/v1/protected', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/v1/protected' }, @@ -398,7 +390,7 @@ describe('API Proxy', () => { it('handles 500 responses from upstream', async () => { const errorResponse = new Response( JSON.stringify({ error: 'Internal Server Error' }), - { status: 500 } + { status: 500 }, ) mockFetch.mockResolvedValue(errorResponse) @@ -425,7 +417,7 @@ describe('API Proxy', () => { const request = new Request('http://localhost/api/proxy/api/v1/data', { method: 'GET', headers: { - 'Host': 'localhost:5173', + Host: 'localhost:5173', }, }) @@ -578,9 +570,12 @@ describe('API Proxy', () => { describe('path traversal security', () => { it('rejects paths with ../ traversal attempts', async () => { - const request = new Request('http://localhost/api/proxy/../../../etc/passwd', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/../../../etc/passwd', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: '../../../etc/passwd' }, @@ -599,9 +594,12 @@ describe('API Proxy', () => { it('rejects URL-encoded traversal attempts (..%2F)', async () => { // Note: SvelteKit typically decodes this, but we test the decoded version - const request = new Request('http://localhost/api/proxy/..%2F..%2Fetc%2Fpasswd', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/..%2F..%2Fetc%2Fpasswd', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: '../../etc/passwd' }, // Decoded by SvelteKit @@ -615,9 +613,12 @@ describe('API Proxy', () => { }) it('rejects paths with encoded traversal in the middle', async () => { - const request = new Request('http://localhost/api/proxy/api/v1/../../../etc/passwd', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/v1/../../../etc/passwd', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/v1/../../../etc/passwd' }, @@ -631,9 +632,12 @@ describe('API Proxy', () => { }) it('rejects paths with backslash traversal (Windows-style)', async () => { - const request = new Request('http://localhost/api/proxy/..\\..\\etc\\passwd', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/..\\..\\etc\\passwd', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: '..\\..\\etc\\passwd' }, @@ -647,9 +651,12 @@ describe('API Proxy', () => { }) it('rejects paths with mixed traversal techniques', async () => { - const request = new Request('http://localhost/api/proxy/api/../v1/../../secret', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/../v1/../../secret', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/../v1/../../secret' }, @@ -665,9 +672,12 @@ describe('API Proxy', () => { it('rejects double-encoded traversal attempts (..%252F)', async () => { // Double-encoded: %25 = %, so ..%252F = ..%2F when decoded once // We need to check if the path contains %2F or similar encoded sequences - const request = new Request('http://localhost/api/proxy/..%252F..%252Fetc', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/..%252F..%252Fetc', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: '..%2F..%2Fetc' }, // SvelteKit decodes once @@ -681,9 +691,12 @@ describe('API Proxy', () => { }) it('rejects paths with null bytes', async () => { - const request = new Request('http://localhost/api/proxy/api/v1/data%00.json', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/v1/data%00.json', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/v1/data\x00.json' }, @@ -700,9 +713,12 @@ describe('API Proxy', () => { const mockResponse = new Response('OK', { status: 200 }) mockFetch.mockResolvedValue(mockResponse) - const request = new Request('http://localhost/api/proxy/api/v1/file.json', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/v1/file.json', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/v1/file.json' }, @@ -741,9 +757,12 @@ describe('API Proxy', () => { const mockResponse = new Response('OK', { status: 200 }) mockFetch.mockResolvedValue(mockResponse) - const request = new Request('http://localhost/api/proxy/api/v1/users/user.name@domain.com', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/v1/users/user.name@domain.com', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/v1/users/user.name@domain.com' }, @@ -757,9 +776,12 @@ describe('API Proxy', () => { }) it('rejects paths that would escape the API root after normalization', async () => { - const request = new Request('http://localhost/api/proxy/api/v1/../../../../root', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/api/v1/../../../../root', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'api/v1/../../../../root' }, @@ -773,9 +795,12 @@ describe('API Proxy', () => { }) it('rejects paths with protocol injection attempts', async () => { - const request = new Request('http://localhost/api/proxy/http://evil.com/malicious', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/http://evil.com/malicious', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'http://evil.com/malicious' }, @@ -789,9 +814,12 @@ describe('API Proxy', () => { }) it('rejects paths with javascript protocol', async () => { - const request = new Request('http://localhost/api/proxy/javascript:alert(1)', { - method: 'GET', - }) + const request = new Request( + 'http://localhost/api/proxy/javascript:alert(1)', + { + method: 'GET', + }, + ) const response = await createHandler({ params: { path: 'javascript:alert(1)' }, @@ -819,7 +847,9 @@ describe('API Proxy', () => { const expectedErrorMessage = 'HTTP URLs are not allowed in production' // This is a documentation test showing what the production behavior should be - expect(expectedErrorMessage).toBe('HTTP URLs are not allowed in production') + expect(expectedErrorMessage).toBe( + 'HTTP URLs are not allowed in production', + ) // The handler in +server.ts lines 82-93 implements: // if (import.meta.env.PROD && baseUrl.startsWith('http://')) { diff --git a/src/routes/create/post/+page.svelte b/src/routes/create/post/+page.svelte index 7e47c22e..6c9e2fdc 100644 --- a/src/routes/create/post/+page.svelte +++ b/src/routes/create/post/+page.svelte @@ -8,7 +8,7 @@ import { PostFormState, type PostFormInit, - } from '$lib/feature/post/form/postform.svelte.js' + } from '$lib/feature/post/form/post-form.svelte' import { postLink } from '$lib/feature/post/helpers.js' import { onDestroy } from 'svelte' diff --git a/src/routes/signup/[instance]/+page.svelte b/src/routes/signup/[instance]/+page.svelte index 0ea10a40..a4bf5007 100644 --- a/src/routes/signup/[instance]/+page.svelte +++ b/src/routes/signup/[instance]/+page.svelte @@ -59,7 +59,10 @@ verifying = $state(false) // eslint-disable-next-line @typescript-eslint/no-unused-vars - const _instanceType: ClientType = $state({ name: 'lemmy', baseUrl: '/api/v3' }) + const _instanceType: ClientType = $state({ + name: 'lemmy', + baseUrl: '/api/v3', + }) const getCaptcha = async () => (captcha = await getClient(instance, fetch).getCaptcha()) @@ -89,7 +92,7 @@ if (res?.jwt) { // Account created successfully - redirect to login for OAuth authentication - // Direct signup doesn't provide the DID/sessionId needed for full authentication + // Direct signup doesn't establish an OAuth session; the user must complete the OAuth flow toast({ content: $t('toast.successSignup'), type: 'success',