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..16d489b 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,11 @@ 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 +248,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 +303,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 +381,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..ef8f2e4 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,12 @@ 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 +1084,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',