refactor(auth): simplify MCP consent screen

.changeset/simplify-mcp-consent.md

@@ -0,0 +1,5 @@
+---
+'@repo/mcp-common': patch
+---
+
+Simplify the required MCP client consent interstitial and align it with Cloudflare's Kumo-based OAuth experience. The page now focuses on the requesting client, redirect hostname, local-redirect warning, and complete configured scope set before continuing to Cloudflare authorization.

packages/mcp-common/src/cloudflare-oauth-handler.spec.ts

@@ -1,12 +1,13 @@
 import {
 	AuthorizationError,
+	CimdFetchError,
 	GrantType,
 	OAuthError as ProviderOAuthError,
 } from '@cloudflare/workers-oauth-provider'
 import { http, HttpResponse } from 'msw'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
-import { generatePKCECodes, getAuthorizationURL, refreshAuthToken } from './cloudflare-auth'
+import { refreshAuthToken } from './cloudflare-auth'
 import {
 	createAuthHandlers,
 	getUserAndAccounts,
@@ -27,8 +28,6 @@ vi.mock('./cloudflare-auth', () => ({
 }))
 
 const mockRefreshAuthToken = vi.mocked(refreshAuthToken)
-const mockGeneratePKCECodes = vi.mocked(generatePKCECodes)
-const mockGetAuthorizationURL = vi.mocked(getAuthorizationURL)
 
 beforeEach(() => {
 	vi.resetAllMocks()
@@ -593,9 +592,8 @@ describe('createAuthHandlers authorize route', () => {
 	function authorizeEnv(oauthProvider: Partial<OAuthHelpers>) {
 		return {
 			OAUTH_PROVIDER: oauthProvider as OAuthHelpers,
-			OAUTH_KV: {
-				put: vi.fn(async () => {}),
-			} as unknown as KVNamespace,
+			OAUTH_KV: undefined as unknown as KVNamespace,
+			MCP_COOKIE_ENCRYPTION_KEY: 'test-key',
 			CLOUDFLARE_CLIENT_ID: 'client',
 			CLOUDFLARE_CLIENT_SECRET: 'secret',
 		}
@@ -649,56 +647,24 @@ describe('createAuthHandlers authorize route', () => {
 		})
 	})
 
-	it('redirects directly to Cloudflare and requests every configured scope', async () => {
-		const scopes = {
-			'account:read': 'Read account details',
-			'workers:write': 'Manage Workers',
-		}
-		const oauthReqInfo = {
-			clientId: 'https://client.example.com/metadata.json',
-			redirectUri: 'https://client.example.com/callback',
-			responseType: 'code',
-			scope: [],
-			state: 'client-state',
-		}
-		const env = authorizeEnv({
-			async parseAuthRequest() {
-				return oauthReqInfo
-			},
-			lookupClient: vi.fn(),
-		})
-		mockGeneratePKCECodes.mockResolvedValue({
-			codeChallenge: 'challenge',
-			codeVerifier: 'verifier',
-		})
-		mockGetAuthorizationURL.mockResolvedValue({
-			authUrl: 'https://dash.cloudflare.com/oauth2/auth?scope=account%3Aread+workers%3Awrite',
-		})
-
-		const app = createAuthHandlers({ scopes, metrics })
+	it('returns a retryable 503 when client metadata resolution fails', async () => {
+		const clientId = 'https://client.example.com/metadata.json'
+		const app = createAuthHandlers({ scopes: {}, metrics })
 		const response = await app.fetch(
-			new Request('https://mcp.example.com/oauth/authorize?client_id=abc'),
-			env,
+			new Request(`https://mcp.example.com/oauth/authorize?client_id=${clientId}`),
+			authorizeEnv({
+				async parseAuthRequest() {
+					return { clientId } as Awaited<ReturnType<OAuthHelpers['parseAuthRequest']>>
+				},
+				async lookupClient() {
+					throw new CimdFetchError(clientId, new Error('HTTP 403'))
+				},
+			}),
 			executionCtx
 		)
 
-		expect(response.status).toBe(302)
-		expect(response.headers.get('location')).toBe(
-			'https://dash.cloudflare.com/oauth2/auth?scope=account%3Aread+workers%3Awrite'
-		)
-		expect(response.headers.get('set-cookie')).toContain('__Host-CONSENTED_STATE=')
-		expect(env.OAUTH_PROVIDER.lookupClient).not.toHaveBeenCalled()
-		expect(mockGetAuthorizationURL).toHaveBeenCalledWith(
-			expect.objectContaining({
-				client_id: 'client',
-				scopes,
-				codeChallenge: 'challenge',
-				state: expect.objectContaining({
-					...oauthReqInfo,
-					scope: Object.keys(scopes),
-					state: expect.any(String),
-				}),
-			})
-		)
+		expect(response.status).toBe(503)
+		expect(response.headers.get('retry-after')).toBe('30')
+		await expect(response.json()).resolves.toMatchObject({ error: 'temporarily_unavailable' })
 	})
 })

packages/mcp-common/src/cloudflare-oauth-handler.ts

@@ -1,5 +1,6 @@
 import {
 	AuthorizationError,
+	CimdFetchError,
 	GrantType,
 	OAuthError as ProviderOAuthError,
 } from '@cloudflare/workers-oauth-provider'
@@ -21,8 +22,12 @@ import { useSentry } from './sentry'
 import { V4Schema } from './v4-api'
 import {
 	bindStateToSession,
+	clientIdAlreadyApproved,
 	createOAuthState,
+	generateCSRFProtection,
 	OAuthError,
+	parseRedirectApproval,
+	renderApprovalDialog,
 	validateOAuthState,
 } from './workers-oauth-utils'
 
@@ -61,8 +66,11 @@ type AuthContext = {
 	Bindings: {
 		OAUTH_PROVIDER: OAuthHelpers
 		OAUTH_KV: KVNamespace
+		MCP_COOKIE_ENCRYPTION_KEY: string
 		CLOUDFLARE_CLIENT_ID: string
 		CLOUDFLARE_CLIENT_SECRET: string
+		MCP_SERVER_NAME?: string
+		MCP_SERVER_DESCRIPTION?: string
 	}
 } & BaseHonoContext
 
@@ -456,7 +464,7 @@ export function createAuthHandlers({
 	app.use(useSentry)
 
 	/**
-	 * GET /oauth/authorize - Redirect to Cloudflare's consent dialog
+	 * GET /oauth/authorize - Show consent dialog or redirect if approved
 	 */
 	app.get(`/oauth/authorize`, async (c) => {
 		try {
@@ -482,17 +490,126 @@ export function createAuthHandlers({
 				}
 				throw e
 			}
-			// Always request the server's complete configured scope set. Cloudflare's
-			// consent dialog is responsible for letting the user decline optional scopes.
+			// Request the complete server scope set; Cloudflare handles optional selection.
 			oauthReqInfo.scope = Object.keys(scopes)
 
+			// Check if client was previously approved (skip consent if so)
+			if (
+				await clientIdAlreadyApproved(
+					c.req.raw,
+					oauthReqInfo.clientId,
+					c.env.MCP_COOKIE_ENCRYPTION_KEY
+				)
+			) {
+				// Client already approved - create state and redirect immediately
+				const { codeChallenge, codeVerifier } = await generatePKCECodes()
+				const stateToken = await createOAuthState(oauthReqInfo, c.env.OAUTH_KV, codeVerifier)
+				const { setCookie: sessionCookie } = await bindStateToSession(stateToken)
+
+				return redirectToCloudflare(c, oauthReqInfo, stateToken, codeChallenge, scopes, {
+					'Set-Cookie': sessionCookie,
+				})
+			}
+
+			// Client not approved - show consent dialog
+			const { token: csrfToken, setCookie: csrfCookie } = generateCSRFProtection()
+
+			// Render approval dialog
+			const client = await c.env.OAUTH_PROVIDER.lookupClient(oauthReqInfo.clientId)
+			const cancelUri = new URL(oauthReqInfo.redirectUri)
+			cancelUri.searchParams.set('error', 'access_denied')
+			cancelUri.searchParams.set('error_description', 'The resource owner denied the request')
+			if (oauthReqInfo.state) cancelUri.searchParams.set('state', oauthReqInfo.state)
+
+			const response = renderApprovalDialog(c.req.raw, {
+				client,
+				serverName: c.env.MCP_SERVER_NAME || 'Cloudflare MCP Server',
+				redirectUri: oauthReqInfo.redirectUri,
+				cancelUri: cancelUri.href,
+				scopes,
+				state: { oauthReqInfo },
+				csrfToken,
+				setCookie: csrfCookie,
+			})
+
+			return response
+		} catch (e) {
+			c.var.sentry?.recordError(e)
+			let message: string | undefined
+			if (e instanceof Error) {
+				message = `${e.name}: ${e.message}`
+			} else if (typeof e === 'string') {
+				message = e
+			} else {
+				message = 'Unknown error'
+			}
+			metrics.logEvent(
+				new AuthUser({
+					errorMessage: `Authorize Error: ${message}`,
+				})
+			)
+			if (e instanceof CimdFetchError) {
+				return new OAuthError(
+					'temporarily_unavailable',
+					'Client metadata is temporarily unavailable. Please try again.',
+					503,
+					{ 'Retry-After': '30' }
+				).toResponse()
+			}
+			if (e instanceof OAuthError) {
+				return e.toResponse()
+			}
+			if (e instanceof McpError) {
+				return mcpErrorToOAuthResponse(e)
+			}
+			console.error(e)
+			return new OAuthError('server_error', 'Internal Error', 500).toResponse()
+		}
+	})
+
+	/**
+	 * POST /oauth/authorize - Handle consent form submission
+	 */
+	app.post(`/oauth/authorize`, async (c) => {
+		try {
+			// Validates CSRF token, extracts state, and generates approved client cookie
+			const { state, headers } = await parseRedirectApproval(
+				c.req.raw,
+				c.env.MCP_COOKIE_ENCRYPTION_KEY
+			)
+
+			if (!state.oauthReqInfo) {
+				return new OAuthError(
+					'invalid_request',
+					'Missing OAuth request info in state',
+					400
+				).toResponse()
+			}
+
+			const oauthReqInfo = state.oauthReqInfo as AuthRequest
+
+			// Create OAuth state in KV and bind to session
 			const { codeChallenge, codeVerifier } = await generatePKCECodes()
 			const stateToken = await createOAuthState(oauthReqInfo, c.env.OAUTH_KV, codeVerifier)
 			const { setCookie: sessionCookie } = await bindStateToSession(stateToken)
 
-			return redirectToCloudflare(c, oauthReqInfo, stateToken, codeChallenge, scopes, {
-				'Set-Cookie': sessionCookie,
-			})
+			// Build redirect response
+			const redirectResponse = await redirectToCloudflare(
+				c,
+				oauthReqInfo,
+				stateToken,
+				codeChallenge,
+				scopes
+			)
+
+			// Add both cookies: approved client cookie (if present) and session binding cookie
+			// Note: We must use append() for multiple Set-Cookie headers, not combine with commas
+			if (headers['Set-Cookie']) {
+				redirectResponse.headers.append('Set-Cookie', headers['Set-Cookie'])
+			}
+			redirectResponse.headers.append('Set-Cookie', sessionCookie)
+
+			return redirectResponse
 		} catch (e) {
 			c.var.sentry?.recordError(e)
 			let message: string | undefined
@@ -505,7 +622,7 @@ export function createAuthHandlers({
 			}
 			metrics.logEvent(
 				new AuthUser({
-					errorMessage: `Authorize Error: ${message}`,
+					errorMessage: `Authorize POST Error: ${message}`,
 				})
 			)
 			if (e instanceof OAuthError) {