From 650d0b1ada22b1e0c42705b52711eed64bf29d2e Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Mon, 28 Sep 2026 17:54:32 +0100 Subject: [PATCH 1/2] fix(auth): workers-oauth-provider 1.2.1 accepts Cursor's registration; drop our own redirect check Some Cursor versions register cursor://anysphere.cursor-mcp/oauth/callback next to their https and loopback callbacks. 1.2.0 refused the whole registration; 1.2.1 accepts a registration with at least one compliant redirect URI and still refuses a request that uses a refused one (cloudflare/workers-oauth-provider#391). The provider now enforces the redirect policy on every request (parseAuthRequest, completeAuthorization, and redirectUri on an AuthorizationError only when safe), so isAllowedOAuthRedirectUri, isLoopbackHostname and invalidRedirectUriResponse duplicated it. Removed, with their unit tests; end-to-end coverage stays in cimd.test.ts and the new cursor-redirects.test.ts. --- package-lock.json | 6 +-- package.json | 2 +- src/auth/oauth-handler.ts | 28 +--------- src/auth/workers-oauth-utils.ts | 30 ----------- tests/auth/approval-dialog.test.ts | 27 ---------- tests/auth/cursor-redirects.test.ts | 79 +++++++++++++++++++++++++++++ 6 files changed, 85 insertions(+), 87 deletions(-) create mode 100644 tests/auth/cursor-redirects.test.ts diff --git a/package-lock.json b/package-lock.json index 4a2d6c02..137eb79c 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": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@391", "@modelcontextprotocol/server": "2.0.0", "hono": "^4.13.5", "zod": "^4.3.5" @@ -170,8 +170,8 @@ }, "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==", + "resolved": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@391", + "integrity": "sha512-ml4Wfcp5c0rfL/SfzuMCksSr8WRQzvVJrg5n9Gz2nZQPcw9zd/2+y3ufmv6PP0SCNnYcTSDR7OwIRpuVwgrw8A==", "license": "MIT" }, "node_modules/@cspotcode/source-map-support": { diff --git a/package.json b/package.json index baab1cd8..6be20ccb 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": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@391", "@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 f4d82790..1a15be35 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 4cdffccc..5163c205 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 579ef917..e9a3f75d 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 00000000..fe02cd19 --- /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' }) + }) +}) From 664a59cdb2f32ddb8702a59cc35bf5e5c0ae55ff Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Mon, 28 Sep 2026 18:05:54 +0100 Subject: [PATCH 2/2] chore(deps): workers-oauth-provider 1.2.1 from npm --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 137eb79c..219fd945 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": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@391", + "@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://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@391", - "integrity": "sha512-ml4Wfcp5c0rfL/SfzuMCksSr8WRQzvVJrg5n9Gz2nZQPcw9zd/2+y3ufmv6PP0SCNnYcTSDR7OwIRpuVwgrw8A==", + "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 6be20ccb..b011d5db 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "seed:prod": "tsx scripts/seed-r2.ts production" }, "dependencies": { - "@cloudflare/workers-oauth-provider": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@391", + "@cloudflare/workers-oauth-provider": "^1.2.1", "@modelcontextprotocol/server": "2.0.0", "hono": "^4.13.5", "zod": "^4.3.5"