diff --git a/package-lock.json b/package-lock.json index 4a2d6c0..219fd94 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,7 +8,7 @@ "name": "cloudflare-mcp", "version": "0.1.0", "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" @@ -169,9 +169,9 @@ } }, "node_modules/@cloudflare/workers-oauth-provider": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/@cloudflare/workers-oauth-provider/-/workers-oauth-provider-1.2.0.tgz", - "integrity": "sha512-geQfLHOWgeDmYb2YYEuTFvmeP8g/2uIXgNEag9K7fUY1baTIRW8aNpNUUMziWr97wQi2Ez5/59ukgHT2n9a4mg==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@cloudflare/workers-oauth-provider/-/workers-oauth-provider-1.2.1.tgz", + "integrity": "sha512-5bw7JJI9Nd4ArHfNnvTF8H2OpSbA2GbqTmX80ly47h7dR+atgjFsn9Wfz+biKlAKClTEUYE0KAr791RkED41VA==", "license": "MIT" }, "node_modules/@cspotcode/source-map-support": { diff --git a/package.json b/package.json index baab1cd..b011d5d 100644 --- a/package.json +++ b/package.json @@ -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" diff --git a/src/auth/oauth-handler.ts b/src/auth/oauth-handler.ts index f4d8279..1a15be3 100644 --- a/src/auth/oauth-handler.ts +++ b/src/auth/oauth-handler.ts @@ -16,7 +16,6 @@ import { } from './scopes' import { AuthProps as AuthPropsSchema, AUTH_PROPS_VERSION, type AuthProps } from './types' import { - isAllowedOAuthRedirectUri, parseRedirectApproval, renderApprovalDialog, renderErrorPage, @@ -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 */ @@ -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) @@ -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)) @@ -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, { @@ -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) @@ -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) diff --git a/src/auth/workers-oauth-utils.ts b/src/auth/workers-oauth-utils.ts index 4cdffcc..5163c20 100644 --- a/src/auth/workers-oauth-utils.ts +++ b/src/auth/workers-oauth-utils.ts @@ -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. diff --git a/tests/auth/approval-dialog.test.ts b/tests/auth/approval-dialog.test.ts index 579ef91..e9a3f75 100644 --- a/tests/auth/approval-dialog.test.ts +++ b/tests/auth/approval-dialog.test.ts @@ -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 @@ -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) - }) -}) diff --git a/tests/auth/cursor-redirects.test.ts b/tests/auth/cursor-redirects.test.ts new file mode 100644 index 0000000..fe02cd1 --- /dev/null +++ b/tests/auth/cursor-redirects.test.ts @@ -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 { + 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 { + 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' }) + }) +})