Validate API paths against the page origin, not a placeholder
The guard resolved paths against a fixed placeholder origin, so //same-origin.invalid/api/v1/me passed and fetch sent the CSRF token to that host. apiFetch now resolves against location.origin, refuses anything off that origin or outside /api/v1/, and fetches the validated path-absolute URL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -37,6 +37,10 @@ describe('apiFetch', () => {
|
|||||||
it.each([
|
it.each([
|
||||||
'https://evil.example/api/v1/me',
|
'https://evil.example/api/v1/me',
|
||||||
'//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',
|
'/auth/logout',
|
||||||
'/api/v1/../auth/logout',
|
'/api/v1/../auth/logout',
|
||||||
'/api/v1/%2e%2e/auth/logout',
|
'/api/v1/%2e%2e/auth/logout',
|
||||||
|
|||||||
+14
-12
@@ -8,16 +8,17 @@ export const ME_PATH = '/api/v1/me'
|
|||||||
// API_PREFIX is the only path apiFetch sends to.
|
// API_PREFIX is the only path apiFetch sends to.
|
||||||
const API_PREFIX = '/api/v1/'
|
const API_PREFIX = '/api/v1/'
|
||||||
|
|
||||||
// requireApiPath refuses any path that would not resolve to a same-origin URL
|
// apiURL resolves path against the page origin and returns it as the
|
||||||
// under /api/v1/, so the CSRF token is never sent anywhere else. Resolving
|
// path-absolute URL to fetch. It refuses any path that would not resolve to a
|
||||||
// against a placeholder origin catches absolute and protocol-relative URLs
|
// same-origin URL under /api/v1/, so the CSRF token is never sent anywhere
|
||||||
// and dot segments that climb out of /api/v1/.
|
// else: absolute and protocol-relative URLs on another origin, and dot
|
||||||
function requireApiPath(method: string, path: string) {
|
// segments that climb out of /api/v1/.
|
||||||
const base = 'https://same-origin.invalid'
|
function apiURL(origin: string, method: string, path: string): string {
|
||||||
const url = new URL(path, base)
|
const url = new URL(path, origin)
|
||||||
if (!path.startsWith('/') || url.origin !== base || !url.pathname.startsWith(API_PREFIX)) {
|
if (!path.startsWith('/') || url.origin !== origin || !url.pathname.startsWith(API_PREFIX)) {
|
||||||
throw new Error(`${method} ${path}: not a same-origin ${API_PREFIX} path`)
|
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
|
// Me is the GET /api/v1/me response: the signed-in member, the household and
|
||||||
@@ -30,7 +31,8 @@ export type Me = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ApiLocation is the part of window.location the client reads and navigates.
|
// ApiLocation is the part of window.location the client reads and navigates.
|
||||||
export type ApiLocation = Pick<Location, 'pathname' | 'search' | 'assign'>
|
// origin is the page origin every request must stay on.
|
||||||
|
export type ApiLocation = Pick<Location, 'origin' | 'pathname' | 'search' | 'assign'>
|
||||||
|
|
||||||
export type ApiRequest = {
|
export type ApiRequest = {
|
||||||
method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'
|
method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'
|
||||||
@@ -87,15 +89,15 @@ export function createApiClient(location: ApiLocation): ApiClient {
|
|||||||
|
|
||||||
async function apiFetch<T>(path: string, request: ApiRequest = {}): Promise<T> {
|
async function apiFetch<T>(path: string, request: ApiRequest = {}): Promise<T> {
|
||||||
const method = request.method ?? 'GET'
|
const method = request.method ?? 'GET'
|
||||||
requireApiPath(method, path)
|
const url = apiURL(location.origin, method, path)
|
||||||
let response = await send(method, path, request.body)
|
let response = await send(method, url, request.body)
|
||||||
if (response.status === 403) {
|
if (response.status === 403) {
|
||||||
const error = await parseForbidden(method, path, response)
|
const error = await parseForbidden(method, path, response)
|
||||||
// Only a non-GET carries the token, so only it can be refused for it.
|
// Only a non-GET carries the token, so only it can be refused for it.
|
||||||
if (error.code !== 'csrf' || method === 'GET') throw error
|
if (error.code !== 'csrf' || method === 'GET') throw error
|
||||||
// The session's token changed: refresh it once and retry once.
|
// The session's token changed: refresh it once and retry once.
|
||||||
await refreshMe()
|
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)
|
if (response.status === 403) throw await parseForbidden(method, path, response)
|
||||||
}
|
}
|
||||||
return decode<T>(method, path, response)
|
return decode<T>(method, path, response)
|
||||||
|
|||||||
@@ -42,7 +42,8 @@ export function request(fetchMock: Mock<typeof fetch>, 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> } {
|
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>() }
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user