Skip to content

Commit c86154f

Browse files
Maximo-GukMaximo-Guk
committed
Update cloudflare oauth handler 2
1 parent 314c3d5 commit c86154f

File tree

4 files changed

+78
-44
lines changed

4 files changed

+78
-44
lines changed

‎.changeset/real-doors-happen.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@repo/mcp-common': patch
3+
---
4+
5+
Update cloudflare oauth handler 2

‎packages/mcp-common/src/cloudflare-auth.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ interface PKCECodes {
1919
codeChallenge: string
2020
codeVerifier: string
2121
}
22-
async function generatePKCECodes(): Promise<PKCECodes> {
22+
export async function generatePKCECodes(): Promise<PKCECodes> {
2323
const output = new Uint32Array(RECOMMENDED_CODE_VERIFIER_LENGTH)
2424
crypto.getRandomValues(output)
2525
const codeVerifier = base64urlEncode(
@@ -80,23 +80,22 @@ export async function getAuthorizationURL({
8080
redirect_uri,
8181
state,
8282
scopes,
83+
codeChallenge,
8384
}: {
8485
client_id: string
8586
redirect_uri: string
8687
state: AuthRequest
8788
scopes: Record<string, string>
88-
}): Promise<{ authUrl: string; codeVerifier: string }> {
89-
const { codeChallenge, codeVerifier } = await generatePKCECodes()
90-
89+
codeChallenge: string
90+
}): Promise<{ authUrl: string }> {
9191
return {
9292
authUrl: generateAuthUrl({
9393
client_id,
9494
redirect_uri,
95-
state: btoa(JSON.stringify({ ...state, codeVerifier })),
95+
state: btoa(JSON.stringify(state)),
9696
code_challenge: codeChallenge,
9797
scopes,
9898
}),
99-
codeVerifier: codeVerifier,
10099
}
101100
}
102101

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

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import { Hono } from 'hono'
33
import { z } from 'zod'
44

55
import { AuthUser } from '../../mcp-observability/src'
6-
import { getAuthorizationURL, getAuthToken, refreshAuthToken } from './cloudflare-auth'
6+
import {
7+
generatePKCECodes,
8+
getAuthorizationURL,
9+
getAuthToken,
10+
refreshAuthToken,
11+
} from './cloudflare-auth'
712
import { McpError } from './mcp-error'
813
import { useSentry } from './sentry'
914
import { V4Schema } from './v4-api'
@@ -46,18 +51,6 @@ const AuthQuery = z.object({
4651
scope: z.string().describe('OAuth scopes granted'),
4752
})
4853

49-
// AuthRequest but with extra params that we use in our authentication logic
50-
const AuthRequestSchemaWithExtraParams = z.object({
51-
responseType: z.string(),
52-
clientId: z.string(),
53-
redirectUri: z.string(),
54-
scope: z.array(z.string()),
55-
state: z.string(),
56-
codeChallenge: z.string().optional(),
57-
codeChallengeMethod: z.string().optional(),
58-
codeVerifier: z.string(),
59-
})
60-
6154
type UserSchema = z.infer<typeof UserSchema>
6255
const UserSchema = z.object({
6356
id: z.string(),
@@ -202,35 +195,36 @@ export async function handleTokenExchangeCallback(
202195
*
203196
* Note: We pass the stateToken as a simple string in the URL.
204197
* The existing getAuthorizationURL function will wrap it with the oauthReqInfo
205-
* and add the PKCE codeVerifier before base64-encoding.
198+
* before base64-encoding.
206199
* On callback, we extract the stateToken, look up the original oauthReqInfo in KV.
207200
*/
208201
async function redirectToCloudflare(
209202
c: Context<AuthContext>,
210203
oauthReqInfo: AuthRequest,
211204
stateToken: string,
205+
codeChallenge: string,
212206
scopes: Record<string, string>,
213207
additionalHeaders: Record<string, string> = {}
214208
): Promise<Response> {
215209
// Create a modified oauthReqInfo that includes our stateToken
216-
// getAuthorizationURL will add the codeVerifier and base64 encode everything
217210
const stateWithToken: AuthRequest = {
218211
...oauthReqInfo,
219-
state: stateToken, // Embed our KV state token
212+
state: stateToken, // embed our KV state token
220213
}
221214

222-
const authUrl = await getAuthorizationURL({
215+
const { authUrl } = await getAuthorizationURL({
223216
client_id: c.env.CLOUDFLARE_CLIENT_ID,
224217
redirect_uri: new URL('/oauth/callback', c.req.url).href,
225218
state: stateWithToken,
226219
scopes,
220+
codeChallenge,
227221
})
228222

229223
return new Response(null, {
230224
status: 302,
231225
headers: {
232226
...additionalHeaders,
233-
Location: authUrl.authUrl,
227+
Location: authUrl,
234228
},
235229
})
236230
}
@@ -273,10 +267,11 @@ export function createAuthHandlers({
273267
)
274268
) {
275269
// Client already approved - create state and redirect immediately
276-
const stateToken = await createOAuthState(oauthReqInfo, c.env.OAUTH_KV)
270+
const { codeChallenge, codeVerifier } = await generatePKCECodes()
271+
const stateToken = await createOAuthState(oauthReqInfo, c.env.OAUTH_KV, codeVerifier)
277272
const { setCookie: sessionCookie } = await bindStateToSession(stateToken)
278273

279-
return redirectToCloudflare(c, oauthReqInfo, stateToken, scopes, {
274+
return redirectToCloudflare(c, oauthReqInfo, stateToken, codeChallenge, scopes, {
280275
'Set-Cookie': sessionCookie,
281276
})
282277
}
@@ -349,11 +344,18 @@ export function createAuthHandlers({
349344
const oauthReqInfo = state.oauthReqInfo as AuthRequest
350345

351346
// Create OAuth state in KV and bind to session
352-
const stateToken = await createOAuthState(oauthReqInfo, c.env.OAUTH_KV)
347+
const { codeChallenge, codeVerifier } = await generatePKCECodes()
348+
const stateToken = await createOAuthState(oauthReqInfo, c.env.OAUTH_KV, codeVerifier)
353349
const { setCookie: sessionCookie } = await bindStateToSession(stateToken)
354350

355351
// Build redirect response
356-
const redirectResponse = await redirectToCloudflare(c, oauthReqInfo, stateToken, scopes)
352+
const redirectResponse = await redirectToCloudflare(
353+
c,
354+
oauthReqInfo,
355+
stateToken,
356+
codeChallenge,
357+
scopes
358+
)
357359

358360
// Add both cookies: approved client cookie (if present) and session binding cookie
359361
// Note: We must use append() for multiple Set-Cookie headers, not combine with commas
@@ -391,27 +393,21 @@ export function createAuthHandlers({
391393
*/
392394
app.get(`/oauth/callback`, zValidator('query', AuthQuery), async (c) => {
393395
try {
394-
const { state: stateParam, code } = c.req.valid('query')
396+
const { code } = c.req.valid('query')
395397

396398
// Validate state using dual validation (KV + session cookie)
397-
const { oauthReqInfo, clearCookie } = await validateOAuthState(c.req.raw, c.env.OAUTH_KV)
399+
const { oauthReqInfo, codeVerifier, clearCookie } = await validateOAuthState(
400+
c.req.raw,
401+
c.env.OAUTH_KV
402+
)
398403

399404
if (!oauthReqInfo.clientId) {
400405
return new OAuthError('invalid_request', 'Invalid OAuth request info', 400).toResponse()
401406
}
402407

403-
// Parse the state parameter to extract the encoded data
404-
const decodedState = AuthRequestSchemaWithExtraParams.parse(JSON.parse(atob(stateParam)))
405-
406-
// Extract code verifier for PKCE validation
407-
const codeVerifier = decodedState.codeVerifier
408-
if (!codeVerifier) {
409-
return new OAuthError('invalid_request', 'Missing code verifier', 400).toResponse()
410-
}
411-
412408
// Exchange code for tokens and get user details
413409
const [{ accessToken, refreshToken, user, accounts }] = await Promise.all([
414-
getTokenAndUserDetails(c, code, codeVerifier),
410+
getTokenAndUserDetails(c, code, codeVerifier), // use codeVerifier from KV
415411
c.env.OAUTH_PROVIDER.createClient({
416412
clientId: oauthReqInfo.clientId,
417413
tokenEndpointAuthMethod: 'none',

‎packages/mcp-common/src/workers-oauth-utils.ts‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { z } from 'zod'
2+
13
import type { AuthRequest, ClientInfo } from '@cloudflare/workers-oauth-provider'
24

35
const COOKIE_NAME = '__Host-MCP_APPROVED_CLIENTS'
@@ -615,6 +617,11 @@ export interface ValidateStateResult {
615617
*/
616618
oauthReqInfo: AuthRequest
617619

620+
/**
621+
* The PKCE code verifier retrieved from server-side storage (never transmitted to client)
622+
*/
623+
codeVerifier: string
624+
618625
/**
619626
* Set-Cookie header value to clear the state cookie
620627
*/
@@ -629,10 +636,16 @@ export function generateCSRFProtection(): { token: string; setCookie: string } {
629636

630637
export async function createOAuthState(
631638
oauthReqInfo: AuthRequest,
632-
kv: KVNamespace
639+
kv: KVNamespace,
640+
codeVerifier: string
633641
): Promise<string> {
634642
const stateToken = crypto.randomUUID()
635-
await kv.put(`oauth:state:${stateToken}`, JSON.stringify(oauthReqInfo), {
643+
const stateData = { oauthReqInfo, codeVerifier } satisfies {
644+
oauthReqInfo: AuthRequest
645+
codeVerifier: string
646+
}
647+
648+
await kv.put(`oauth:state:${stateToken}`, JSON.stringify(stateData), {
636649
expirationTtl: 600,
637650
})
638651
return stateToken
@@ -722,12 +735,33 @@ export async function validateOAuthState(
722735
throw new Error('State token does not match session - possible CSRF attack detected')
723736
}
724737

725-
const oauthReqInfo = JSON.parse(storedDataJson) as AuthRequest
738+
// Parse and validate stored OAuth state data
739+
const StoredOAuthStateSchema = z.object({
740+
oauthReqInfo: z
741+
.object({
742+
clientId: z.string(),
743+
scope: z.array(z.string()),
744+
state: z.string(),
745+
responseType: z.string(),
746+
redirectUri: z.string(),
747+
})
748+
.passthrough(), // preserve any other fields from oauth-provider
749+
codeVerifier: z.string().min(1), // Our code verifier for Cloudflare OAuth
750+
})
751+
752+
const parseResult = StoredOAuthStateSchema.safeParse(JSON.parse(storedDataJson))
753+
if (!parseResult.success) {
754+
throw new Error('Invalid OAuth state data format - PKCE security violation')
755+
}
726756

727757
await kv.delete(`oauth:state:${stateToken}`)
728758
const clearCookie = `${consentedStateCookieName}=; HttpOnly; Secure; Path=/; SameSite=Lax; Max-Age=0`
729759

730-
return { oauthReqInfo, clearCookie }
760+
return {
761+
oauthReqInfo: parseResult.data.oauthReqInfo,
762+
codeVerifier: parseResult.data.codeVerifier,
763+
clearCookie,
764+
}
731765
}
732766

733767
/**

0 commit comments

Comments
 (0)