diff --git a/src/oauth-protected-resource/responses.ts b/src/oauth-protected-resource/responses.ts index 356224d..279ac18 100644 --- a/src/oauth-protected-resource/responses.ts +++ b/src/oauth-protected-resource/responses.ts @@ -1,4 +1,9 @@ -import { getAuthUrl, getResourceMetadataUrl, getResourceUrl } from './url.js' +import { + getAuthUrl, + getResourceMetadataUrl, + getResourceUrl, + percentEncodeQuotes, +} from './url.js' import type { ResourceMetadataOptions, UnauthorizedResponseOptions, @@ -6,8 +11,8 @@ import type { /** * `401` response with a `WWW-Authenticate: Bearer resource_metadata="..."` header (RFC 9728). - * Auto-constructs the metadata URL from `X-Forwarded-*` headers. - * Pass `resourceMetadataUrl` to override for custom setups. + * The metadata URL defaults to the Edge Functions derivation and throws off + * platform; pass `resourceMetadataUrl` to override for custom setups. * * @category Middleware */ @@ -15,8 +20,9 @@ export function unauthorizedResponse( req: Request, options?: UnauthorizedResponseOptions, ): Response { - const metadataUrl = - options?.resourceMetadataUrl ?? getResourceMetadataUrl(req) + const metadataUrl = percentEncodeQuotes( + options?.resourceMetadataUrl ?? getResourceMetadataUrl(req), + ) return new Response(JSON.stringify({ error: 'Unauthorized' }), { status: 401, headers: { @@ -29,7 +35,8 @@ export function unauthorizedResponse( /** * RFC 9728 OAuth Protected Resource Metadata response. * Advertises the authorization server, resource URI, and bearer methods supported. - * Auto-constructs URLs from `X-Forwarded-*` headers. + * URLs default to the Edge Functions derivation and throw off platform; pass + * `resource` / `authorizationServers` to override. * * @category Middleware */ diff --git a/src/oauth-protected-resource/url.ts b/src/oauth-protected-resource/url.ts index 988642d..8f0fc2a 100644 --- a/src/oauth-protected-resource/url.ts +++ b/src/oauth-protected-resource/url.ts @@ -28,6 +28,18 @@ export function trimTrailingSlash(value: string): string { return value.endsWith('/') ? value.slice(0, -1) : value } +/** + * Percent-encodes `"` and `\` — invalid URL code points that would otherwise + * break out of the RFC 9110 quoted-string in `WWW-Authenticate`. Applied to + * every advertised URL, so the header, the metadata document, and the ctx + * contribution agree on one spelling (RFC 9728 §3.3 requires an exact match). + * + * @internal + */ +export function percentEncodeQuotes(value: string): string { + return value.replace(/["\\]/g, (c) => (c === '"' ? '%22' : '%5C')) +} + /** * The externally-visible origin of a Supabase Edge Function. * @@ -76,6 +88,10 @@ function edgeOrigin(req: Request): string { * The two forms differ only for a request at a sub-path of the function: the * canonical one reports the function, the reconstructed one the sub-path. * + * @throws {EnvError} `MISSING_RESOURCE_SERVER` on a root path with no slug — + * there is no function segment to restore, and a bare `/functions/v1` + * identifies no resource. + * * @internal */ function edgeResourcePath(req: Request): string { @@ -86,6 +102,9 @@ function edgeResourcePath(req: Request): string { METADATA_SUFFIX_PATTERN, '', ) + if (received === '' || received === '/') { + throw Errors[MissingResourceServerError]() + } return `${EDGE_FUNCTIONS_PATH_PREFIX}${received}` } @@ -96,7 +115,8 @@ function edgeResourcePath(req: Request): string { * There is no environment fallback — `SUPABASE_URL` names the Supabase project, * not this endpoint — so off Edge Functions it throws. * - * @throws {EnvError} `MISSING_RESOURCE_SERVER` off Edge Functions. + * @throws {EnvError} `MISSING_RESOURCE_SERVER` off Edge Functions, or on a + * root path with no `SUPABASE_FUNCTION_SLUG` — see {@link edgeResourcePath}. * * @internal */ @@ -170,7 +190,7 @@ export function resolveUrlOption( fallback: (req: Request) => string, ): string { const value = typeof option === 'function' ? option(req) : option - return trimTrailingSlash(value ?? fallback(req)) + return percentEncodeQuotes(trimTrailingSlash(value ?? fallback(req))) } /** diff --git a/src/oauth-protected-resource/with-oauth-protected-resource.test.ts b/src/oauth-protected-resource/with-oauth-protected-resource.test.ts index 3f4f6e4..a718bea 100644 --- a/src/oauth-protected-resource/with-oauth-protected-resource.test.ts +++ b/src/oauth-protected-resource/with-oauth-protected-resource.test.ts @@ -1,3 +1,6 @@ +import { createServer } from 'node:http' +import type { AddressInfo } from 'node:net' + import { afterEach, describe, expect, it, vi } from 'vitest' import { pipeline } from '@supabase/middleware' @@ -247,6 +250,15 @@ describe('unauthorizedResponse', () => { `Bearer resource_metadata="${url}"`, ) }) + + it('percent-encodes `"` in the resourceMetadataUrl override (quoted-string integrity)', () => { + const res = unauthorizedResponse(req('POST', '/my-fn'), { + resourceMetadataUrl: 'https://x.example.com/a"b/oauth-protected-resource', + }) + expect(res.headers.get('WWW-Authenticate')).toBe( + 'Bearer resource_metadata="https://x.example.com/a%22b/oauth-protected-resource"', + ) + }) }) describe('withOAuthProtectedResource - resourceServer / authorizationServer', () => { @@ -776,3 +788,129 @@ describe('withOAuthProtectedResource - off-platform defaults fail loudly', () => ]) }) }) + +describe('withOAuthProtectedResource - WWW-Authenticate quoted-string integrity', () => { + // A raw `"` in the advertised URL terminates the RFC 9110 quoted-string + // early, letting the remainder parse as extra auth-params (parameter + // injection). `"` and `\` are invalid URL code points anyway, so they are + // percent-encoded — in the header, the metadata document, and the ctx + // contribution alike, keeping the RFC 9728 §3.3 comparison intact. + it('percent-encodes `"` arriving via X-Forwarded-Host', async () => { + const res = await withOAuthProtectedResource(returns401)( + req('POST', '/my-fn', { 'X-Forwarded-Host': 'evil.test", scope="admin' }), + ) + expect(res.headers.get('WWW-Authenticate')).toBe( + 'Bearer resource_metadata="http://evil.test%22, scope=%22admin/functions/v1/my-fn/oauth-protected-resource"', + ) + }) + + it('percent-encodes `"` and `\\` in a configured resourceServer', async () => { + const res = await withOAuthProtectedResource( + { + resourceServer: 'https://api.example.com/m"c\\p', + authorizationServer: 'https://auth.example.com', + }, + returns401, + )(req('POST', '/my-fn')) + expect(res.headers.get('WWW-Authenticate')).toBe( + 'Bearer resource_metadata="https://api.example.com/m%22c%5Cp/oauth-protected-resource"', + ) + }) + + it('advertises the same encoded resource in the metadata document (§3.3 agreement)', async () => { + const res = await withOAuthProtectedResource(passthrough)( + req('GET', '/my-fn/oauth-protected-resource', { + 'X-Forwarded-Host': 'evil.test"h', + }), + ) + const body = await res.json() + expect(body.resource).toBe('http://evil.test%22h/functions/v1/my-fn') + }) +}) + +describe('withOAuthProtectedResource - 401 enrichment resilience', () => { + it('enriches a 401 whose body the handler already consumed', async () => { + const handler = async () => { + const res = new Response('denied', { status: 401 }) + await res.text() + return res + } + const res = await withOAuthProtectedResource(handler)(req('POST', '/my-fn')) + expect(res.status).toBe(401) + expect(res.headers.get('WWW-Authenticate')).toMatch(/^Bearer /) + }) + + it('enriches the 401 in place, so fetch-carried fields (.url, .redirected) survive', async () => { + let issued: Response | undefined + const handler = async () => { + issued = new Response(null, { status: 401 }) + return issued + } + const res = await withOAuthProtectedResource(handler)(req('POST', '/my-fn')) + expect(res).toBe(issued) + }) + + it('enriches a fetch()-proxied 401, whose headers are immutable', async () => { + const upstream = createServer((_req, res) => { + res.statusCode = 401 + res.setHeader('X-Upstream', 'yes') + res.end('denied') + }) + await new Promise((resolve) => upstream.listen(0, resolve)) + const { port } = upstream.address() as AddressInfo + try { + const handler = async () => fetch(`http://127.0.0.1:${port}/`) + const res = await withOAuthProtectedResource(handler)( + req('POST', '/my-fn'), + ) + expect(res.status).toBe(401) + expect(res.headers.get('WWW-Authenticate')).toMatch(/resource_metadata=/) + expect(res.headers.get('X-Upstream')).toBe('yes') + expect(await res.text()).toBe('denied') + } finally { + upstream.close() + } + }) +}) + +describe('withOAuthProtectedResource - root path (no function segment)', () => { + // With no slug and no path segment there is no function name to restore, so + // the reconstruction would advertise a bare `/functions/v1` — a URL that + // identifies no resource. Failing loudly matches the off-platform contract. + it('throws MISSING_RESOURCE_SERVER on a bare /oauth-protected-resource (edge default)', async () => { + setEnv('SUPABASE_PUBLIC_URL', undefined) + setEnv('SUPABASE_FUNCTION_SLUG', undefined) + await expect( + withOAuthProtectedResource(passthrough)( + req('GET', '/oauth-protected-resource'), + ), + ).rejects.toMatchObject({ + constructor: EnvError, + code: MissingResourceServerError, + status: 500, + }) + }) + + it('resourceMetadataResponse on a root path throws instead of advertising a bare /functions/v1', () => { + setEnv('SUPABASE_PUBLIC_URL', undefined) + setEnv('SUPABASE_FUNCTION_SLUG', undefined) + let thrown: unknown + try { + resourceMetadataResponse(req('GET', '/')) + } catch (e) { + thrown = e + } + expect(thrown).toBeInstanceOf(EnvError) + expect(thrown).toMatchObject({ code: MissingResourceServerError }) + }) + + it('SUPABASE_FUNCTION_SLUG rescues a root path with a canonical identifier', async () => { + setEnv('SUPABASE_PUBLIC_URL', undefined) + setEnv('SUPABASE_FUNCTION_SLUG', 'my-fn') + const res = await withOAuthProtectedResource(passthrough)( + req('GET', '/oauth-protected-resource'), + ) + const body = await res.json() + expect(body.resource).toBe('http://localhost/functions/v1/my-fn') + }) +}) diff --git a/src/oauth-protected-resource/with-oauth-protected-resource.ts b/src/oauth-protected-resource/with-oauth-protected-resource.ts index 158c341..5c3bcea 100644 --- a/src/oauth-protected-resource/with-oauth-protected-resource.ts +++ b/src/oauth-protected-resource/with-oauth-protected-resource.ts @@ -166,16 +166,23 @@ export const withOAuthProtectedResource: Middleware< response.status === 401 && !response.headers.has('WWW-Authenticate') ) { - const headers = new Headers(response.headers) - headers.set( - 'WWW-Authenticate', - `Bearer resource_metadata="${resourceMetadataUrl}"`, - ) - return new Response(response.body, { - status: 401, - statusText: response.statusText, - headers, - }) + const challenge = `Bearer resource_metadata="${resourceMetadataUrl}"` + try { + response.headers.set('WWW-Authenticate', challenge) + return response + } catch { + // Headers on a fetch()-proxied response carry the immutable guard, + // so enrichment falls back to a copy. A copy cannot carry `.url` or + // `.redirected` and cannot reuse a consumed body stream, which is + // why in-place mutation is the primary path. + const headers = new Headers(response.headers) + headers.set('WWW-Authenticate', challenge) + return new Response(response.bodyUsed ? null : response.body, { + status: response.status, + statusText: response.statusText, + headers, + }) + } } return response