diff --git a/demos/remote-mcp-github-oauth/src/github-handler.ts b/demos/remote-mcp-github-oauth/src/github-handler.ts index f7d880700..ff8928942 100644 --- a/demos/remote-mcp-github-oauth/src/github-handler.ts +++ b/demos/remote-mcp-github-oauth/src/github-handler.ts @@ -1,12 +1,16 @@ import { env } from "cloudflare:workers"; -import type { AuthRequest, OAuthHelpers } from "@cloudflare/workers-oauth-provider"; +import type { OAuthHelpers } from "@cloudflare/workers-oauth-provider"; import { Hono } from "hono"; import { Octokit } from "octokit"; import { fetchUpstreamAuthToken, getUpstreamAuthorizeUrl, type Props } from "./utils"; import { + bindStateToSession, clientIdAlreadyApproved, + createOAuthState, + generateCSRFProtection, parseRedirectApproval, renderApprovalDialog, + validateOAuthState, } from "./workers-oauth-utils"; const app = new Hono<{ Bindings: Env & { OAUTH_PROVIDER: OAuthHelpers } }>(); @@ -21,16 +25,23 @@ app.get("/authorize", async (c) => { if ( await clientIdAlreadyApproved(c.req.raw, oauthReqInfo.clientId, env.COOKIE_ENCRYPTION_KEY) ) { - return redirectToGithub(c.req.raw, oauthReqInfo); + // Skip approval dialog but still create secure state and bind to session + const stateToken = await createOAuthState(oauthReqInfo, c.env.OAUTH_KV); + const { setCookie: sessionBindingCookie } = await bindStateToSession(stateToken); + return redirectToGithub(c.req.raw, stateToken, { "Set-Cookie": sessionBindingCookie }); } + const { token: csrfToken, setCookie } = generateCSRFProtection(); + return renderApprovalDialog(c.req.raw, { client: await c.env.OAUTH_PROVIDER.lookupClient(clientId), + csrfToken, server: { description: "This is a demo MCP Remote Server using GitHub for authentication.", logo: "https://avatars.githubusercontent.com/u/314135?s=200&v=4", name: "Cloudflare GitHub MCP Server", // optional }, + setCookie, state: { oauthReqInfo }, // arbitrary data that flows through the form submission below }); }); @@ -42,12 +53,23 @@ app.post("/authorize", async (c) => { return c.text("Invalid request", 400); } - return redirectToGithub(c.req.raw, state.oauthReqInfo, headers); + // Create OAuth state and bind it to this user's session + const stateToken = await createOAuthState(state.oauthReqInfo, c.env.OAUTH_KV); + const { setCookie: sessionBindingCookie } = await bindStateToSession(stateToken); + + // Set both cookies: approved client list + session binding + const allHeaders = new Headers(); + for (const [key, value] of Object.entries(headers)) { + allHeaders.append(key, value); + } + allHeaders.append("Set-Cookie", sessionBindingCookie); + + return redirectToGithub(c.req.raw, stateToken, Object.fromEntries(allHeaders)); }); async function redirectToGithub( request: Request, - oauthReqInfo: AuthRequest, + stateToken: string, headers: Record = {}, ) { return new Response(null, { @@ -57,7 +79,7 @@ async function redirectToGithub( client_id: env.GITHUB_CLIENT_ID, redirect_uri: new URL("/callback", request.url).href, scope: "read:user", - state: btoa(JSON.stringify(oauthReqInfo)), + state: stateToken, upstream_url: "https://github.com/login/oauth/authorize", }), }, @@ -69,13 +91,22 @@ async function redirectToGithub( * OAuth Callback Endpoint * * This route handles the callback from GitHub after user authentication. - * It exchanges the temporary code for an access token, then stores some - * user metadata & the auth token as part of the 'props' on the token passed + * It validates the state parameter, exchanges the temporary code for an access token, + * then stores user metadata & the auth token as part of the 'props' on the token passed * down to the client. It ends by redirecting the client back to _its_ callback URL + * + * SECURITY: This endpoint validates that the state parameter from GitHub + * matches both: + * 1. A valid state token in KV (proves it was created by our server) + * 2. The __Host-CONSENTED_STATE cookie (proves THIS browser consented to it) + * + * This prevents CSRF attacks where an attacker's state token is injected + * into a victim's OAuth flow. */ app.get("/callback", async (c) => { - // Get the oathReqInfo out of KV - const oauthReqInfo = JSON.parse(atob(c.req.query("state") as string)) as AuthRequest; + // Validate OAuth state with session binding + // This checks both KV storage AND the session cookie + const { oauthReqInfo, clearCookie } = await validateOAuthState(c.req.raw, c.env.OAUTH_KV); if (!oauthReqInfo.clientId) { return c.text("Invalid state", 400); } @@ -111,7 +142,16 @@ app.get("/callback", async (c) => { userId: login, }); - return Response.redirect(redirectTo); + // Clear the session binding cookie (one-time use) by creating response with headers + const headers = new Headers({ Location: redirectTo }); + if (clearCookie) { + headers.set("Set-Cookie", clearCookie); + } + + return new Response(null, { + status: 302, + headers, + }); }); export { app as GitHubHandler }; diff --git a/demos/remote-mcp-github-oauth/src/workers-oauth-utils.ts b/demos/remote-mcp-github-oauth/src/workers-oauth-utils.ts index 8148550c9..620038ad4 100644 --- a/demos/remote-mcp-github-oauth/src/workers-oauth-utils.ts +++ b/demos/remote-mcp-github-oauth/src/workers-oauth-utils.ts @@ -1,44 +1,8 @@ -// workers-oauth-utils.ts +import type { AuthRequest, ClientInfo } from "@cloudflare/workers-oauth-provider"; -import type { AuthRequest, ClientInfo } from "@cloudflare/workers-oauth-provider"; // Adjust path if necessary - -const COOKIE_NAME = "mcp-approved-clients"; +const COOKIE_NAME = "__Host-MCP_APPROVED_CLIENTS"; const ONE_YEAR_IN_SECONDS = 31536000; -// --- Helper Functions --- - -/** - * Encodes arbitrary data to a URL-safe base64 string. - * @param data - The data to encode (will be stringified). - * @returns A URL-safe base64 encoded string. - */ -function _encodeState(data: any): string { - try { - const jsonString = JSON.stringify(data); - // Use btoa for simplicity, assuming Worker environment supports it well enough - // For complex binary data, a Buffer/Uint8Array approach might be better - return btoa(jsonString); - } catch (e) { - console.error("Error encoding state:", e); - throw new Error("Could not encode state"); - } -} - -/** - * Decodes a URL-safe base64 string back to its original data. - * @param encoded - The URL-safe base64 encoded string. - * @returns The original data. - */ -function decodeState(encoded: string): T { - try { - const jsonString = atob(encoded); - return JSON.parse(jsonString); - } catch (e) { - console.error("Error decoding state:", e); - throw new Error("Could not decode state"); - } -} - /** * Imports a secret key string for HMAC-SHA256 signing. * @param secret - The raw secret key string. @@ -89,13 +53,11 @@ async function verifySignature( ): Promise { const enc = new TextEncoder(); try { - // Convert hex signature back to ArrayBuffer const signatureBytes = new Uint8Array( signatureHex.match(/.{1,2}/g)!.map((byte) => Number.parseInt(byte, 16)), ); return await crypto.subtle.verify("HMAC", key, signatureBytes.buffer, enc.encode(data)); } catch (e) { - // Handle errors during hex parsing or verification console.error("Error verifying signature:", e); return false; } @@ -155,8 +117,6 @@ async function getApprovedClientsFromCookie( } } -// --- Exported Functions --- - /** * Checks if a given client ID has already been approved by the user, * based on a signed cookie. @@ -200,31 +160,13 @@ export interface ApprovalDialogOptions { */ state: Record; /** - * Name of the cookie to use for storing approvals - * @default "mcp_approved_clients" - */ - cookieName?: string; - /** - * Secret used to sign cookies for verification - * Can be a string or Uint8Array - * @default Built-in Uint8Array key - */ - cookieSecret?: string | Uint8Array; - /** - * Cookie domain - * @default current domain - */ - cookieDomain?: string; - /** - * Cookie path - * @default "/" + * CSRF token to include in the approval form */ - cookiePath?: string; + csrfToken: string; /** - * Cookie max age in seconds - * @default 30 days + * Set-Cookie header to include in the approval response */ - cookieMaxAge?: number; + setCookie: string; } /** @@ -237,35 +179,28 @@ export interface ApprovalDialogOptions { * @returns A Response containing the HTML approval dialog */ export function renderApprovalDialog(request: Request, options: ApprovalDialogOptions): Response { - const { client, server, state } = options; - - // Encode state for form submission + const { client, server, state, csrfToken, setCookie } = options; const encodedState = btoa(JSON.stringify(state)); - // Sanitize any untrusted content const serverName = sanitizeHtml(server.name); const clientName = client?.clientName ? sanitizeHtml(client.clientName) : "Unknown MCP Client"; const serverDescription = server.description ? sanitizeHtml(server.description) : ""; - // Safe URLs const logoUrl = server.logo ? sanitizeHtml(server.logo) : ""; const clientUri = client?.clientUri ? sanitizeHtml(client.clientUri) : ""; const policyUri = client?.policyUri ? sanitizeHtml(client.policyUri) : ""; const tosUri = client?.tosUri ? sanitizeHtml(client.tosUri) : ""; - // Client contacts const contacts = client?.contacts && client.contacts.length > 0 ? sanitizeHtml(client.contacts.join(", ")) : ""; - // Get redirect URIs const redirectUris = client?.redirectUris && client.redirectUris.length > 0 - ? client.redirectUris.map((uri) => sanitizeHtml(uri)) + ? client.redirectUris.map((uri) => sanitizeHtml(uri)).filter((uri) => uri !== "") : []; - // Generate HTML for the approval dialog const htmlContent = ` @@ -544,7 +479,8 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp
- + +
@@ -558,7 +494,10 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp return new Response(htmlContent, { headers: { + "Content-Security-Policy": "frame-ancestors 'none'", "Content-Type": "text/html; charset=utf-8", + "Set-Cookie": setCookie, + "X-Frame-Options": "DENY", }, }); } @@ -567,8 +506,8 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp * Result of parsing the approval form submission. */ export interface ParsedApprovalResult { - /** The original state object passed through the form. */ - state: any; + /** The original state object containing the OAuth request information. */ + state: { oauthReqInfo?: AuthRequest }; /** Headers to set on the redirect response, including the Set-Cookie header. */ headers: Record; } @@ -589,47 +528,44 @@ export async function parseRedirectApproval( if (request.method !== "POST") { throw new Error("Invalid request method. Expected POST."); } + + const formData = await request.formData(); - let state: any; - let clientId: string | undefined; + const tokenFromForm = formData.get("csrf_token"); + if (!tokenFromForm || typeof tokenFromForm !== "string") { + throw new Error("Missing CSRF token in form data"); + } - try { - const formData = await request.formData(); - const encodedState = formData.get("state"); + const cookieHeader = request.headers.get("Cookie") || ""; + const cookies = cookieHeader.split(";").map((c) => c.trim()); + const csrfCookie = cookies.find((c) => c.startsWith("__Host-CSRF_TOKEN=")); + const tokenFromCookie = csrfCookie ? csrfCookie.substring("__Host-CSRF_TOKEN=".length) : null; - if (typeof encodedState !== "string" || !encodedState) { - throw new Error("Missing or invalid 'state' in form data."); - } + if (!tokenFromCookie || tokenFromForm !== tokenFromCookie) { + throw new Error("CSRF token mismatch"); + } - state = decodeState<{ oauthReqInfo?: AuthRequest }>(encodedState); // Decode the state - clientId = state?.oauthReqInfo?.clientId; // Extract clientId from within the state + const encodedState = formData.get("state"); + if (!encodedState || typeof encodedState !== "string") { + throw new Error("Missing state in form data"); + } - if (!clientId) { - throw new Error("Could not extract clientId from state object."); - } - } catch (e) { - console.error("Error processing form submission:", e); - // Rethrow or handle as appropriate, maybe return a specific error response - throw new Error( - `Failed to parse approval form: ${e instanceof Error ? e.message : String(e)}`, - ); + const state = JSON.parse(atob(encodedState)); + if (!state.oauthReqInfo || !state.oauthReqInfo.clientId) { + throw new Error("Invalid state data"); } - // Get existing approved clients - const cookieHeader = request.headers.get("Cookie"); const existingApprovedClients = - (await getApprovedClientsFromCookie(cookieHeader, cookieSecret)) || []; - - // Add the newly approved client ID (avoid duplicates) - const updatedApprovedClients = Array.from(new Set([...existingApprovedClients, clientId])); + (await getApprovedClientsFromCookie(request.headers.get("Cookie"), cookieSecret)) || []; + const updatedApprovedClients = Array.from( + new Set([...existingApprovedClients, state.oauthReqInfo.clientId]), + ); - // Sign the updated list const payload = JSON.stringify(updatedApprovedClients); const key = await importKey(cookieSecret); const signature = await signData(key, payload); const newCookieValue = `${signature}.${btoa(payload)}`; // signature.base64(payload) - // Generate Set-Cookie header const headers: Record = { "Set-Cookie": `${COOKIE_NAME}=${newCookieValue}; HttpOnly; Secure; Path=/; SameSite=Lax; Max-Age=${ONE_YEAR_IN_SECONDS}`, }; @@ -637,6 +573,129 @@ export async function parseRedirectApproval( return { headers, state }; } +/** + * Result from bindStateToSession containing the cookie to set + */ +export interface BindStateResult { + /** + * Set-Cookie header value to bind the state to the user's session + */ + setCookie: string; +} + +/** + * Result from validateOAuthState containing the original OAuth request info and cookie to clear + */ +export interface ValidateStateResult { + /** + * The original OAuth request information that was stored with the state token + */ + oauthReqInfo: AuthRequest; + + /** + * Set-Cookie header value to clear the state cookie + */ + clearCookie: string; +} + +export function generateCSRFProtection(): { token: string; setCookie: string } { + const token = crypto.randomUUID(); + const setCookie = `__Host-CSRF_TOKEN=${token}; HttpOnly; Secure; Path=/; SameSite=Lax; Max-Age=600`; + return { token, setCookie }; +} + +export async function createOAuthState( + oauthReqInfo: AuthRequest, + kv: KVNamespace, +): Promise { + const stateToken = crypto.randomUUID(); + await kv.put(`oauth:state:${stateToken}`, JSON.stringify(oauthReqInfo), { + expirationTtl: 600, + }); + return stateToken; +} + +/** + * Binds an OAuth state token to the user's browser session using a secure cookie. + * + * @param stateToken - The state token to bind to the session + * @returns Object containing the Set-Cookie header to send to the client + */ +export async function bindStateToSession(stateToken: string): Promise { + const consentedStateCookieName = "__Host-CONSENTED_STATE"; + + // Hash the state token to provide defense-in-depth + const encoder = new TextEncoder(); + const data = encoder.encode(stateToken); + const hashBuffer = await crypto.subtle.digest("SHA-256", data); + const hashArray = Array.from(new Uint8Array(hashBuffer)); + const hashHex = hashArray.map((b) => b.toString(16).padStart(2, "0")).join(""); + + const setCookie = `${consentedStateCookieName}=${hashHex}; HttpOnly; Secure; Path=/; SameSite=Lax; Max-Age=600`; + + return { setCookie }; +} + +/** + * Validates OAuth state from the request, ensuring: + * 1. The state parameter exists in KV (proves it was created by our server) + * 2. The state hash matches the session cookie (proves this browser consented to it) + * + * This prevents attacks where an attacker's valid state token is injected into + * a victim's OAuth flow. + * + * @param request - The HTTP request containing state parameter and cookies + * @param kv - Cloudflare KV namespace for storing OAuth state data + * @returns Object containing the original OAuth request info and cookie to clear + * @throws If state is missing, mismatched, or expired + */ +export async function validateOAuthState( + request: Request, + kv: KVNamespace, +): Promise { + const consentedStateCookieName = "__Host-CONSENTED_STATE"; + const url = new URL(request.url); + const stateFromQuery = url.searchParams.get("state"); + + if (!stateFromQuery) { + throw new Error("Missing state parameter"); + } + + const storedDataJson = await kv.get(`oauth:state:${stateFromQuery}`); + if (!storedDataJson) { + throw new Error("Invalid or expired state"); + } + + const cookieHeader = request.headers.get("Cookie") || ""; + const cookies = cookieHeader.split(";").map((c) => c.trim()); + const consentedStateCookie = cookies.find((c) => c.startsWith(`${consentedStateCookieName}=`)); + const consentedStateHash = consentedStateCookie + ? consentedStateCookie.substring(consentedStateCookieName.length + 1) + : null; + + if (!consentedStateHash) { + throw new Error("Missing session binding cookie - authorization flow must be restarted"); + } + + const encoder = new TextEncoder(); + const data = encoder.encode(stateFromQuery); + const hashBuffer = await crypto.subtle.digest("SHA-256", data); + const hashArray = Array.from(new Uint8Array(hashBuffer)); + const stateHash = hashArray.map((b) => b.toString(16).padStart(2, "0")).join(""); + + if (stateHash !== consentedStateHash) { + throw new Error("State token does not match session - possible CSRF attack detected"); + } + + const oauthReqInfo = JSON.parse(storedDataJson) as AuthRequest; + + await kv.delete(`oauth:state:${stateFromQuery}`); + const clearCookie = `${consentedStateCookieName}=; HttpOnly; Secure; Path=/; SameSite=Lax; Max-Age=0`; + + return { oauthReqInfo, clearCookie }; +} + + /** * Sanitizes HTML content to prevent XSS attacks * @param unsafe - The unsafe string that might contain HTML