diff --git a/web/package.json b/web/package.json index 0055edc..9ce06bc 100644 --- a/web/package.json +++ b/web/package.json @@ -5,7 +5,7 @@ "type": "module", "scripts": { "dev": "vite", - "build": "tsc --noEmit && vite build && npm run check:dist-fonts", + "build": "npm run lint:colors && tsc --noEmit && vite build && npm run check:dist-fonts", "preview": "vite preview", "typecheck": "tsc --noEmit", "test": "npm run lint:colors && vitest run", diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index 9a2d2c8..f6fa5be 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -34,6 +34,30 @@ describe('apiFetch', () => { expect(fetchMock).not.toHaveBeenCalled() }) + it.each([ + 'https://evil.example/api/v1/me', + '//evil.example/api/v1/me', + // The placeholder origin an earlier guard compared against. + '//same-origin.invalid/api/v1/me', + 'https://same-origin.invalid/api/v1/me', + '/\\evil.example/api/v1/me', + '/auth/logout', + '/api/v1/../auth/logout', + '/api/v1/%2e%2e/auth/logout', + 'api/v1/me', + ])('refuses %s without sending it or the CSRF token', async (path) => { + const fetchMock = stubFetch(jsonResponse(200, ME)) + const api = createApiClient(fakeLocation()) + await api.loadMe() + + await expect(api.apiFetch(path, { method: 'POST', body: {} })).rejects.toThrow( + `POST ${path}: not a same-origin /api/v1/ path`, + ) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(request(fetchMock, 0)).toMatchObject({ path: '/api/v1/me', method: 'GET' }) + expect(request(fetchMock, 0).headers.has(CSRF_HEADER)).toBe(false) + }) + it('rejects with a typed ApiError from the error body', async () => { stubFetch(errorResponse(503, 'unavailable', true)) const api = createApiClient(fakeLocation()) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index ac5cc2c..a4f840e 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -5,6 +5,22 @@ export const CSRF_HEADER = 'X-MealQuest-CSRF' export const ME_PATH = '/api/v1/me' +// API_PREFIX is the only path apiFetch sends to. +const API_PREFIX = '/api/v1/' + +// apiURL resolves path against the page origin and returns it as the +// path-absolute URL to fetch. It refuses any path that would not resolve to a +// same-origin URL under /api/v1/, so the CSRF token is never sent anywhere +// else: absolute and protocol-relative URLs on another origin, and dot +// segments that climb out of /api/v1/. +function apiURL(origin: string, method: string, path: string): string { + const url = new URL(path, origin) + if (!path.startsWith('/') || url.origin !== origin || !url.pathname.startsWith(API_PREFIX)) { + throw new Error(`${method} ${path}: not a same-origin ${API_PREFIX} path`) + } + return url.pathname + url.search +} + // Me is the GET /api/v1/me response: the signed-in member, the household and // the session's CSRF token. export type Me = { @@ -15,7 +31,8 @@ export type Me = { } // ApiLocation is the part of window.location the client reads and navigates. -export type ApiLocation = Pick +// origin is the page origin every request must stay on. +export type ApiLocation = Pick export type ApiRequest = { method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' @@ -24,8 +41,9 @@ export type ApiRequest = { } export type ApiClient = { - // apiFetch sends a same-origin JSON request to path and resolves with the - // decoded response body, or rejects with an ApiError. + // apiFetch sends a same-origin JSON request to path, which must be under + // /api/v1/, and resolves with the decoded response body, or rejects with an + // ApiError. apiFetch(path: string, request?: ApiRequest): Promise // loadMe fetches GET /api/v1/me and caches its CSRF token. loadMe(): Promise @@ -71,14 +89,15 @@ export function createApiClient(location: ApiLocation): ApiClient { async function apiFetch(path: string, request: ApiRequest = {}): Promise { const method = request.method ?? 'GET' - let response = await send(method, path, request.body) + const url = apiURL(location.origin, method, path) + let response = await send(method, url, request.body) if (response.status === 403) { const error = await parseForbidden(method, path, response) // Only a non-GET carries the token, so only it can be refused for it. if (error.code !== 'csrf' || method === 'GET') throw error // The session's token changed: refresh it once and retry once. await refreshMe() - response = await send(method, path, request.body) + response = await send(method, url, request.body) if (response.status === 403) throw await parseForbidden(method, path, response) } return decode(method, path, response) diff --git a/web/src/test/fetchStub.ts b/web/src/test/fetchStub.ts index 9e84d14..9a7a932 100644 --- a/web/src/test/fetchStub.ts +++ b/web/src/test/fetchStub.ts @@ -42,7 +42,8 @@ export function request(fetchMock: Mock, n: number) { } } -// fakeLocation stands in for window.location, recording navigations. +// fakeLocation stands in for window.location on https://mealquest.test, +// recording navigations. export function fakeLocation(pathname = '/', search = ''): ApiLocation & { assign: Mock<(url: string | URL) => void> } { - return { pathname, search, assign: vi.fn<(url: string | URL) => void>() } + return { origin: 'https://mealquest.test', pathname, search, assign: vi.fn<(url: string | URL) => void>() } }