Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,37 @@ const config = [
'react-hooks/immutability': 'off',
},
},
// P1.5 — Discourage bare `fetch('/api/...')`.
// The team must migrate to `apiFetch<T>()` from `@/lib/api-client` so that
// 401 / 403 / 5xx / network failures are handled uniformly.
// Phase 1 (this PR): warn level, allow incremental migration.
// Phase 3 (final cleanup): upgrade to error and forbid merges that introduce
// new bare fetch('/api/...') sites.
// Selector rationale:
// - covers single-quoted, double-quoted, and template-literal forms
// - filters by /api prefix so cross-origin / external fetches stay untouched
// - exempts api-client.ts itself (the one allowed implementer)
{
files: ['src/**/*.{ts,tsx,js,jsx}'],
ignores: ['src/lib/api-client.ts'],
rules: {
'no-restricted-syntax': [
'warn',
{
selector:
"CallExpression[callee.name='fetch'] > Literal[value=/^\\/api\\//]",
message:
"Use apiFetch<T>() from '@/lib/api-client' instead of bare fetch('/api/...'). It handles 401 redirect, 403/5xx typed errors, and network failures uniformly. See PR-api-client.md.",
},
{
selector:
"CallExpression[callee.name='fetch'] > TemplateLiteral.arguments:first-child[quasis.0.value.raw=/^\\/api\\//]",
message:
"Use apiFetch<T>() from '@/lib/api-client' instead of bare fetch(`/api/...`). It handles 401 redirect, 403/5xx typed errors, and network failures uniformly.",
},
],
},
},
]

export default config
2 changes: 2 additions & 0 deletions src/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { NextIntlClientProvider } from 'next-intl'
import { getLocale, getMessages } from 'next-intl/server'
import { THEME_IDS } from '@/lib/themes'
import { ThemeBackground } from '@/components/ui/theme-background'
import { AuthExpiredListener } from '@/components/auth-expired-listener'
import './globals.css'

const inter = Inter({
Expand Down Expand Up @@ -114,6 +115,7 @@ export default async function RootLayout({
disableTransitionOnChange
>
<ThemeBackground />
<AuthExpiredListener />
<div className="h-screen overflow-hidden bg-background text-foreground">
{children}
</div>
Expand Down
33 changes: 33 additions & 0 deletions src/components/auth-expired-listener.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use client'

import { useEffect } from 'react'

/**
* Listens for `mc:auth-expired` CustomEvent dispatched by `apiFetch()` when the
* server returns 401. The redirect to `/login?from=...` is already handled inside
* `apiFetch`; this listener exists so we have a single observability hook the
* team can extend (toast, telemetry, Sentry).
*
* Mounted once at the root layout. SSR-safe (effect runs only on the client).
*
* Why a separate component?
* - layout.tsx is a server component (uses `await headers()`); we cannot
* attach window listeners there directly.
* - Co-locating the listener with the api-client keeps the auth-failure
* contract in one place.
*/
export function AuthExpiredListener(): null {
useEffect(() => {
const onExpired = (e: Event) => {
const detail = (e as CustomEvent<{ path: string; status: number }>).detail
// No toast lib installed yet — log for now. Replace with sonner in next PR.
console.warn(
`[mc] session expired on ${detail?.path ?? 'unknown'} (status=${detail?.status ?? 401}), redirecting to /login`
)
}
window.addEventListener('mc:auth-expired', onExpired)
return () => window.removeEventListener('mc:auth-expired', onExpired)
}, [])

return null
}
114 changes: 114 additions & 0 deletions src/lib/__tests__/api-client.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
import { apiFetch, ApiError } from '../api-client'

const realFetch = global.fetch
const realLocation = window.location

function mockResponse(status: number, body: unknown = {}, opts: { json?: boolean } = { json: true }) {
return new Response(opts.json ? JSON.stringify(body) : String(body), {
status,
headers: { 'Content-Type': 'application/json' },
})
}

describe('apiFetch — global 401 / 403 / 5xx / network handling', () => {
let dispatched: CustomEvent[] = []
let originalHref = ''
let authExpiredListener: ((e: Event) => void) | null = null

beforeEach(() => {
dispatched = []
authExpiredListener = (e) => dispatched.push(e as CustomEvent)
window.addEventListener('mc:auth-expired', authExpiredListener)

// Stub location so redirect-to-login is observable
originalHref = window.location.href
Object.defineProperty(window, 'location', {
writable: true,
value: {
...realLocation,
pathname: '/cost-tracker',
search: '',
href: 'http://127.0.0.1:3000/cost-tracker',
},
})
})

afterEach(() => {
if (authExpiredListener) {
window.removeEventListener('mc:auth-expired', authExpiredListener)
authExpiredListener = null
}
global.fetch = realFetch
Object.defineProperty(window, 'location', { writable: true, value: realLocation })
window.location.href = originalHref
vi.restoreAllMocks()
})

it('returns parsed JSON on 200', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(200, { ok: true, count: 42 }))
const data = await apiFetch<{ ok: boolean; count: number }>('/api/tokens')
expect(data).toEqual({ ok: true, count: 42 })
})

it('emits mc:auth-expired and redirects to /login on 401', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(401, { error: 'Authentication required' }))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'UNAUTHENTICATED',
status: 401,
})
expect(dispatched).toHaveLength(1)
expect(dispatched[0].detail).toMatchObject({ path: '/api/tokens', status: 401 })
expect(window.location.href).toContain('/login?from=%2Fcost-tracker')
})

it('does NOT redirect when redirectOnUnauthenticated=false', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(401, { error: 'Authentication required' }))
await expect(
apiFetch('/api/tokens', { redirectOnUnauthenticated: false })
).rejects.toThrow(ApiError)
expect(window.location.href).toBe('http://127.0.0.1:3000/cost-tracker')
})

it('throws FORBIDDEN on 403 without redirecting', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(403, { error: 'Requires admin role' }))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'FORBIDDEN',
status: 403,
})
expect(window.location.href).toBe('http://127.0.0.1:3000/cost-tracker')
})

it('throws SERVER_ERROR on 500 with upstream message', async () => {
global.fetch = vi.fn().mockResolvedValue(mockResponse(500, { error: 'database is locked' }))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'SERVER_ERROR',
status: 500,
message: 'database is locked',
})
})

it('throws NETWORK_ERROR when fetch rejects', async () => {
global.fetch = vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))
await expect(apiFetch('/api/tokens')).rejects.toMatchObject({
code: 'NETWORK_ERROR',
status: 0,
})
})

it('does not redirect when already on /login (avoid infinite loop)', async () => {
Object.defineProperty(window, 'location', {
writable: true,
value: { ...realLocation, pathname: '/login', search: '', href: 'http://127.0.0.1:3000/login' },
})
global.fetch = vi.fn().mockResolvedValue(mockResponse(401))
await expect(apiFetch('/api/auth/login', { method: 'POST' })).rejects.toThrow(ApiError)
expect(window.location.href).toBe('http://127.0.0.1:3000/login')
})

it('returns undefined for 204 No Content', async () => {
global.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 204 }))
const data = await apiFetch('/api/sessions/123', { method: 'DELETE' })
expect(data).toBeUndefined()
})
})
145 changes: 145 additions & 0 deletions src/lib/api-client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/**
* API client with global 401 / 403 / network handling.
*
* Why this exists
* ---------------
* Mission Control had no global auth-failure handler. When a session expired,
* `fetch('/api/...')` returned 401 silently and panels (cost-tracker, dashboard,
* activities) stuck on loading skeletons forever — users perceived "the service
* died" but the backend was healthy. Root cause logged in:
* src/lib/auth.ts:629 requireRole()
*
* Contract
* --------
* apiFetch<T>(path, init?) -> Promise<T>
*
* - Always sends cookies (credentials: 'include')
* - On 401: emits an `mc:auth-expired` CustomEvent, redirects to /login?from=…
* - On 403: throws ApiError with code='FORBIDDEN'
* - On 5xx: throws ApiError with code='SERVER_ERROR' and the upstream message
* - On network failure: throws ApiError with code='NETWORK_ERROR'
*
* Listen once at app root:
* window.addEventListener('mc:auth-expired', () => showToast('Session expired'))
*/

export type ApiErrorCode =
| 'UNAUTHENTICATED'
| 'FORBIDDEN'
| 'NOT_FOUND'
| 'SERVER_ERROR'
| 'NETWORK_ERROR'
| 'PARSE_ERROR'

export class ApiError extends Error {
readonly code: ApiErrorCode
readonly status: number
readonly payload: unknown

constructor(code: ApiErrorCode, status: number, message: string, payload?: unknown) {
super(message)
this.name = 'ApiError'
this.code = code
this.status = status
this.payload = payload
}
}

// Browser-only redirect to /login. SSR / unit tests skip it.
function redirectToLogin(): void {
if (typeof window === 'undefined') return
const from = window.location.pathname + window.location.search
// Avoid redirect loops if user is already on /login
if (window.location.pathname === '/login') return
window.location.href = `/login?from=${encodeURIComponent(from)}`
}

function emitAuthExpired(detail: { path: string; status: number }): void {
if (typeof window === 'undefined') return
window.dispatchEvent(new CustomEvent('mc:auth-expired', { detail }))
}
export interface ApiFetchOptions extends RequestInit {
/** When true (default) and the response is 401, redirect to /login. */
redirectOnUnauthenticated?: boolean
/** When true, return raw Response instead of parsed JSON. */
raw?: boolean
}

export async function apiFetch<T = unknown>(
path: string,
options: ApiFetchOptions = {}
): Promise<T> {
const {
redirectOnUnauthenticated = true,
raw = false,
headers,
...rest
} = options

let response: Response
try {
response = await fetch(path, {
credentials: 'include',
headers: {
Accept: 'application/json',
...(rest.body && !(rest.body instanceof FormData)
? { 'Content-Type': 'application/json' }
: {}),
...headers,
},
...rest,
})
} catch (err) {
throw new ApiError(
'NETWORK_ERROR',
0,
err instanceof Error ? err.message : 'Network request failed'
)
}

// 401 — emit event, optionally redirect, always throw
if (response.status === 401) {
emitAuthExpired({ path, status: 401 })
if (redirectOnUnauthenticated) redirectToLogin()
throw new ApiError('UNAUTHENTICATED', 401, 'Authentication required')
}

// 403 — throw, do NOT redirect (user is logged in but lacks role)
if (response.status === 403) {
const payload = await safeParseJson(response)
throw new ApiError('FORBIDDEN', 403, 'Insufficient permissions', payload)
}

if (response.status === 404) {
throw new ApiError('NOT_FOUND', 404, `Not found: ${path}`)
}

if (response.status >= 500) {
const payload = await safeParseJson(response)
const msg =
(typeof payload === 'object' && payload !== null && 'error' in payload &&
typeof (payload as { error: unknown }).error === 'string'
? (payload as { error: string }).error
: null) || `Server error ${response.status}`
throw new ApiError('SERVER_ERROR', response.status, msg, payload)
}

if (raw) return response as unknown as T

if (response.status === 204) return undefined as T

try {
return (await response.json()) as T
} catch {
throw new ApiError('PARSE_ERROR', response.status, 'Response was not valid JSON')
}
}

async function safeParseJson(res: Response): Promise<unknown> {
try {
return await res.json()
} catch {
return null
}
}