Skip to content

Commit 019abcf

Browse files
authored
fix: use opaque state parameter in OAuth authorization flow (#41)
* fix: use opaque state parameter in OAuth authorization flow Replace base64-encoded AuthRequest in the OAuth state parameter with an opaque UUID token. The full AuthRequest is already stored in KV via createOAuthState() — embedding it in the URL was redundant and caused authorization URLs to exceed Cloudflare's size limits when combined with CIMD client IDs or many scopes. Security is unchanged: state is still validated via KV lookup + SHA-256 session cookie binding + single-use deletion. * test: assert opaque OAuth state token in e2e + unit coverage Update OAuth tests for the opaque-state-token contract: - cloudflare-auth.test.ts: getAuthorizationURL now takes stateToken and passes it through verbatim (no base64 AuthRequest). - oauth-routes.test.ts full-flow: assert the state forwarded to Cloudflare is opaque, not a base64-encoded AuthRequest. - oauth-routes.test.ts reject case: use a plain opaque token now that state is the KV lookup key directly. * test: harden opaque OAuth state flow * fix: accept in-flight legacy OAuth state
1 parent 13023f8 commit 019abcf

5 files changed

Lines changed: 220 additions & 96 deletions

File tree

‎src/auth/cloudflare-auth.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import { z } from 'zod'
22

3-
import type { AuthRequest } from '@cloudflare/workers-oauth-provider'
4-
53
import { OAuthError } from './workers-oauth-utils'
64

75
/**
@@ -71,7 +69,7 @@ export async function generatePKCECodes(): Promise<PKCECodes> {
7169
export async function getAuthorizationURL(params: {
7270
client_id: string
7371
redirect_uri: string
74-
state: AuthRequest
72+
stateToken: string
7573
scopes: string[]
7674
codeChallenge: string
7775
oauthDomain: string
@@ -80,7 +78,7 @@ export async function getAuthorizationURL(params: {
8078
response_type: 'code',
8179
client_id: params.client_id,
8280
redirect_uri: params.redirect_uri,
83-
state: btoa(JSON.stringify(params.state)),
81+
state: params.stateToken,
8482
code_challenge: params.codeChallenge,
8583
code_challenge_method: 'S256',
8684
scope: params.scopes.join(' ')

‎src/auth/oauth-handler.ts‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -558,21 +558,15 @@ export async function handleTokenExchangeCallback(
558558
*/
559559
async function redirectToCloudflare(
560560
requestUrl: string,
561-
oauthReqInfo: AuthRequest,
562561
stateToken: string,
563562
codeChallenge: string,
564563
scopes: string[],
565564
additionalHeaders: Record<string, string> = {}
566565
): Promise<Response> {
567-
const stateWithToken: AuthRequest = {
568-
...oauthReqInfo,
569-
state: stateToken
570-
}
571-
572566
const { authUrl } = await getAuthorizationURL({
573567
client_id: env.CLOUDFLARE_CLIENT_ID,
574568
redirect_uri: new URL('/oauth/callback', requestUrl).href,
575-
state: stateWithToken,
569+
stateToken,
576570
scopes,
577571
codeChallenge,
578572
oauthDomain: env.CLOUDFLARE_OAUTH_DOMAIN
@@ -617,16 +611,9 @@ export function createAuthHandlers() {
617611
const stateToken = await createOAuthState(oauthReqInfo, env.OAUTH_KV, codeVerifier)
618612
const { setCookie: sessionCookie } = await bindStateToSession(stateToken)
619613

620-
return redirectToCloudflare(
621-
c.req.url,
622-
oauthReqInfo,
623-
stateToken,
624-
codeChallenge,
625-
defaultScopes,
626-
{
627-
'Set-Cookie': sessionCookie
628-
}
629-
)
614+
return redirectToCloudflare(c.req.url, stateToken, codeChallenge, defaultScopes, {
615+
'Set-Cookie': sessionCookie
616+
})
630617
}
631618

632619
// Client not approved - show consent dialog with scope selection
@@ -691,7 +678,6 @@ export function createAuthHandlers() {
691678

692679
const redirectResponse = await redirectToCloudflare(
693680
c.req.url,
694-
oauthReqInfo,
695681
stateToken,
696682
codeChallenge,
697683
scopesToRequest

‎src/auth/workers-oauth-utils.ts‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,36 @@ const APPROVED_CLIENTS_COOKIE = '__Host-MCP_APPROVED_CLIENTS'
1010
const CSRF_COOKIE = '__Host-CSRF_TOKEN'
1111
const STATE_COOKIE = '__Host-CONSENTED_STATE'
1212
const ONE_YEAR_IN_SECONDS = 31536000
13+
const OAuthStateToken = z.uuid()
14+
const LegacyOAuthState = z.object({ state: OAuthStateToken }).passthrough()
15+
const MAX_LEGACY_OAUTH_STATE_LENGTH = 32_768
16+
17+
function encodeBase64Utf8(value: string): string {
18+
const bytes = new TextEncoder().encode(value)
19+
let binary = ''
20+
for (const byte of bytes) binary += String.fromCharCode(byte)
21+
return btoa(binary)
22+
}
23+
24+
function decodeBase64Utf8(value: string): string {
25+
const binary = atob(value)
26+
return new TextDecoder().decode(Uint8Array.from(binary, (char) => char.charCodeAt(0)))
27+
}
28+
29+
function parseOAuthStateToken(state: string): string | undefined {
30+
const current = OAuthStateToken.safeParse(state)
31+
if (current.success) return current.data
32+
33+
// TODO: Remove this legacy base64-JSON reader after all OAuth flows started
34+
// before the opaque-state deployment have exceeded the 600-second KV TTL.
35+
if (state.length > MAX_LEGACY_OAUTH_STATE_LENGTH) return undefined
36+
try {
37+
const legacy = LegacyOAuthState.safeParse(JSON.parse(atob(state)))
38+
return legacy.success ? legacy.data.state : undefined
39+
} catch {
40+
return undefined
41+
}
42+
}
1343

1444
/**
1545
* OAuth error class for handling OAuth-specific errors
@@ -449,7 +479,7 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp
449479
requiredScopes = []
450480
} = options
451481

452-
const encodedState = btoa(JSON.stringify(state))
482+
const encodedState = encodeBase64Utf8(JSON.stringify(state))
453483
const clientName = client?.clientName ? sanitizeHtml(client.clientName) : 'Unknown MCP Client'
454484
const requiredSet = new Set(requiredScopes)
455485
const categories = groupScopesByCategory(allScopes, requiredSet)
@@ -1469,7 +1499,7 @@ export async function parseRedirectApproval(
14691499
throw new OAuthError('invalid_request', 'Missing state')
14701500
}
14711501

1472-
const state = JSON.parse(atob(encodedState))
1502+
const state = JSON.parse(decodeBase64Utf8(encodedState))
14731503
if (!state.oauthReqInfo || !state.oauthReqInfo.clientId) {
14741504
throw new OAuthError('invalid_request', 'Invalid state data')
14751505
}
@@ -1754,16 +1784,9 @@ export async function validateOAuthState(
17541784
throw new OAuthError('invalid_request', 'Missing state parameter')
17551785
}
17561786

1757-
// Decode state to extract embedded stateToken
1758-
let stateToken: string
1759-
try {
1760-
const decodedState = JSON.parse(atob(stateFromQuery))
1761-
stateToken = decodedState.state
1762-
if (!stateToken) {
1763-
throw new Error('State token not found')
1764-
}
1765-
} catch {
1766-
throw new OAuthError('invalid_request', 'Failed to decode state')
1787+
const stateToken = parseOAuthStateToken(stateFromQuery)
1788+
if (!stateToken) {
1789+
throw new OAuthError('invalid_request', 'Invalid state parameter')
17671790
}
17681791

17691792
// Validate state exists in KV

‎tests/auth/cloudflare-auth.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,12 @@ describe('generatePKCECodes', () => {
9292
})
9393

9494
describe('getAuthorizationURL', () => {
95-
it('builds the Cloudflare /oauth2/auth URL with S256 PKCE and encoded state', async () => {
96-
const state = { clientId: 'mcp-client', redirectUri: 'https://app/cb' } as never
95+
it('builds the Cloudflare /oauth2/auth URL with S256 PKCE and an opaque state token', async () => {
96+
const stateToken = 'b9f1c0de-1234-4abc-9def-0123456789ab'
9797
const { authUrl } = await getAuthorizationURL({
9898
client_id: 'client-id',
9999
redirect_uri: 'https://mcp.example.com/oauth/callback',
100-
state,
100+
stateToken,
101101
scopes: ['user:read', 'account:read'],
102102
codeChallenge: 'challenge-123',
103103
oauthDomain: OAUTH_DOMAIN
@@ -111,8 +111,8 @@ describe('getAuthorizationURL', () => {
111111
expect(url.searchParams.get('code_challenge')).toBe('challenge-123')
112112
expect(url.searchParams.get('code_challenge_method')).toBe('S256')
113113
expect(url.searchParams.get('scope')).toBe('user:read account:read')
114-
// State is base64-encoded JSON of the AuthRequest.
115-
expect(JSON.parse(atob(url.searchParams.get('state')!))).toEqual(state)
114+
// State is the opaque KV lookup token, passed through verbatim (RFC 6749 §10.12).
115+
expect(url.searchParams.get('state')).toBe(stateToken)
116116
})
117117
})
118118

0 commit comments

Comments
 (0)