From 5b8463c6f4517d9c2a73d8eba267edc868d792a6 Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Mon, 23 Feb 2026 14:22:11 +0000 Subject: [PATCH 1/2] fix: classify upstream errors correctly instead of returning 500 Upstream 4xx from Cloudflare OAuth and API endpoints were thrown as generic Error objects, caught by route handlers, and returned as 500 Server Error pages. This violates the MCP auth spec which requires 401 for invalid tokens, 403 for insufficient permissions, and 400 for malformed requests. Changes: - Preserve upstream HTTP status codes (400/401/403/429) through OAuthError with RFC-precise error codes (invalid_grant, invalid_token, insufficient_scope, etc.) - Map upstream 5xx to 502 Bad Gateway - Replace generic Error throws in parseRedirectApproval with typed OAuthError instances - Forward OAuthError statusCode through renderErrorPage and toHtmlResponse - Add error correlation IDs to unexpected error responses for log tracing - Remove raw error message leakage from client-facing responses - Respect OAuthError status in API token mode catch block --- src/auth/api-token-mode.ts | 7 ++++-- src/auth/cloudflare-auth.ts | 29 ++++++++++++++++++++---- src/auth/oauth-handler.ts | 39 +++++++++++++++++++++++++++------ src/auth/workers-oauth-utils.ts | 20 ++++++++++------- 4 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/auth/api-token-mode.ts b/src/auth/api-token-mode.ts index eed7f19..5c222f9 100644 --- a/src/auth/api-token-mode.ts +++ b/src/auth/api-token-mode.ts @@ -1,4 +1,5 @@ import { getUserAndAccounts } from './oauth-handler' +import { OAuthError } from './workers-oauth-utils' import type { AuthProps } from './types' @@ -76,8 +77,10 @@ export async function handleApiTokenRequest( const props = buildAuthProps(token, user, accounts) return createMcpResponse(token, undefined, props) } catch (err) { - const message = err instanceof Error ? err.message : 'Token verification failed' - return new Response(JSON.stringify({ error: message }), { + if (err instanceof OAuthError) { + return err.toResponse() + } + return new Response(JSON.stringify({ error: 'Token verification failed' }), { status: 401, headers: { 'Content-Type': 'application/json' } }) diff --git a/src/auth/cloudflare-auth.ts b/src/auth/cloudflare-auth.ts index 0ed18d9..be80ee5 100644 --- a/src/auth/cloudflare-auth.ts +++ b/src/auth/cloudflare-auth.ts @@ -2,6 +2,27 @@ import { z } from 'zod' import type { AuthRequest } from '@cloudflare/workers-oauth-provider' +import { OAuthError } from './workers-oauth-utils' + +/** + * Convert an upstream Cloudflare OAuth error response to an OAuthError. + * 4xx: preserves the status code with a safe message. + * 5xx: uses 502 Bad Gateway (we're proxying). + */ +function throwUpstreamError(status: number, context: string): never { + if (status >= 500) { + throw new OAuthError('server_error', `${context}: upstream service unavailable`, 502) + } + const codeMap: Record = { + 400: ['invalid_grant', `${context}: invalid or expired grant`], + 401: ['invalid_client', `${context}: invalid client credentials`], + 403: ['unauthorized_client', `${context}: insufficient permissions`], + 429: ['temporarily_unavailable', `${context}: rate limited, try again later`] + } + const [code, desc] = codeMap[status] || ['invalid_grant', `${context}: request failed`] + throw new OAuthError(code, desc, status) +} + const PKCE_CHARSET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~' const CODE_VERIFIER_LENGTH = 96 @@ -101,8 +122,8 @@ export async function getAuthToken(params: { }) if (!resp.ok) { - const text = await resp.text() - throw new Error(`Token exchange failed: ${text}`) + console.error(`Token exchange failed: ${resp.status}`, await resp.text()) + throwUpstreamError(resp.status, 'Token exchange failed') } return AuthorizationToken.parse(await resp.json()) @@ -132,8 +153,8 @@ export async function refreshAuthToken(params: { }) if (!resp.ok) { - const text = await resp.text() - throw new Error(`Token refresh failed: ${text}`) + console.error(`Token refresh failed: ${resp.status}`, await resp.text()) + throwUpstreamError(resp.status, 'Token refresh failed') } return AuthorizationToken.parse(await resp.json()) diff --git a/src/auth/oauth-handler.ts b/src/auth/oauth-handler.ts index 1718506..e5c0d4e 100644 --- a/src/auth/oauth-handler.ts +++ b/src/auth/oauth-handler.ts @@ -56,6 +56,25 @@ export async function getUserAndAccounts(accessToken: string): Promise<{ fetch(`${env.CLOUDFLARE_API_BASE}/accounts`, { headers }) ]) + // Check for upstream errors before parsing + if (!userResp.ok && !accountsResp.ok) { + const status = userResp.status + console.error(`Cloudflare API error: user=${userResp.status}, accounts=${accountsResp.status}`) + if (status >= 500) { + throw new OAuthError('server_error', 'Cloudflare API is temporarily unavailable', 502) + } + if (status === 401) { + throw new OAuthError('invalid_token', 'Access token is invalid or expired', 401) + } + if (status === 403) { + throw new OAuthError('insufficient_scope', 'Insufficient permissions', 403) + } + if (status === 429) { + throw new OAuthError('temporarily_unavailable', 'Rate limited, try again later', 429) + } + throw new OAuthError('invalid_token', 'Failed to verify token', status) + } + const userData = (await userResp.json()) as CloudflareApiResponse<{ id: string; email: string }> const accountsData = (await accountsResp.json()) as CloudflareApiResponse< Array<{ id: string; name: string }> @@ -78,7 +97,7 @@ export async function getUserAndAccounts(accessToken: string): Promise<{ return { user: null, accounts } } - throw new Error('Failed to fetch user or accounts') + throw new OAuthError('invalid_token', 'Failed to verify token: no user or account information', 401) } /** @@ -225,11 +244,13 @@ export function createAuthHandlers() { }) } catch (e) { if (e instanceof OAuthError) return e.toHtmlResponse() - console.error('Authorize error:', e) + const errorId = crypto.randomUUID() + console.error(`Authorize error [${errorId}]:`, e) return renderErrorPage( 'Server Error', 'An unexpected error occurred. Please try again.', - e instanceof Error ? e.message : undefined + `Error ID: ${errorId}`, + 500 ) } }) @@ -278,11 +299,13 @@ export function createAuthHandlers() { return redirectResponse } catch (e) { if (e instanceof OAuthError) return e.toHtmlResponse() - console.error('Authorize POST error:', e) + const errorId = crypto.randomUUID() + console.error(`Authorize POST error [${errorId}]:`, e) return renderErrorPage( 'Server Error', 'An unexpected error occurred. Please try again.', - e instanceof Error ? e.message : undefined + `Error ID: ${errorId}`, + 500 ) } }) @@ -354,11 +377,13 @@ export function createAuthHandlers() { }) } catch (e) { if (e instanceof OAuthError) return e.toHtmlResponse() - console.error('Callback error:', e) + const errorId = crypto.randomUUID() + console.error(`Callback error [${errorId}]:`, e) return renderErrorPage( 'Server Error', 'An unexpected error occurred during authorization.', - e instanceof Error ? e.message : undefined + `Error ID: ${errorId}`, + 500 ) } }) diff --git a/src/auth/workers-oauth-utils.ts b/src/auth/workers-oauth-utils.ts index c6ff380..52714d8 100644 --- a/src/auth/workers-oauth-utils.ts +++ b/src/auth/workers-oauth-utils.ts @@ -36,15 +36,19 @@ export class OAuthError extends Error { toHtmlResponse(): Response { const titles: Record = { invalid_request: 'Invalid Request', + invalid_grant: 'Invalid Grant', + invalid_client: 'Invalid Client', + invalid_token: 'Invalid Token', unauthorized_client: 'Unauthorized Client', access_denied: 'Access Denied', unsupported_response_type: 'Unsupported Response Type', invalid_scope: 'Invalid Scope', + insufficient_scope: 'Insufficient Scope', server_error: 'Server Error', temporarily_unavailable: 'Temporarily Unavailable' } const title = titles[this.code] || 'Authorization Error' - return renderErrorPage(title, this.description, `Error code: ${this.code}`) + return renderErrorPage(title, this.description, `Error code: ${this.code}`, this.statusCode) } } @@ -803,7 +807,7 @@ export async function parseRedirectApproval( cookieSecret: string ): Promise { if (request.method !== 'POST') { - throw new Error('Invalid request method') + throw new OAuthError('invalid_request', 'Invalid request method', 405) } const formData = await request.formData() @@ -811,7 +815,7 @@ export async function parseRedirectApproval( // Validate CSRF token const tokenFromForm = formData.get('csrf_token') if (!tokenFromForm || typeof tokenFromForm !== 'string') { - throw new Error('Missing CSRF token') + throw new OAuthError('invalid_request', 'Missing CSRF token') } const cookieHeader = request.headers.get('Cookie') || '' @@ -820,17 +824,17 @@ export async function parseRedirectApproval( const tokenFromCookie = csrfCookie ? csrfCookie.substring(CSRF_COOKIE.length + 1) : null if (!tokenFromCookie || tokenFromForm !== tokenFromCookie) { - throw new Error('CSRF token mismatch') + throw new OAuthError('access_denied', 'CSRF token mismatch', 403) } const encodedState = formData.get('state') if (!encodedState || typeof encodedState !== 'string') { - throw new Error('Missing state') + throw new OAuthError('invalid_request', 'Missing state') } const state = JSON.parse(atob(encodedState)) if (!state.oauthReqInfo || !state.oauthReqInfo.clientId) { - throw new Error('Invalid state data') + throw new OAuthError('invalid_request', 'Invalid state data') } // Extract selected scopes (from checkboxes) and template @@ -917,7 +921,7 @@ const StoredOAuthStateSchema = z.object({ /** * Renders a styled error page matching Cloudflare's design system */ -export function renderErrorPage(title: string, message: string, details?: string): Response { +export function renderErrorPage(title: string, message: string, details?: string, status = 400): Response { const htmlContent = ` @@ -1075,7 +1079,7 @@ export function renderErrorPage(title: string, message: string, details?: string ` return new Response(htmlContent, { - status: 400, + status, headers: { 'Content-Security-Policy': "frame-ancestors 'none'", 'Content-Type': 'text/html; charset=utf-8', From bc0cbfb789993e75b914eb1ea9382bf1649ecab1 Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Mon, 23 Feb 2026 14:40:36 +0000 Subject: [PATCH 2/2] fix: formatting issues in oauth-handler and workers-oauth-utils --- src/auth/oauth-handler.ts | 6 +++++- src/auth/workers-oauth-utils.ts | 7 ++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/auth/oauth-handler.ts b/src/auth/oauth-handler.ts index e5c0d4e..16d489b 100644 --- a/src/auth/oauth-handler.ts +++ b/src/auth/oauth-handler.ts @@ -97,7 +97,11 @@ export async function getUserAndAccounts(accessToken: string): Promise<{ return { user: null, accounts } } - throw new OAuthError('invalid_token', 'Failed to verify token: no user or account information', 401) + throw new OAuthError( + 'invalid_token', + 'Failed to verify token: no user or account information', + 401 + ) } /** diff --git a/src/auth/workers-oauth-utils.ts b/src/auth/workers-oauth-utils.ts index 52714d8..ef8f2e4 100644 --- a/src/auth/workers-oauth-utils.ts +++ b/src/auth/workers-oauth-utils.ts @@ -921,7 +921,12 @@ const StoredOAuthStateSchema = z.object({ /** * Renders a styled error page matching Cloudflare's design system */ -export function renderErrorPage(title: string, message: string, details?: string, status = 400): Response { +export function renderErrorPage( + title: string, + message: string, + details?: string, + status = 400 +): Response { const htmlContent = `