Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"seed:prod": "tsx scripts/seed-r2.ts production"
},
"dependencies": {
"@cloudflare/workers-oauth-provider": "^1.2.0",
"@cloudflare/workers-oauth-provider": "^1.2.1",
"@modelcontextprotocol/server": "2.0.0",
"hono": "^4.13.5",
"zod": "^4.3.5"
Expand Down
28 changes: 2 additions & 26 deletions src/auth/oauth-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
} from './scopes'
import { AuthProps as AuthPropsSchema, AUTH_PROPS_VERSION, type AuthProps } from './types'
import {
isAllowedOAuthRedirectUri,
parseRedirectApproval,
renderApprovalDialog,
renderErrorPage,
Expand Down Expand Up @@ -163,13 +162,6 @@ function cimdCallbackFailureResponse(): Response {
).toHtmlResponse()
}

function invalidRedirectUriResponse(): Response {
return new OAuthError(
'invalid_request',
'Redirect URI must use HTTPS or a local loopback address'
).toHtmlResponse()
}

/**
* Create OAuth route handlers using patterns from workers-oauth-provider
*/
Expand All @@ -184,7 +176,8 @@ export function createAuthHandlers() {
oauthReqInfo = await env.OAUTH_PROVIDER.parseAuthRequest(c.req.raw)
} catch (error) {
if (error instanceof AuthorizationError) {
if (!error.redirectUri || !isAllowedOAuthRedirectUri(error.redirectUri)) {
// workers-oauth-provider sets redirectUri only when the error may be sent there.
if (!error.redirectUri) {
return new OAuthError(error.code, error.description).toHtmlResponse()
}
const redirect = new URL(error.redirectUri)
Expand All @@ -202,9 +195,6 @@ export function createAuthHandlers() {
}
throw error
}
if (!isAllowedOAuthRedirectUri(oauthReqInfo.redirectUri)) {
return invalidRedirectUriResponse()
}
const defaultScopes = [...SCOPE_TEMPLATES[DEFAULT_TEMPLATE].scopes]
const requestedScopes = oauthReqInfo.scope ?? []
const unknownScopes = requestedScopes.filter((scope) => !ALLOWED_SCOPES.has(scope))
Expand Down Expand Up @@ -275,10 +265,6 @@ export function createAuthHandlers() {
const approved = await env.OAUTH_PROVIDER.approveConsent(c.req.raw, handle, {
scope: scopesToRequest
})
if (!isAllowedOAuthRedirectUri(approved.request.redirectUri)) {
return invalidRedirectUriResponse()
}

// Create the upstream state only now, after consent, bound to this browser.
const { codeChallenge, codeVerifier } = await generatePKCECodes()
const upstream = await env.OAUTH_PROVIDER.beginUpstream(approved.request, {
Expand Down Expand Up @@ -324,12 +310,6 @@ export function createAuthHandlers() {
headers
} = await env.OAUTH_PROVIDER.finishUpstream<{ codeVerifier: string }>(c.req.raw)

if (!isAllowedOAuthRedirectUri(oauthReqInfo.redirectUri)) {
const response = invalidRedirectUriResponse()
for (const cookie of headers.getSetCookie()) response.headers.append('Set-Cookie', cookie)
return response
}

// The user declined (or sign-in failed) at Cloudflare: tell the MCP client.
if (c.req.query('error')) {
const redirect = new URL(oauthReqInfo.redirectUri)
Expand Down Expand Up @@ -381,10 +361,6 @@ export function createAuthHandlers() {
} satisfies AuthProps
})

if (!isAllowedOAuthRedirectUri(redirectTo)) {
throw new OAuthError('server_error', 'Authorization produced an unsafe redirect URI')
}

metrics.logEvent(new AuthUser({ userId: identity.user.id }))

headers.set('Location', redirectTo)
Expand Down
30 changes: 0 additions & 30 deletions src/auth/workers-oauth-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,6 @@ function renderDisplayUrl(
}
}

function isLoopbackHostname(hostname: string): boolean {
const normalized = hostname.toLowerCase()
if (normalized === 'localhost' || normalized === '::1' || normalized === '[::1]') return true

const octets = normalized.split('.')
return (
octets.length === 4 &&
octets[0] === '127' &&
octets.every((octet) => /^\d{1,3}$/.test(octet) && Number(octet) <= 255)
)
}

/**
* MCP requires authorization redirects to use HTTPS, except for loopback
* callbacks used by native clients. Reject URL features that make the
* destination ambiguous or are forbidden for OAuth redirect endpoints.
*/
export function isAllowedOAuthRedirectUri(value: string): boolean {
if (value !== value.trim()) return false

try {
const url = new URL(value)
if (!url.hostname || url.username || url.password || url.hash) return false
if (url.protocol === 'https:') return true
return url.protocol === 'http:' && isLoopbackHostname(url.hostname)
} catch {
return false
}
}

/**
* Kumo's stacked Cloudflare logo with the current brand cloud colours. The
* wordmark uses `currentColor`, so it follows the text colour in dark mode.
Expand Down
27 changes: 0 additions & 27 deletions tests/auth/approval-dialog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { describe, expect, it } from 'vitest'

import { REQUIRED_SCOPES, SCOPE_DEFINITIONS, SCOPE_TEMPLATES } from '../../src/auth/scopes'
import {
isAllowedOAuthRedirectUri,
renderApprovalDialog,
renderErrorPage,
type ApprovalDialogOptions
Expand Down Expand Up @@ -171,29 +170,3 @@ describe('OAuth approval dialog templates', () => {
expect(body).toContain('const INITIAL_TEMPLATE = "read-only";')
})
})

describe('OAuth redirect URI policy', () => {
it.each([
'https://client.example/callback',
'https://client.example:8443/callback?source=mcp',
'http://localhost:3210/callback',
'http://127.0.0.1:3210/callback',
'http://127.255.255.255:3210/callback',
'http://[::1]:3210/callback'
])('allows HTTPS and local loopback callbacks: %s', (redirectUri) => {
expect(isAllowedOAuthRedirectUri(redirectUri)).toBe(true)
})

it.each([
'http://client.example/callback',
'http://localhost.example/callback',
'ftp://client.example/callback',
'com.example.app:/callback',
'//client.example/callback',
'https://user@client.example/callback',
'https://client.example/callback#fragment',
' https://client.example/callback'
])('rejects non-HTTPS remote or ambiguous callbacks: %s', (redirectUri) => {
expect(isAllowedOAuthRedirectUri(redirectUri)).toBe(false)
})
})
79 changes: 79 additions & 0 deletions tests/auth/cursor-redirects.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { env, exports } from 'cloudflare:workers'
import { afterEach, describe, expect, it } from 'vitest'
import { clearKv } from '../helpers/kv'

const MCP_ORIGIN = 'https://mcp.cloudflare.com'
const DOWNSTREAM_CODE_CHALLENGE = 'I4fhllfHqqQsgap17V2SDI0scSei8H7U0e0rZBDIcbo'

// What some Cursor versions register (seen in production): a private-use callback next to
// https and loopback ones. Cursor signs in with the loopback (or the https) one.
const CURSOR_REDIRECT_URIS = [
'cursor://anysphere.cursor-mcp/oauth/callback',
'https://www.cursor.com/agents/mcp/oauth/callback',
'http://localhost:8787/callback'
]

function register(redirectUris: string[]): Promise<Response> {
return exports.default.fetch(
new Request(`${MCP_ORIGIN}/register`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
client_name: 'Cursor',
redirect_uris: redirectUris,
grant_types: ['authorization_code', 'refresh_token'],
response_types: ['code'],
token_endpoint_auth_method: 'none'
})
})
)
}

function authorize(clientId: string, redirectUri: string): Promise<Response> {
const url = new URL(`${MCP_ORIGIN}/authorize`)
url.search = new URLSearchParams({
response_type: 'code',
client_id: clientId,
redirect_uri: redirectUri,
resource: `${MCP_ORIGIN}/mcp`,
scope: 'user:read',
state: 'client-state',
code_challenge: DOWNSTREAM_CODE_CHALLENGE,
code_challenge_method: 'S256'
}).toString()
return exports.default.fetch(new Request(url), { redirect: 'manual' })
}

describe('Cursor registrations with a cursor:// callback', () => {
afterEach(async () => {
await clearKv(env.OAUTH_KV)
})

it("accepts Cursor's registration and signs in through its loopback callback", async () => {
const registration = await register(CURSOR_REDIRECT_URIS)
expect(registration.status).toBe(201)
const { client_id: clientId } = (await registration.json()) as { client_id: string }

const consent = await authorize(clientId, 'http://localhost:8787/callback')
expect(consent.status).toBe(200)
expect(await consent.text()).toContain('name="handle"')
})

it('still refuses an authorization that uses the private-use callback, without redirecting', async () => {
const registration = await register(CURSOR_REDIRECT_URIS)
const { client_id: clientId } = (await registration.json()) as { client_id: string }

const refused = await authorize(clientId, 'cursor://anysphere.cursor-mcp/oauth/callback')
expect(refused.status).toBe(400)
expect(refused.headers.get('location')).toBeNull()
// workers-oauth-provider holds the URI each request uses to https or loopback http.
expect(await refused.text()).toContain('Invalid redirect URI')
expect((await env.OAUTH_KV.list({ prefix: 'grant:' })).keys).toHaveLength(0)
})

it('still refuses a registration with a remote http callback', async () => {
const registration = await register(['http://remote.example/callback'])
expect(registration.status).toBe(400)
await expect(registration.json()).resolves.toMatchObject({ error: 'invalid_client_metadata' })
})
})
Loading