Files
accounted/lib/auth/__tests__/oauth-allowlist.test.ts
T
MattssonandClaude Fable 5.1 bc5da12372 fix(mcp-oauth): allowlist Cursor's OAuth callbacks so its dynamic registration succeeds (#2225)
* fix(mcp-oauth): allowlist Cursor's OAuth callbacks so its dynamic registration succeeds

Cursor (IDE, CLI, and Grok Bot on top of it) registers three redirect URIs
in one /register request: cursor://anysphere.cursor-mcp/oauth/callback,
https://www.cursor.com/agents/mcp/oauth/callback and
http://localhost:8787/callback. Only the loopback matched a built-in
pattern and /register fails the whole set on any unknown URI, so every
Cursor connection to the URL we hand out in Settings died with
"Redirect URI not allowed". Users cannot self-register the cursor://
form either (the settings panel requires https).

Add a built-in `cursor` provider with the two non-loopback callbacks as
exact matches (no cursor.com prefix), name it "Cursor (Anysphere)" on
the consent page, list the pre-approved clients in the OAuth clients
settings text (sv + en) and the mcp-server rules, and cover the
register, allowlist and consent paths with tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBFeTvQXgMCNXR6fG9drff

* fix(mcp-oauth): show the cursor:// deeplink unverified and let CSP pass its post-consent redirect

Review findings on #2225, one pass:

- Skeptic (correctness), REFUTED: new URL('cursor://...').origin is the
  string "null", so the consent page emitted form-action 'self' null and
  Chromium would block the 303 to the deeplink after Allow. The header
  now uses the scheme-source (cursor:) when the origin is opaque; a test
  pins the header on the cursor:// URI.
- Skeptic (security), CodeRabbit (Major) and Superagent (P2): a custom
  scheme can be claimed by any local app (RFC 8252 section 8.4), so it
  must not be presented as a vendor-verified callback. The deeplink is
  its own provider, cursor_deeplink, rendered "Cursor (Anysphere)" with
  the localhost tag "Din egen dator" and verified: false. The https
  cursor.com callback keeps the verified label. A test pins that a code
  minted without a code_challenge can never be exchanged, which is what
  keeps a scheme hijack from turning into a token.
- CodeRabbit (Minor): the rules doc now says the Grok callback matches
  with or without the trailing slash.
- Regression skeptic: docs/WHITELABEL.md listed only Claude and
  localhost and pointed at the wrong file; now lists the built-ins and
  points at lib/auth/oauth-allowlist.ts.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DBFeTvQXgMCNXR6fG9drff

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-03 15:33:58 +02:00

299 lines
12 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
import { describe, it, expect, vi, afterEach } from 'vitest'
import {
builtInRedirectProvider,
capScopesForRole,
isAllowedRedirectUri,
isBuiltInRedirectUri,
lookupCompanyRole,
resolveRedirectUri,
} from '../oauth-allowlist'
import { ALL_SCOPES, type ApiKeyScope } from '../scope-catalog'
import type { SupabaseClient } from '@supabase/supabase-js'
describe('isBuiltInRedirectUri', () => {
it.each([
['https://claude.ai/api/oauth/callback', true],
['https://claude.com/api/oauth/callback', true],
['https://chatgpt.com/connector/oauth/abc123', true],
['https://chatgpt.com/connector_platform_oauth_redirect', true],
['https://chatgpt.com/connector_platform_oauth_redirect/extra', false],
['https://chatgpt.com/other/path', false],
['https://chatgpt.com.evil.com/connector/oauth/x', false],
['https://grok.com/connectors-oauth-exchange-code/', true],
['https://grok.com/connectors-oauth-exchange-code', true],
['https://grok.com/connectors-oauth-exchange-code/extra', false],
['https://grok.com/connectors-oauth-exchange-code/?next=x', false],
['https://grok.com/other/path', false],
['https://grok.com.evil.com/connectors-oauth-exchange-code/', false],
['http://grok.com/connectors-oauth-exchange-code/', false],
['cursor://anysphere.cursor-mcp/oauth/callback', true],
['cursor://anysphere.cursor-mcp/oauth/callback/', false],
['cursor://anysphere.cursor-mcp/oauth/callback?x=1', false],
['cursor://anysphere.cursor-mcp/oauth/callback/extra', false],
['cursor://evil.extension/oauth/callback', false],
['https://www.cursor.com/agents/mcp/oauth/callback', true],
['https://www.cursor.com/agents/mcp/oauth/callback/', false],
['https://www.cursor.com/agents/mcp/oauth/callback2', false],
['https://cursor.com/agents/mcp/oauth/callback', false],
['https://www.cursor.com.evil.com/agents/mcp/oauth/callback', false],
['http://www.cursor.com/agents/mcp/oauth/callback', false],
['http://localhost:8787/callback', true],
['http://localhost:3000/cb', true],
['http://localhost/cb', true],
['http://127.0.0.1:8080/cb', true],
['https://evil.com/cb', false],
['https://example.com/api/foo', false],
['ftp://localhost/cb', false],
['', false],
])('classifies %s as %s', (uri, expected) => {
expect(isBuiltInRedirectUri(uri)).toBe(expected)
})
})
describe('builtInRedirectProvider', () => {
it.each([
['https://claude.ai/api/oauth/callback', 'claude'],
['https://claude.com/api/oauth/callback', 'claude'],
['https://chatgpt.com/connector/oauth/abc123', 'chatgpt'],
['https://chatgpt.com/connector_platform_oauth_redirect', 'chatgpt'],
['https://grok.com/connectors-oauth-exchange-code/', 'grok'],
['https://grok.com/connectors-oauth-exchange-code/extra', null],
['cursor://anysphere.cursor-mcp/oauth/callback', 'cursor_deeplink'],
['https://www.cursor.com/agents/mcp/oauth/callback', 'cursor'],
['http://localhost:8787/callback', 'local'],
['http://localhost:3000/cb', 'local'],
['http://127.0.0.1:8080/cb', 'local'],
['https://claude-login.example/cb', null],
['', null],
])('maps %s to %s', (uri, expected) => {
expect(builtInRedirectProvider(uri)).toBe(expected)
})
})
type Row = Record<string, unknown>
/**
* Minimal PostgREST-shaped fake over in-memory tables: eq/is filters are
* applied, everything else is a no-op, and the chain resolves to the filtered
* rows (all rows when awaited, first row via maybeSingle). `failTable` makes
* every query against that table return a DB error.
*/
function fakeClient(tables: Record<string, Row[]>, failTable?: string) {
const from = vi.fn((table: string) => {
const filters: [string, unknown][] = []
const run = () =>
(tables[table] ?? []).filter((row) =>
filters.every(([col, val]) => (val === null ? row[col] == null : row[col] === val)),
)
const result = () =>
table === failTable
? { data: null, error: { message: 'db down' } }
: { data: run(), error: null }
const chain: Record<string, unknown> = {}
for (const method of ['select', 'order', 'range', 'limit']) chain[method] = () => chain
chain.eq = (col: string, val: unknown) => {
filters.push([col, val])
return chain
}
chain.is = (col: string, val: unknown) => {
filters.push([col, val])
return chain
}
chain.maybeSingle = async () => {
const r = result()
return { data: Array.isArray(r.data) ? (r.data[0] ?? null) : null, error: r.error }
}
chain.then = (resolve: (v: unknown) => void) => resolve(result())
return chain
})
return { from } as unknown as SupabaseClient & { from: ReturnType<typeof vi.fn> }
}
const REGISTRATIONS: Row[] = [
{ id: 'reg-1', user_id: 'user-2', client_name: 'Byråns bot', redirect_uri: 'https://app.example.com/cb', revoked_at: null },
{ id: 'reg-2', user_id: 'user-9', client_name: 'Evil', redirect_uri: 'https://evil.example/cb', revoked_at: null },
{ id: 'reg-3', user_id: 'user-1', client_name: 'Min egen app', redirect_uri: 'https://mine.example/cb', revoked_at: null },
{ id: 'reg-4', user_id: 'user-1', client_name: 'Gammal app', redirect_uri: 'https://old.example/cb', revoked_at: '2026-01-01T00:00:00Z' },
]
const MEMBERSHIPS: Row[] = [
{ id: 'm1', user_id: 'user-1', company_id: 'company-1', role: 'owner' },
{ id: 'm2', user_id: 'user-2', company_id: 'company-1', role: 'member' },
{ id: 'm3', user_id: 'user-2', company_id: 'company-2', role: 'owner' },
{ id: 'm4', user_id: 'user-9', company_id: 'company-9', role: 'owner' },
]
const DB = { oauth_client_registrations: REGISTRATIONS, company_members: MEMBERSHIPS }
describe('resolveRedirectUri', () => {
afterEach(() => {
vi.unstubAllEnvs()
})
it('short-circuits to the built-in provider without touching the DB', async () => {
const sb = {
from: vi.fn(() => {
throw new Error('should not be called')
}),
} as unknown as SupabaseClient
expect(await resolveRedirectUri('https://claude.ai/api/cb', sb, { consentingUserId: 'user-1' })).toEqual({
allowed: true,
kind: 'built_in',
provider: 'claude',
})
expect(await resolveRedirectUri('http://localhost:3000/cb', sb)).toEqual({
allowed: true,
kind: 'built_in',
provider: 'local',
})
})
it("accepts the consenting user's own registration", async () => {
const result = await resolveRedirectUri('https://mine.example/cb', fakeClient(DB), {
consentingUserId: 'user-1',
})
expect(result).toEqual({
allowed: true,
kind: 'registered',
clientName: 'Min egen app',
registeredByConsentingUser: true,
})
})
it('accepts a registration by a colleague who shares a company', async () => {
const result = await resolveRedirectUri('https://app.example.com/cb', fakeClient(DB), {
consentingUserId: 'user-1',
})
expect(result).toEqual({
allowed: true,
kind: 'registered',
clientName: 'Byråns bot',
registeredByConsentingUser: false,
})
})
it('rejects a registration by an unrelated user', async () => {
// user-9 is a real member of the instance, just not of any company user-1
// belongs to: their registration must not be a valid target for user-1.
const result = await resolveRedirectUri('https://evil.example/cb', fakeClient(DB), {
consentingUserId: 'user-1',
})
expect(result).toEqual({ allowed: false })
})
it('accepts any active registration when no consenting user is given (anonymous /register)', async () => {
const result = await resolveRedirectUri('https://evil.example/cb', fakeClient(DB))
expect(result).toEqual({
allowed: true,
kind: 'registered',
clientName: 'Evil',
registeredByConsentingUser: false,
})
})
it('rejects a revoked registration, even the user’s own', async () => {
const result = await resolveRedirectUri('https://old.example/cb', fakeClient(DB), {
consentingUserId: 'user-1',
})
expect(result).toEqual({ allowed: false })
})
it('rejects an unknown URI', async () => {
expect(await resolveRedirectUri('https://nowhere.example/cb', fakeClient(DB))).toEqual({ allowed: false })
})
it('fails closed when the registration lookup errors', async () => {
const result = await resolveRedirectUri(
'https://mine.example/cb',
fakeClient(DB, 'oauth_client_registrations'),
{ consentingUserId: 'user-1' },
)
expect(result).toEqual({ allowed: false })
})
it('fails closed when the shared-company check errors', async () => {
const result = await resolveRedirectUri('https://app.example.com/cb', fakeClient(DB, 'company_members'), {
consentingUserId: 'user-1',
})
expect(result).toEqual({ allowed: false })
})
it('fails closed when no client is given and none can be constructed', async () => {
vi.stubEnv('NEXT_PUBLIC_SUPABASE_URL', '')
vi.stubEnv('SUPABASE_SERVICE_ROLE_KEY', '')
expect(await resolveRedirectUri('https://mine.example/cb', undefined, { consentingUserId: 'user-1' })).toEqual({
allowed: false,
})
})
it('returns not allowed for empty / non-string inputs', async () => {
expect(await resolveRedirectUri('')).toEqual({ allowed: false })
expect(await resolveRedirectUri(undefined as unknown as string)).toEqual({ allowed: false })
})
})
describe('isAllowedRedirectUri', () => {
it('is the boolean view of resolveRedirectUri', async () => {
expect(await isAllowedRedirectUri('https://claude.ai/api/cb')).toBe(true)
expect(await isAllowedRedirectUri('https://mine.example/cb', fakeClient(DB), { consentingUserId: 'user-1' })).toBe(true)
expect(await isAllowedRedirectUri('https://evil.example/cb', fakeClient(DB), { consentingUserId: 'user-1' })).toBe(false)
expect(await isAllowedRedirectUri('https://evil.example/cb', fakeClient(DB))).toBe(true)
expect(await isAllowedRedirectUri('', fakeClient(DB))).toBe(false)
})
})
describe('capScopesForRole', () => {
const mixed: ApiKeyScope[] = [
'transactions:read',
'transactions:write',
'pending_operations:approve',
'webhooks:manage',
'reconciliation:signoff',
'reports:read',
]
it('keeps every scope for owner, admin and member', () => {
for (const role of ['owner', 'admin', 'member']) {
expect(capScopesForRole(mixed, role)).toEqual(mixed)
}
})
it('caps a viewer to :read scopes only', () => {
expect(capScopesForRole(mixed, 'viewer')).toEqual(['transactions:read', 'reports:read'])
})
it('caps an unknown role and a missing membership to :read scopes', () => {
expect(capScopesForRole(mixed, 'superuser')).toEqual(['transactions:read', 'reports:read'])
expect(capScopesForRole(mixed, null)).toEqual(['transactions:read', 'reports:read'])
})
it('leaves no write-kind scope in a capped set for the full catalogue', () => {
const capped = capScopesForRole(ALL_SCOPES, 'viewer')
expect(capped.length).toBeGreaterThan(0)
expect(capped.every((s) => s.endsWith(':read'))).toBe(true)
expect(capped).not.toContain('pending_operations:approve')
})
it('returns a copy, never the caller’s array', () => {
const result = capScopesForRole(mixed, 'owner')
expect(result).not.toBe(mixed)
})
})
describe('lookupCompanyRole', () => {
it('returns the role for an existing membership', async () => {
expect(await lookupCompanyRole(fakeClient(DB), 'user-2', 'company-1')).toEqual({ role: 'member', error: null })
expect(await lookupCompanyRole(fakeClient(DB), 'user-2', 'company-2')).toEqual({ role: 'owner', error: null })
})
it('returns a null role when no membership row exists', async () => {
expect(await lookupCompanyRole(fakeClient(DB), 'user-9', 'company-1')).toEqual({ role: null, error: null })
})
it('surfaces a query error instead of guessing', async () => {
const result = await lookupCompanyRole(fakeClient(DB, 'company_members'), 'user-1', 'company-1')
expect(result.role).toBeNull()
expect(result.error).toBe('db down')
})
})