diff --git a/AGENTS.md b/AGENTS.md index ce3c245..95acabd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,8 +10,8 @@ When modifying MCP or OAuth functionality, **always check the latest published MCP specification**: -- **Specification:** https://modelcontextprotocol.io/specification/2025-11-25 -- **Authorization section:** https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization +- **Specification:** https://modelcontextprotocol.io/specification/2026-07-28 +- **Authorization section:** https://modelcontextprotocol.io/specification/2026-07-28/basic/authorization ## Repository structure @@ -27,7 +27,8 @@ cloudflare-mcp/ │ ├── metrics.ts # Analytics Engine metrics (auth_user/tool_call) │ ├── auth/ │ │ ├── types.ts # Auth props schemas (Zod discriminated union) -│ │ ├── api-token-mode.ts # Direct Cloudflare API token support +│ │ ├── api-token-mode.ts # Prefix classification & external resolver +│ │ ├── cloudflare-identity.ts # Owner-aware Cloudflare API identity probes │ │ ├── cloudflare-auth.ts # PKCE & OAuth utilities │ │ ├── oauth-handler.ts # OAuth authorization flow │ │ ├── derived-oauth-scopes.ts # Canonical production OAuth scope API metadata @@ -120,10 +121,12 @@ Code execution uses Cloudflare's Worker Loader API to dynamically create isolate ### Authentication -Two modes via Zod discriminated union (`AuthProps`): +Two credential paths produce the same Zod-validated `AuthProps` union: -- **OAuth mode** (default): Uses `@cloudflare/workers-oauth-provider` with PKCE. Supports both account-scoped and user-scoped tokens. -- **API token mode**: Direct Cloudflare API tokens bypass OAuth. Detection: OAuth tokens have 3 colon-separated parts; API tokens don't. +- **OAuth mode** (default): `@cloudflare/workers-oauth-provider` validates its own access tokens and restores encrypted Cloudflare OAuth props. The upstream authorization-code and refresh flows use PKCE and remain separate from direct credential resolution. +- **Direct Cloudflare credential mode**: after the provider's internal lookup misses, `resolveExternalToken` validates the bearer against the Cloudflare API and returns request-local props. Prefixes are owner hints: `cfat_` account tokens query only `/accounts`; `cfut_` user API tokens and `cfoat_` Cloudflare OAuth credentials query `/user` and `/accounts`; unprefixed legacy tokens retain response-based inference. Expected failures use the provider's `ExternalTokenError`, which generates RFC 6750/9728 `401`/`403` challenges and preserves retry guidance. + +Validated direct-credential identity is cached by token hash in `OAUTH_KV`; provider-issued MCP tokens never invoke the external resolver. The consent picker uses the production catalog returned by `GET /oauth/scopes` in every deployment. Staging may register additional scopes, but the MCP picker exposes them only after they reach production. Only the user, account, and offline-access OAuth bootstrap scopes sit outside the API catalog. Terraform registration must land before deploying picker additions. The app does not impose a scope-count cap. diff --git a/package-lock.json b/package-lock.json index 723ea21..202f1ad 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": "^0.8.3", + "@cloudflare/workers-oauth-provider": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@272", "@modelcontextprotocol/server": "2.0.0", "hono": "^4.12.25", "zod": "^4.3.5" @@ -168,8 +168,8 @@ }, "node_modules/@cloudflare/workers-oauth-provider": { "version": "0.8.3", - "resolved": "https://registry.npmjs.org/@cloudflare/workers-oauth-provider/-/workers-oauth-provider-0.8.3.tgz", - "integrity": "sha512-5dIeQlJTDKAQvy3GcrtWBhPvTb9lHeRp/m8MoostTkcrbrGImH8zKSdJhogQ5YSTFUCg+b7FBg38b7KCM/x3VQ==", + "resolved": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@272", + "integrity": "sha512-WCPwKtejzaj1VvidZ5K9VKnFicVEGABPi9hcjW4slBt0w9Ukqi80MJURwyNKAEljQhjpnPOAtXuC2ucuO4eMow==", "license": "MIT" }, "node_modules/@cspotcode/source-map-support": { diff --git a/package.json b/package.json index 9832127..1e3607c 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "seed:prod": "tsx scripts/seed-r2.ts production" }, "dependencies": { - "@cloudflare/workers-oauth-provider": "^0.8.3", + "@cloudflare/workers-oauth-provider": "https://pkg.pr.new/cloudflare/workers-oauth-provider/@cloudflare/workers-oauth-provider@272", "@modelcontextprotocol/server": "2.0.0", "hono": "^4.12.25", "zod": "^4.3.5" diff --git a/src/auth/api-token-mode.ts b/src/auth/api-token-mode.ts index f4a069e..d3f04f2 100644 --- a/src/auth/api-token-mode.ts +++ b/src/auth/api-token-mode.ts @@ -1,17 +1,25 @@ -import { env as cloudflareEnv } from 'cloudflare:workers' - -import { getUserAndAccounts } from './oauth-handler' +import { + ExternalTokenError, + type ResolveExternalTokenInput, + type ResolveExternalTokenResult +} from '@cloudflare/workers-oauth-provider' + +import { + CloudflareIdentitySchema, + resolveCloudflareCredential, + type CloudflareIdentity, + type CloudflareTokenOwner +} from './cloudflare-identity' +import { AUTH_PROPS_VERSION, type AuthProps } from './types' import { OAuthError } from './workers-oauth-utils' -import { AUTH_PROPS_VERSION, type AccountSchema, type AuthProps, type UserSchema } from './types' - -const env = cloudflareEnv as Env const API_TOKEN_IDENTITY_CACHE_TTL_SECONDS = 2_592_000 -type ApiTokenIdentity = { - user: UserSchema | null - accounts: AccountSchema[] - accountCount?: number +/** Prefixes are ownership hints; unprefixed legacy credentials remain supported. */ +export function cloudflareTokenOwner(token: string): CloudflareTokenOwner { + if (token.startsWith('cfat_')) return 'account' + if (token.startsWith('cfut_') || token.startsWith('cfoat_')) return 'user' + return 'unknown' } async function hashApiToken(token: string): Promise { @@ -19,24 +27,28 @@ async function hashApiToken(token: string): Promise { return [...new Uint8Array(digest)].map((byte) => byte.toString(16).padStart(2, '0')).join('') } -async function getCachedApiTokenIdentity(token: string): Promise { - const tokenHash = await hashApiToken(token) - // v2 namespace: abandons pre-versioning entries that may hold a truncated - // first page of accounts rather than the full (or count-only) list. - const cacheKey = `api-token-identity:v2:${tokenHash}` +async function getCachedIdentity( + token: string, + tokenOwner: CloudflareTokenOwner, + kv: KVNamespace +): Promise { + const cacheKey = `api-token-identity:v4:${await hashApiToken(token)}` + try { - const cached = await env.OAUTH_KV.get(cacheKey, 'json') - if (cached) { - return cached + const cachedValue = await kv.get(cacheKey, 'json') + if (cachedValue !== null) { + const cached = CloudflareIdentitySchema.safeParse(cachedValue) + if (cached.success) return cached.data + console.warn('api_token_identity_probe ignored invalid cache entry') } } catch (error) { console.warn('api_token_identity_probe kv-cache read failed', error) } - const identity = await getUserAndAccounts(token, 'api_token_identity_probe') + const identity = await resolveCloudflareCredential(token, tokenOwner) try { - await env.OAUTH_KV.put(cacheKey, JSON.stringify(identity), { + await kv.put(cacheKey, JSON.stringify(identity), { expirationTtl: API_TOKEN_IDENTITY_CACHE_TTL_SECONDS }) } catch (error) { @@ -46,117 +58,71 @@ async function getCachedApiTokenIdentity(token: string): Promise Promise -): Promise { - if (!isDirectApiToken(request)) { - return null +function externalTokenError( + error: OAuthError, + tokenOwner: CloudflareTokenOwner +): ExternalTokenError { + const common = { description: error.description, headers: error.headers } + + switch (error.code) { + case 'invalid_token': + return new ExternalTokenError('invalid_token', { ...common, statusCode: 401 }) + case 'insufficient_scope': + return new ExternalTokenError('insufficient_scope', { + ...common, + statusCode: 403, + requiredScopes: tokenOwner === 'account' ? ['account:read'] : ['user:read', 'account:read'] + }) + case 'temporarily_unavailable': + return new ExternalTokenError('temporarily_unavailable', { + ...common, + statusCode: error.statusCode + }) + case 'server_error': + return new ExternalTokenError('server_error', { + ...common, + statusCode: error.statusCode + }) + default: + return new ExternalTokenError(error.statusCode >= 500 ? 'server_error' : 'invalid_token', { + ...common, + statusCode: error.statusCode >= 500 ? 502 : 401 + }) } +} - const token = extractBearerToken(request) - if (!token) { - return new Response(JSON.stringify({ error: 'Authorization header required' }), { - status: 401, - headers: { 'Content-Type': 'application/json' } - }) - } - - try { - const { user, accounts, accountCount } = await getCachedApiTokenIdentity(token) - - // Account-scoped token - if (!user) { - if (accounts.length === 0) { - return new Response(JSON.stringify({ error: 'Invalid token' }), { - status: 401, - headers: { 'Content-Type': 'application/json' } - }) +/** Convert a verified Cloudflare identity into request-local tool props. */ +export function buildAuthProps(token: string, identity: CloudflareIdentity): AuthProps { + switch (identity.type) { + case 'account': + return { + type: 'account_token', + accessToken: token, + account: identity.account } - if (accounts.length > 1) { - return new Response( - JSON.stringify({ - error: 'Token has access to multiple accounts - use account_id parameter' - }), - { status: 400, headers: { 'Content-Type': 'application/json' } } - ) + case 'user': + return { + type: 'user_token', + accessToken: token, + user: identity.user, + accounts: identity.accounts, + accountCount: identity.accountCount, + version: AUTH_PROPS_VERSION } - const props = buildAuthProps(token, null, accounts) - return createMcpResponse(props) - } - - // User token - const props = buildAuthProps(token, user, accounts, accountCount) - return createMcpResponse(props) - } catch (err) { - if (err instanceof OAuthError) { - return err.toResponse() - } - return new Response(JSON.stringify({ error: 'Token verification failed' }), { - status: 401, - headers: { 'Content-Type': 'application/json' } - }) } } -/** - * Build AuthProps from verified token info - */ -export function buildAuthProps( - token: string, - user?: { id: string; email: string } | null, - accounts?: Array<{ id: string; name: string }>, - accountCount?: number -): AuthProps { - if (user) { - return { - type: 'user_token', - accessToken: token, - user, - accounts: accounts || [], - accountCount, - version: AUTH_PROPS_VERSION - } - } - - if (!accounts || accounts.length === 0) { - throw new Error('Cannot build auth props: no user or account information') - } - - return { - type: 'account_token', - accessToken: token, - account: accounts[0] +/** Resolve direct Cloudflare credentials after the provider's internal token lookup misses. */ +export async function resolveExternalToken({ + token, + env +}: ResolveExternalTokenInput): Promise { + const tokenOwner = cloudflareTokenOwner(token) + try { + const identity = await getCachedIdentity(token, tokenOwner, env.OAUTH_KV) + return { props: buildAuthProps(token, identity) } + } catch (error) { + if (error instanceof OAuthError) throw externalTokenError(error, tokenOwner) + throw error } } diff --git a/src/auth/cloudflare-identity.ts b/src/auth/cloudflare-identity.ts new file mode 100644 index 0000000..9a1d8ac --- /dev/null +++ b/src/auth/cloudflare-identity.ts @@ -0,0 +1,287 @@ +import { env as cloudflareEnv } from 'cloudflare:workers' +import { z } from 'zod' + +import { fetchWithRetry } from '../utils/fetch-retry' +import { + AccountSchema, + AccountsSchema, + ACCOUNTS_PROBE_PAGE_SIZE, + MAX_STORED_ACCOUNTS, + UserSchema +} from './types' +import { OAuthError } from './workers-oauth-utils' + +const env = cloudflareEnv as Env +const API_TOKEN_IDENTITY_CALLER = 'api_token_identity_probe' +const OAUTH_IDENTITY_CALLER = 'oauth_callback_identity_probe' +// /accounts enforces a minimum page size of five. That is still enough to +// prove the account-token invariant without fetching the 31-record user page. +const ACCOUNT_TOKEN_PROBE_PAGE_SIZE = 5 + +export type CloudflareTokenOwner = 'account' | 'unknown' | 'user' + +const AccountIdentitySchema = z.object({ + type: z.literal('account'), + account: AccountSchema +}) + +const UserIdentitySchema = z.object({ + type: z.literal('user'), + user: UserSchema, + accounts: AccountsSchema, + accountCount: z.number().int().nonnegative().optional() +}) + +export const CloudflareIdentitySchema = z.discriminatedUnion('type', [ + AccountIdentitySchema, + UserIdentitySchema +]) + +export type CloudflareIdentity = z.infer +export type CloudflareUserIdentity = z.infer + +type AccountPage = { + accounts: z.infer + totalCount?: number +} + +const UserResponseSchema = z.object({ + success: z.boolean().optional(), + result: UserSchema.nullable().optional() +}) + +const AccountsResponseSchema = z.object({ + success: z.boolean().optional(), + result: AccountsSchema.optional(), + result_info: z + .object({ + count: z.number().optional(), + total_count: z.number().optional() + }) + .optional() +}) + +function retryAfterHeaders(...responses: Response[]): Record { + return { + 'Retry-After': + responses.find((response) => response.status === 429)?.headers.get('Retry-After') ?? '30' + } +} + +function throwIdentityProbeError(...responses: [Response, ...Response[]]): never { + const statuses = responses.map((response) => response.status) + + if (statuses.some((status) => status >= 500)) { + throw new OAuthError('server_error', 'Cloudflare API is temporarily unavailable', 502) + } + if (statuses.includes(429)) { + throw new OAuthError( + 'temporarily_unavailable', + 'Rate limited, try again later', + 429, + retryAfterHeaders(...responses) + ) + } + if (statuses.includes(401)) { + throw new OAuthError('invalid_token', 'Access token is invalid or expired', 401) + } + if (statuses.includes(403)) { + throw new OAuthError( + 'insufficient_scope', + 'Token lacks required user:read or account:read scope', + 403 + ) + } + if (statuses.includes(400)) { + throw new OAuthError( + 'invalid_token', + 'Access token appears malformed; reauthenticate and try again', + 401 + ) + } + + throw new OAuthError('invalid_token', 'Access token is invalid or expired', statuses[0]) +} + +async function fetchProbe(url: string, accessToken: string, caller: string): Promise { + try { + return await fetchWithRetry( + url, + { headers: { Authorization: `Bearer ${accessToken}` } }, + { caller } + ) + } catch (error) { + console.error('Cloudflare API request failed', error) + throw new OAuthError('server_error', 'Cloudflare API is temporarily unavailable', 502) + } +} + +function fetchUser(accessToken: string, caller: string): Promise { + return fetchProbe(`${env.CLOUDFLARE_API_BASE}/user`, accessToken, caller) +} + +function fetchAccounts( + accessToken: string, + caller: string, + pageSize = ACCOUNTS_PROBE_PAGE_SIZE +): Promise { + return fetchProbe(`${env.CLOUDFLARE_API_BASE}/accounts?per_page=${pageSize}`, accessToken, caller) +} + +async function parseUser(response: Response): Promise { + try { + const parsed = UserResponseSchema.safeParse(await response.json()) + if (parsed.success && parsed.data.success && parsed.data.result) return parsed.data.result + if (!parsed.success) { + console.error('Cloudflare API /user payload did not match expected shape', parsed.error) + } + } catch (error) { + console.error('Cloudflare API /user response is not valid JSON', error) + } + return null +} + +async function parseAccounts(response: Response): Promise { + try { + const parsed = AccountsResponseSchema.safeParse(await response.json()) + if (!parsed.success) { + console.error('Cloudflare API /accounts payload did not match expected shape', parsed.error) + return { accounts: [] } + } + if (!parsed.data.success || !parsed.data.result) return { accounts: [] } + + const accounts = parsed.data.result + const reported = parsed.data.result_info?.total_count + if (reported !== undefined && Number.isFinite(reported) && reported >= 0) { + return { accounts, totalCount: reported } + } + + const pageCount = parsed.data.result_info?.count + const totalCount = + pageCount !== undefined && Number.isFinite(pageCount) && pageCount >= 0 + ? Math.max(pageCount, accounts.length) + : accounts.length + console.warn( + `Cloudflare API /accounts response is missing a valid result_info.total_count; ` + + `falling back to page count ${totalCount}` + ) + return { accounts, totalCount } + } catch (error) { + console.error('Cloudflare API /accounts response is not valid JSON', error) + return { accounts: [] } + } +} + +function accountIdentity(page: AccountPage): CloudflareIdentity { + if (page.accounts.length !== 1 || (page.totalCount !== undefined && page.totalCount !== 1)) { + throw new OAuthError( + 'invalid_token', + 'Account token must resolve to exactly one Cloudflare account', + 401 + ) + } + return { type: 'account', account: page.accounts[0] } +} + +function userIdentity( + user: CloudflareUserIdentity['user'], + page: AccountPage +): CloudflareUserIdentity { + if (page.totalCount !== undefined && page.totalCount > MAX_STORED_ACCOUNTS) { + return { type: 'user', user, accounts: [], accountCount: page.totalCount } + } + return { type: 'user', user, accounts: page.accounts } +} + +async function resolveAccountCredential( + accessToken: string, + caller: string +): Promise { + // One minimum-size page is enough to prove whether an account-owned token + // resolves to exactly one account without fetching the larger user page. + const accountsResponse = await fetchAccounts(accessToken, caller, ACCOUNT_TOKEN_PROBE_PAGE_SIZE) + if (!accountsResponse.ok) throwIdentityProbeError(accountsResponse) + return accountIdentity(await parseAccounts(accountsResponse)) +} + +async function resolveUserCredential( + accessToken: string, + caller: string +): Promise { + const [userResponse, accountsResponse] = await Promise.all([ + fetchUser(accessToken, caller), + fetchAccounts(accessToken, caller) + ]) + const failures = [userResponse, accountsResponse].filter((response) => !response.ok) + if (failures.length > 0) { + throwIdentityProbeError(failures[0], ...failures.slice(1)) + } + + const [user, accounts] = await Promise.all([ + parseUser(userResponse), + parseAccounts(accountsResponse) + ]) + if (!user) { + throw new OAuthError('invalid_token', 'Failed to verify token: no user information', 401) + } + return userIdentity(user, accounts) +} + +async function resolveLegacyCredential( + accessToken: string, + caller: string +): Promise { + const [userResponse, accountsResponse] = await Promise.all([ + fetchUser(accessToken, caller), + fetchAccounts(accessToken, caller) + ]) + if (!accountsResponse.ok) { + if (!userResponse.ok) throwIdentityProbeError(userResponse, accountsResponse) + throwIdentityProbeError(accountsResponse) + } + + const [user, accounts] = await Promise.all([ + userResponse.ok ? parseUser(userResponse) : Promise.resolve(null), + accountsResponse.ok ? parseAccounts(accountsResponse) : Promise.resolve({ accounts: [] }) + ]) + if (user) return userIdentity(user, accounts) + + // A transient /user failure cannot prove account ownership. + if (!userResponse.ok && userResponse.status >= 429) throwIdentityProbeError(userResponse) + return accountIdentity(accounts) +} + +/** Resolve a direct Cloudflare API or OAuth credential using its ownership hint. */ +export function resolveCloudflareCredential( + accessToken: string, + tokenOwner: CloudflareTokenOwner +): Promise { + switch (tokenOwner) { + case 'account': + return resolveAccountCredential(accessToken, API_TOKEN_IDENTITY_CALLER) + case 'user': + return resolveUserCredential(accessToken, API_TOKEN_IDENTITY_CALLER) + case 'unknown': + return resolveLegacyCredential(accessToken, API_TOKEN_IDENTITY_CALLER) + } +} + +/** Resolve the upstream Cloudflare OAuth grant used by this server's OAuth flow. */ +export async function getCloudflareOAuthUser(accessToken: string): Promise { + const [userResponse, accountsResponse] = await Promise.all([ + fetchUser(accessToken, OAUTH_IDENTITY_CALLER), + fetchAccounts(accessToken, OAUTH_IDENTITY_CALLER) + ]) + if (!userResponse.ok && !accountsResponse.ok) { + throwIdentityProbeError(userResponse, accountsResponse) + } + + const [user, accounts] = await Promise.all([ + userResponse.ok ? parseUser(userResponse) : Promise.resolve(null), + accountsResponse.ok ? parseAccounts(accountsResponse) : Promise.resolve({ accounts: [] }) + ]) + if (!user) { + throw new OAuthError('server_error', 'Failed to fetch user information from Cloudflare', 500) + } + return userIdentity(user, accounts) +} diff --git a/src/auth/oauth-handler.ts b/src/auth/oauth-handler.ts index 8418676..1f84e66 100644 --- a/src/auth/oauth-handler.ts +++ b/src/auth/oauth-handler.ts @@ -8,16 +8,7 @@ import { refreshAuthToken } from './cloudflare-auth' import { DEFAULT_TEMPLATE, REQUIRED_SCOPES, SCOPE_DEFINITIONS, SCOPE_TEMPLATES } from './scopes' -import { - UserSchema, - AccountsSchema, - AuthProps as AuthPropsSchema, - AUTH_PROPS_VERSION, - ACCOUNTS_PROBE_PAGE_SIZE, - MAX_STORED_ACCOUNTS, - type AuthProps, - type AccountSchema -} from './types' +import { AuthProps as AuthPropsSchema, AUTH_PROPS_VERSION, type AuthProps } from './types' import { clientIdAlreadyApproved, createOAuthState, @@ -29,7 +20,7 @@ import { validateOAuthState, OAuthError } from './workers-oauth-utils' -import { fetchWithRetry } from '../utils/fetch-retry' +import { getCloudflareOAuthUser } from './cloudflare-identity' import { MetricsTracker, AuthUser } from '../metrics' import { SERVER_INFO } from '../constants' @@ -309,156 +300,6 @@ export async function guardRefreshTokenExchange( return refreshPromise } -function getRetryAfterHeader(...responses: Response[]): Record { - return { - 'Retry-After': - responses.find((response) => response.status === 429)?.headers.get('Retry-After') ?? '30' - } -} - -function throwCombinedCloudflareApiError(userResp: Response, accountsResp: Response): never { - const statuses = [userResp.status, accountsResp.status] - - if (statuses.some((status) => status >= 500)) { - throw new OAuthError('server_error', 'Cloudflare API is temporarily unavailable', 502) - } - - if (statuses.includes(429)) { - throw new OAuthError('temporarily_unavailable', 'Rate limited, try again later', 429, { - ...getRetryAfterHeader(userResp, accountsResp) - }) - } - - if (statuses.includes(401)) { - throw new OAuthError('invalid_token', 'Access token is invalid or expired', 401) - } - - if (statuses.includes(403)) { - throw new OAuthError('insufficient_scope', 'Insufficient permissions', 403) - } - - throw new OAuthError('invalid_token', 'Failed to verify token', userResp.status) -} - -async function fetchCloudflareProbes( - accessToken: string, - caller = 'oauth_callback_identity_probe' -): Promise<[Response, Response]> { - const headers = { Authorization: `Bearer ${accessToken}` } - - try { - return await Promise.all([ - fetchWithRetry(`${env.CLOUDFLARE_API_BASE}/user`, { headers }, { caller }), - fetchWithRetry( - `${env.CLOUDFLARE_API_BASE}/accounts?per_page=${ACCOUNTS_PROBE_PAGE_SIZE}`, - { headers }, - { caller } - ) - ]) - } catch (error) { - console.error('Cloudflare API request failed', error) - throw new OAuthError('server_error', 'Cloudflare API is temporarily unavailable', 502) - } -} - -/** - * Fetch user and accounts from Cloudflare API - */ -export async function getUserAndAccounts( - accessToken: string, - caller = 'oauth_callback_identity_probe' -): Promise<{ - user: UserSchema | null - accounts: AccountSchema[] - accountCount?: number -}> { - const [userResp, accountsResp] = await fetchCloudflareProbes(accessToken, caller) - - // Check for upstream errors before parsing - if (!userResp.ok && !accountsResp.ok) { - console.error(`Cloudflare API error: user=${userResp.status}, accounts=${accountsResp.status}`) - throwCombinedCloudflareApiError(userResp, accountsResp) - } - - // Parse user from response - let user: UserSchema | null = null - if (userResp.ok) { - try { - const json = (await userResp.json()) as { success?: boolean; result?: unknown } - if (json.success && json.result) { - const parsed = UserSchema.safeParse(json.result) - if (parsed.success) { - user = parsed.data - } else { - console.error('Cloudflare API /user payload did not match expected shape', parsed.error) - } - } - } catch (error) { - console.error('Cloudflare API /user response is not valid JSON', error) - } - } - - // Parse accounts from response - let accounts: AccountSchema[] = [] - let totalAccountCount: number | undefined - if (accountsResp.ok) { - try { - const json = (await accountsResp.json()) as { - success?: boolean - result?: unknown - result_info?: { count?: unknown; total_count?: unknown } - } - if (json.success && json.result) { - const parsed = AccountsSchema.safeParse(json.result) - if (parsed.success) { - accounts = parsed.data - const reported = json.result_info?.total_count - if (typeof reported === 'number' && Number.isFinite(reported) && reported >= 0) { - totalAccountCount = reported - } else { - const pageCount = json.result_info?.count - totalAccountCount = - typeof pageCount === 'number' && Number.isFinite(pageCount) && pageCount >= 0 - ? Math.max(pageCount, accounts.length) - : accounts.length - console.warn( - `Cloudflare API /accounts response is missing a valid result_info.total_count; ` + - `falling back to page count ${totalAccountCount}` - ) - } - } else { - console.error( - 'Cloudflare API /accounts payload did not match expected shape', - parsed.error - ) - } - } - } catch (error) { - console.error('Cloudflare API /accounts response is not valid JSON', error) - } - } - - if (user) { - // Don't persist a long account list. Prefer the API-reported total; the - // page count fallback preserves availability if pagination metadata drifts. - if (totalAccountCount !== undefined && totalAccountCount > MAX_STORED_ACCOUNTS) { - return { user, accounts: [], accountCount: totalAccountCount } - } - return { user, accounts } - } - - // Account-scoped token - user will be null - if (accounts.length > 0) { - return { user: null, accounts } - } - - throw new OAuthError( - 'invalid_token', - 'Failed to verify token: no user or account information', - 401 - ) -} - /** * Handle token refresh for workers-oauth-provider. * @@ -703,36 +544,26 @@ export function createAuthHandlers() { }) ]) - // Fetch user and accounts - const { user, accounts, accountCount } = await getUserAndAccounts(access_token) - - // Account-scoped tokens (user: null) are only supported via API token mode - // (see api-token-mode.ts). The OAuth flow always requires a user identity. - if (!user) { - return new OAuthError( - 'server_error', - 'Failed to fetch user information from Cloudflare' - ).toHtmlResponse() - } + const identity = await getCloudflareOAuthUser(access_token) // Complete authorization const { redirectTo } = await env.OAUTH_PROVIDER.completeAuthorization({ request: oauthReqInfo, - userId: user.id, - metadata: { label: user.email }, + userId: identity.user.id, + metadata: { label: identity.user.email }, scope: oauthReqInfo.scope, props: { type: 'user_token', - user, - accounts, - accountCount, + user: identity.user, + accounts: identity.accounts, + accountCount: identity.accountCount, version: AUTH_PROPS_VERSION, accessToken: access_token, refreshToken: refresh_token } satisfies AuthProps }) - metrics.logEvent(new AuthUser({ userId: user.id })) + metrics.logEvent(new AuthUser({ userId: identity.user.id })) return new Response(null, { status: 302, diff --git a/src/index.ts b/src/index.ts index ae5dba9..7f7f8f9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,10 +3,9 @@ import OAuthProvider, { type OAuthProviderOptions } from '@cloudflare/workers-oauth-provider' import { createAuthHandlers, handleTokenExchangeCallback } from './auth/oauth-handler' -import { isDirectApiToken, handleApiTokenRequest } from './auth/api-token-mode' +import { resolveExternalToken } from './auth/api-token-mode' import { MCP_ROUTE, - handleAuthenticatedMcpRequest, handleMcpPreflight, oauthMcpHandler, rejectInvalidMcpRequest @@ -34,15 +33,8 @@ export default { if (request.method === 'OPTIONS') return handleMcpPreflight(request) } - // Check for direct API token first (like GitHub MCP's PAT support) - if (isMcpRoute && isDirectApiToken(request)) { - const response = await handleApiTokenRequest(request, (props) => - handleAuthenticatedMcpRequest(request, props) - ) - if (response) return response - } - - // OAuth mode - handle via workers-oauth-provider + // workers-oauth-provider resolves its own access tokens first, then delegates + // direct Cloudflare API/OAuth credentials to resolveExternalToken. const oauthOptions: OAuthProviderOptions = { apiHandlers: { [MCP_ROUTE]: oauthMcpHandler @@ -52,6 +44,7 @@ export default { authorizeEndpoint: '/authorize', tokenEndpoint: '/token', clientRegistrationEndpoint: '/register', + resolveExternalToken, tokenExchangeCallback: (options) => handleTokenExchangeCallback( options, diff --git a/tests/auth/api-token-mode.test.ts b/tests/auth/api-token-mode.test.ts index f31d509..8d25938 100644 --- a/tests/auth/api-token-mode.test.ts +++ b/tests/auth/api-token-mode.test.ts @@ -1,203 +1,343 @@ -import { env } from 'cloudflare:workers' +import { env, exports } from 'cloudflare:workers' +import { ExternalTokenError } from '@cloudflare/workers-oauth-provider' import { http, HttpResponse } from 'msw' -import { describe, it, expect, vi, afterEach } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { - isDirectApiToken, - extractBearerToken, buildAuthProps, - handleApiTokenRequest + cloudflareTokenOwner, + resolveExternalToken } from '../../src/auth/api-token-mode' import { AUTH_PROPS_VERSION } from '../../src/auth/types' import { API_BASE, cfAccountsSuccess, cfError, cfSuccess } from '../helpers/cloudflare-api' import { clearKv } from '../helpers/kv' +import { MCP_URL, modernMcpRequest } from '../helpers/mcp' import { server } from '../setup/msw' -/** - * Helper to create a mock Request with given Authorization header - */ -function mockRequest(authHeader?: string): Request { - const headers = new Headers() - if (authHeader) { - headers.set('Authorization', authHeader) +vi.mock('../../src/utils/fetch-retry', async (importOriginal) => { + const original = await importOriginal() + return { + ...original, + fetchWithRetry: (input: RequestInfo, init?: RequestInit) => + original.fetchWithRetry(input, init, { maxRetries: 0 }) + } +}) + +const USER = { id: 'user-1', email: 'user@example.com' } +const ACCOUNT = { id: 'account-1', name: 'Account One' } + +function resolverInput(token: string) { + return { + token, + request: new Request(MCP_URL), + env } - return new Request('https://example.com', { headers }) } -afterEach(() => clearKv(env.OAUTH_KV)) +async function sha256Hex(value: string): Promise { + const digest = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(value)) + return [...new Uint8Array(digest)].map((byte) => byte.toString(16).padStart(2, '0')).join('') +} -describe('isDirectApiToken', () => { - it('should return false for requests without Authorization header', () => { - const request = mockRequest() - expect(isDirectApiToken(request)).toBe(false) - }) +function mockIdentity(opts: { user?: () => Response; accounts?: () => Response }): { + userCalls: () => number + accountCalls: () => number +} { + let userCalls = 0 + let accountCalls = 0 + if (opts.user) { + server.use( + http.get(`${API_BASE}/user`, () => { + userCalls++ + return opts.user!() + }) + ) + } + if (opts.accounts) { + server.use( + http.get(`${API_BASE}/accounts`, () => { + accountCalls++ + return opts.accounts!() + }) + ) + } + return { userCalls: () => userCalls, accountCalls: () => accountCalls } +} - it('should return false for non-Bearer auth schemes', () => { - const request = mockRequest('Basic dXNlcjpwYXNz') - expect(isDirectApiToken(request)).toBe(false) - }) +async function expectExternalTokenError( + promise: Promise, + code: string, + statusCode: number +): Promise { + try { + await promise + throw new Error('Expected ExternalTokenError') + } catch (error) { + expect(error).toBeInstanceOf(ExternalTokenError) + expect(error).toMatchObject({ code, statusCode }) + return error as ExternalTokenError + } +} - it('should return false for OAuth tokens (3 colon-separated parts)', () => { - // OAuth tokens from workers-oauth-provider have format: userId:grantId:secret - const request = mockRequest('Bearer user123:grant456:secretabc') - expect(isDirectApiToken(request)).toBe(false) - }) +afterEach(() => clearKv(env.OAUTH_KV)) - it('should return true for Cloudflare API tokens (no colons)', () => { - // Cloudflare API tokens are typically long alphanumeric strings - const request = mockRequest('Bearer abcdef1234567890abcdef1234567890') - expect(isDirectApiToken(request)).toBe(true) +describe('Cloudflare token ownership', () => { + it.each([ + ['cfat_account-token', 'account'], + ['cfut_user-token', 'user'], + ['cfoat_wrangler-token', 'user'], + ['legacy-unprefixed-token', 'unknown'] + ] as const)('classifies %s as %s', (token, owner) => { + expect(cloudflareTokenOwner(token)).toBe(owner) }) +}) - it('should return true for tokens with 1 colon (not OAuth format)', () => { - const request = mockRequest('Bearer part1:part2') - expect(isDirectApiToken(request)).toBe(true) +describe('buildAuthProps', () => { + it('builds versioned user props', () => { + expect( + buildAuthProps('token', { + type: 'user', + user: USER, + accounts: [ACCOUNT], + accountCount: 1 + }) + ).toEqual({ + type: 'user_token', + accessToken: 'token', + user: USER, + accounts: [ACCOUNT], + accountCount: 1, + version: AUTH_PROPS_VERSION + }) }) - it('should return true for tokens with 4+ colons (not OAuth format)', () => { - const request = mockRequest('Bearer a:b:c:d:e') - expect(isDirectApiToken(request)).toBe(true) + it('builds account props from an account identity', () => { + expect(buildAuthProps('token', { type: 'account', account: ACCOUNT })).toEqual({ + type: 'account_token', + accessToken: 'token', + account: ACCOUNT + }) }) }) -describe('extractBearerToken', () => { - it('should return null for requests without Authorization header', () => { - const request = mockRequest() - expect(extractBearerToken(request)).toBeNull() - }) +describe('resolveExternalToken', () => { + it('uses only /accounts for prefixed account tokens', async () => { + const calls = mockIdentity({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) - it('should return null for non-Bearer auth schemes', () => { - const request = mockRequest('Basic dXNlcjpwYXNz') - expect(extractBearerToken(request)).toBeNull() + await expect(resolveExternalToken(resolverInput('cfat_account-token'))).resolves.toEqual({ + props: { + type: 'account_token', + accessToken: 'cfat_account-token', + account: ACCOUNT + } + }) + expect(calls.userCalls()).toBe(0) + expect(calls.accountCalls()).toBe(1) }) - it('should extract token from valid Bearer header', () => { - const request = mockRequest('Bearer my-secret-token') - expect(extractBearerToken(request)).toBe('my-secret-token') - }) + it.each(['cfut_user-token', 'cfoat_wrangler-token'])( + 'uses /user and /accounts for prefixed user credential %s', + async (token) => { + const calls = mockIdentity({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) - it('should handle tokens with special characters', () => { - const request = mockRequest('Bearer abc:def:ghi') - expect(extractBearerToken(request)).toBe('abc:def:ghi') - }) + await expect(resolveExternalToken(resolverInput(token))).resolves.toMatchObject({ + props: { type: 'user_token', accessToken: token, user: USER, accounts: [ACCOUNT] } + }) + expect(calls.userCalls()).toBe(1) + expect(calls.accountCalls()).toBe(1) + } + ) + + it('retains response-based account inference for legacy credentials', async () => { + const calls = mockIdentity({ + user: () => HttpResponse.json(cfError([], null), { status: 403 }), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) - it('should handle tokens with spaces after Bearer', () => { - // "Bearer token" - double space, should return " token" - const request = mockRequest('Bearer token-with-leading-space') - expect(extractBearerToken(request)).toBe(' token-with-leading-space') + await expect( + resolveExternalToken(resolverInput('legacy-account-token')) + ).resolves.toMatchObject({ + props: { type: 'account_token', account: ACCOUNT } + }) + expect(calls.userCalls()).toBe(1) + expect(calls.accountCalls()).toBe(1) }) -}) -describe('buildAuthProps', () => { - const mockToken = 'test-token-123' - const mockUser = { id: 'user-1', email: 'test@example.com' } - const mockAccounts = [ - { id: 'acc-1', name: 'Account One' }, - { id: 'acc-2', name: 'Account Two' } - ] + it('does not infer account ownership for a known user token', async () => { + mockIdentity({ + user: () => HttpResponse.json(cfError([], null), { status: 403 }), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) - it('should build user_token props when user is provided', () => { - const props = buildAuthProps(mockToken, mockUser, mockAccounts) + const error = await expectExternalTokenError( + resolveExternalToken(resolverInput('cfut_user-token-no-user-scope')), + 'insufficient_scope', + 403 + ) + expect(error.requiredScopes).toEqual(['user:read', 'account:read']) + }) - expect(props).toEqual({ - type: 'user_token', - accessToken: mockToken, - user: mockUser, - accounts: mockAccounts, - accountCount: undefined, - version: AUTH_PROPS_VERSION + it('uses account-only scope guidance for account token failures', async () => { + const calls = mockIdentity({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => HttpResponse.json(cfError([], null), { status: 403 }) }) - }) - it('should build user_token props with empty accounts if not provided', () => { - const props = buildAuthProps(mockToken, mockUser) + const error = await expectExternalTokenError( + resolveExternalToken(resolverInput('cfat_account-token-no-read-scope')), + 'insufficient_scope', + 403 + ) + expect(error.requiredScopes).toEqual(['account:read']) + expect(calls.userCalls()).toBe(0) + expect(calls.accountCalls()).toBe(1) + }) - expect(props).toEqual({ - type: 'user_token', - accessToken: mockToken, - user: mockUser, - accounts: [], - accountCount: undefined, - version: AUTH_PROPS_VERSION + it('caches a verified identity by token hash', async () => { + const calls = mockIdentity({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) }) + const input = resolverInput('cfut_cached-user-token') + + await resolveExternalToken(input) + await resolveExternalToken(input) + + expect(calls.userCalls()).toBe(1) + expect(calls.accountCalls()).toBe(1) }) - it('should build account_token props when no user but has accounts', () => { - const props = buildAuthProps(mockToken, null, mockAccounts) + it('ignores malformed cached identity data and revalidates upstream', async () => { + const token = 'cfut_invalid-cache-token' + await env.OAUTH_KV.put( + `api-token-identity:v4:${await sha256Hex(token)}`, + JSON.stringify({ user: 'not-an-object', accounts: [] }) + ) + const calls = mockIdentity({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) - expect(props).toEqual({ - type: 'account_token', - accessToken: mockToken, - account: mockAccounts[0] // Uses first account + await expect(resolveExternalToken(resolverInput(token))).resolves.toMatchObject({ + props: { type: 'user_token', user: USER, accounts: [ACCOUNT] } }) + expect(calls.userCalls()).toBe(1) + expect(calls.accountCalls()).toBe(1) }) - it('should throw error when no user and no accounts', () => { - expect(() => buildAuthProps(mockToken, null, [])).toThrow( - 'Cannot build auth props: no user or account information' + it('maps malformed credentials to invalid_token', async () => { + mockIdentity({ + user: () => HttpResponse.json(cfError([], null), { status: 400 }), + accounts: () => HttpResponse.json(cfError([], null), { status: 400 }) + }) + + const error = await expectExternalTokenError( + resolveExternalToken(resolverInput('malformed-token')), + 'invalid_token', + 401 ) + expect(error.description).toContain('appears malformed') }) - it('should throw error when no user and accounts undefined', () => { - expect(() => buildAuthProps(mockToken, null, undefined)).toThrow( - 'Cannot build auth props: no user or account information' + it('preserves rate-limit backoff', async () => { + mockIdentity({ + user: () => + HttpResponse.json(cfError([], null), { + status: 429, + headers: { 'Retry-After': '17' } + }), + accounts: () => HttpResponse.json(cfError([], null), { status: 429 }) + }) + + const error = await expectExternalTokenError( + resolveExternalToken(resolverInput('cfoat_rate-limited-token')), + 'temporarily_unavailable', + 429 ) + expect(error.headers).toEqual({ 'Retry-After': '17' }) }) - it('should treat undefined user same as null', () => { - const props = buildAuthProps(mockToken, undefined, mockAccounts) + it('maps upstream failures to server_error', async () => { + mockIdentity({ + user: () => HttpResponse.json(cfError([], null), { status: 502 }), + accounts: () => HttpResponse.json(cfError([], null), { status: 401 }) + }) - expect(props.type).toBe('account_token') + await expectExternalTokenError( + resolveExternalToken(resolverInput('legacy-upstream-failure')), + 'server_error', + 502 + ) }) }) -describe('handleApiTokenRequest identity probe caching', () => { - const token = 'api-token-123' - const tokenHash = '9bdb81d121b42d1c7819c816fa3cfbb6ee109726f9ed2475edb169374881d7b3' - const cacheKey = `api-token-identity:v2:${tokenHash}` - const user = { id: 'user-1', email: 'test@example.com' } - const accounts = [{ id: 'acc-1', name: 'Account One' }] - - // Count upstream identity probes so we can assert cache hits/misses without - // mocking our own getUserAndAccounts — the REAL function runs against MSW. - function mockUserProbe(): () => number { - let calls = 0 - server.use( - http.get(`${API_BASE}/user`, () => { - calls++ - return HttpResponse.json(cfSuccess(user)) - }), - http.get(`${API_BASE}/accounts`, () => HttpResponse.json(cfAccountsSuccess(accounts))) +describe('external token resource responses', () => { + it('lets the provider construct the RFC 6750/9728 invalid-token response', async () => { + mockIdentity({ + user: () => HttpResponse.json(cfError([], null), { status: 400 }), + accounts: () => HttpResponse.json(cfError([], null), { status: 400 }) + }) + + const response = await exports.default.fetch( + modernMcpRequest('malformed-provider-token', 'server/discover') ) - return () => calls - } - it('stores API token identity lookups in KV by token hash', async () => { - const probeCalls = mockUserProbe() - const createMcpResponse = vi.fn().mockResolvedValue(new Response('ok')) + expect(response.status).toBe(401) + expect(response.headers.get('cache-control')).toBe('no-store') + expect(response.headers.get('www-authenticate')).toContain('error="invalid_token"') + expect(response.headers.get('www-authenticate')).toContain( + `resource_metadata="${new URL(MCP_URL).origin}/.well-known/oauth-protected-resource/mcp"` + ) + await expect(response.json()).resolves.toEqual({ + error: 'invalid_token', + error_description: 'Access token appears malformed; reauthenticate and try again' + }) + }) + + it('lets the provider construct an insufficient-scope challenge', async () => { + mockIdentity({ + user: () => HttpResponse.json(cfError([], null), { status: 403 }), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) - await handleApiTokenRequest(mockRequest(`Bearer ${token}`), createMcpResponse) + const response = await exports.default.fetch( + modernMcpRequest('cfut_provider-insufficient-scope', 'server/discover') + ) - // The real probe ran once and the identity was written to real KV. - expect(probeCalls()).toBe(1) - expect(await env.OAUTH_KV.get(cacheKey, 'json')).toEqual({ user, accounts }) - expect(createMcpResponse).toHaveBeenCalledWith(buildAuthProps(token, user, accounts)) + expect(response.status).toBe(403) + expect(response.headers.get('www-authenticate')).toContain('error="insufficient_scope"') + expect(response.headers.get('www-authenticate')).toContain('scope="user:read account:read"') + await expect(response.json()).resolves.toEqual({ + error: 'insufficient_scope', + error_description: 'Token lacks required user:read or account:read scope' + }) }) - it('uses cached API token identity from KV without probing upstream', async () => { - // Seed the real cache; any upstream /user hit would throw (unhandled). - await env.OAUTH_KV.put(cacheKey, JSON.stringify({ user, accounts })) - let probed = false - server.use( - http.get(`${API_BASE}/user`, () => { - probed = true - return HttpResponse.json(cfError([], null)) - }) - ) - const createMcpResponse = vi.fn().mockResolvedValue(new Response('ok')) + it('lets the provider preserve a retryable rate-limit response', async () => { + mockIdentity({ + user: () => + HttpResponse.json(cfError([], null), { + status: 429, + headers: { 'Retry-After': '23' } + }), + accounts: () => HttpResponse.json(cfError([], null), { status: 429 }) + }) - await handleApiTokenRequest(mockRequest(`Bearer ${token}`), createMcpResponse) + const response = await exports.default.fetch( + modernMcpRequest('cfoat_provider-rate-limit', 'server/discover') + ) - expect(probed).toBe(false) - expect(createMcpResponse).toHaveBeenCalledWith(buildAuthProps(token, user, accounts)) + expect(response.status).toBe(429) + expect(response.headers.get('retry-after')).toBe('23') + expect(response.headers.get('cache-control')).toBe('no-store') + await expect(response.json()).resolves.toEqual({ + error: 'temporarily_unavailable', + error_description: 'Rate limited, try again later' + }) }) }) diff --git a/tests/auth/cloudflare-identity.test.ts b/tests/auth/cloudflare-identity.test.ts new file mode 100644 index 0000000..9c53219 --- /dev/null +++ b/tests/auth/cloudflare-identity.test.ts @@ -0,0 +1,331 @@ +import { OAuthError as ProviderOAuthError } from '@cloudflare/workers-oauth-provider' +import { http, HttpResponse } from 'msw' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { + getCloudflareOAuthUser, + resolveCloudflareCredential +} from '../../src/auth/cloudflare-identity' +import { OAuthError } from '../../src/auth/workers-oauth-utils' +import { API_BASE, cfAccountsSuccess, cfSuccess } from '../helpers/cloudflare-api' +import { server } from '../setup/msw' + +const USER = { id: 'user-1', email: 'user@example.com' } +const ACCOUNT = { id: 'account-1', name: 'Account One' } + +function mockProbes(opts: { user?: () => Response; accounts?: () => Response }) { + if (opts.user) server.use(http.get(`${API_BASE}/user`, opts.user)) + if (opts.accounts) server.use(http.get(`${API_BASE}/accounts`, opts.accounts)) +} + +vi.mock('../../src/utils/fetch-retry', async (importOriginal) => { + const original = await importOriginal() + return { + ...original, + fetchWithRetry: (input: RequestInfo, init?: RequestInit) => + original.fetchWithRetry(input, init, { maxRetries: 0 }) + } +}) + +async function expectOAuthError( + promise: Promise, + code: string, + statusCode: number +): Promise { + try { + await promise + throw new Error('Expected OAuthError to be thrown') + } catch (error) { + expect(error).toBeInstanceOf(OAuthError) + expect(error).toBeInstanceOf(ProviderOAuthError) + expect(error).toMatchObject({ code, statusCode }) + return error as OAuthError + } +} + +afterEach(() => vi.restoreAllMocks()) + +describe('resolveCloudflareCredential', () => { + it('resolves an account-owned token with one minimum-size /accounts probe', async () => { + let userCalls = 0 + let accountsUrl = '' + mockProbes({ + user: () => { + userCalls++ + return HttpResponse.json(cfSuccess(USER)) + } + }) + server.use( + http.get(`${API_BASE}/accounts`, ({ request }) => { + accountsUrl = request.url + return HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) + ) + + await expect(resolveCloudflareCredential('cfat_token', 'account')).resolves.toEqual({ + type: 'account', + account: ACCOUNT + }) + expect(userCalls).toBe(0) + expect(new URL(accountsUrl).searchParams.get('per_page')).toBe('5') + }) + + it('requires both identity probes for a known user credential', async () => { + mockProbes({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) + + await expect(resolveCloudflareCredential('cfut_token', 'user')).resolves.toEqual({ + type: 'user', + user: USER, + accounts: [ACCOUNT] + }) + }) + + it('does not infer account ownership for a known user credential', async () => { + mockProbes({ + user: () => new HttpResponse('Forbidden', { status: 403 }), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) + + await expectOAuthError( + resolveCloudflareCredential('cfut_token', 'user'), + 'insufficient_scope', + 403 + ) + }) + + it.each([ + ['forbidden user probe', () => new HttpResponse('Forbidden', { status: 403 })], + ['invalid user JSON', () => new HttpResponse('not-json', { status: 200 })], + ['unsuccessful user envelope', () => HttpResponse.json({ success: false })] + ] as const)('retains legacy account inference for an %s', async (_, userResponse) => { + mockProbes({ + user: userResponse, + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) + + await expect(resolveCloudflareCredential('legacy-token', 'unknown')).resolves.toEqual({ + type: 'account', + account: ACCOUNT + }) + }) + + it('requires account discovery even when a legacy user probe succeeds', async () => { + mockProbes({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => new HttpResponse('Forbidden', { status: 403 }) + }) + + await expectOAuthError( + resolveCloudflareCredential('legacy-token', 'unknown'), + 'insufficient_scope', + 403 + ) + }) + + it('never infers account ownership from a transient /user failure', async () => { + mockProbes({ + user: () => new HttpResponse('rate limited', { status: 429 }), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) + + await expectOAuthError( + resolveCloudflareCredential('legacy-token', 'unknown'), + 'temporarily_unavailable', + 429 + ) + }) + + it('requires an account token to resolve to exactly one account', async () => { + mockProbes({ accounts: () => HttpResponse.json(cfAccountsSuccess([])) }) + + await expectOAuthError( + resolveCloudflareCredential('cfat_token', 'account'), + 'invalid_token', + 401 + ) + }) + + it('rejects a truncated account-token page whose total count exceeds one', async () => { + mockProbes({ + accounts: () => + HttpResponse.json({ + ...cfSuccess([ACCOUNT]), + result_info: { page: 1, per_page: 5, count: 1, total_count: 2 } + }) + }) + + await expectOAuthError( + resolveCloudflareCredential('cfat_token', 'account'), + 'invalid_token', + 401 + ) + }) +}) + +describe('user account metadata', () => { + it('requests one account past the prompt-list cutoff', async () => { + let accountsUrl = '' + mockProbes({ user: () => HttpResponse.json(cfSuccess(USER)) }) + server.use( + http.get(`${API_BASE}/accounts`, ({ request }) => { + accountsUrl = request.url + return HttpResponse.json(cfAccountsSuccess([])) + }) + ) + + await resolveCloudflareCredential('cfut_token', 'user') + + expect(new URL(accountsUrl).searchParams.get('per_page')).toBe('31') + }) + + it('stores only the count when the complete account list is too large', async () => { + const accounts = Array.from({ length: 31 }, (_, index) => ({ + id: `account-${index + 1}`, + name: `Account ${index + 1}` + })) + mockProbes({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => + HttpResponse.json({ + ...cfSuccess(accounts), + result_info: { page: 1, per_page: 31, count: 31, total_count: 137 } + }) + }) + + await expect(resolveCloudflareCredential('cfut_token', 'user')).resolves.toEqual({ + type: 'user', + user: USER, + accounts: [], + accountCount: 137 + }) + }) + + it('keeps a verified user when an accounts payload is malformed', async () => { + mockProbes({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => new HttpResponse('not-json', { status: 200 }) + }) + + await expect(resolveCloudflareCredential('cfut_token', 'user')).resolves.toEqual({ + type: 'user', + user: USER, + accounts: [] + }) + }) + + it('falls back to the page count when total_count is missing', async () => { + const accounts = Array.from({ length: 31 }, (_, index) => ({ + id: `account-${index + 1}`, + name: `Account ${index + 1}` + })) + mockProbes({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => + HttpResponse.json({ + ...cfSuccess(accounts), + result_info: { page: 1, per_page: 31, count: 31 } + }) + }) + const warning = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + await expect(resolveCloudflareCredential('cfut_token', 'user')).resolves.toEqual({ + type: 'user', + user: USER, + accounts: [], + accountCount: 31 + }) + expect(warning).toHaveBeenCalledWith( + expect.stringContaining('missing a valid result_info.total_count') + ) + }) +}) + +describe('getCloudflareOAuthUser', () => { + it('preserves OAuth availability when /accounts is forbidden', async () => { + mockProbes({ + user: () => HttpResponse.json(cfSuccess(USER)), + accounts: () => new HttpResponse('Forbidden', { status: 403 }) + }) + + await expect(getCloudflareOAuthUser('oauth-token')).resolves.toEqual({ + type: 'user', + user: USER, + accounts: [] + }) + }) + + it('rejects an OAuth result without a user', async () => { + mockProbes({ + user: () => new HttpResponse('Forbidden', { status: 403 }), + accounts: () => HttpResponse.json(cfAccountsSuccess([ACCOUNT])) + }) + + await expectOAuthError(getCloudflareOAuthUser('oauth-token'), 'server_error', 500) + }) +}) + +describe('identity probe errors', () => { + it.each([ + [401, 401, 'invalid_token', 401], + [429, 429, 'temporarily_unavailable', 429], + [500, 500, 'server_error', 502], + [418, 418, 'invalid_token', 418], + [403, 500, 'server_error', 502] + ] as const)( + 'maps /user=%i and /accounts=%i to %s', + async (userStatus, accountsStatus, code, statusCode) => { + mockProbes({ + user: () => new HttpResponse('upstream error', { status: userStatus }), + accounts: () => new HttpResponse('upstream error', { status: accountsStatus }) + }) + + await expectOAuthError( + resolveCloudflareCredential('legacy-token', 'unknown'), + code, + statusCode + ) + } + ) + + it('uses a default Retry-After when the upstream omits it', async () => { + mockProbes({ + user: () => new HttpResponse('rate limited', { status: 429 }), + accounts: () => new HttpResponse('rate limited', { status: 429 }) + }) + + const error = await expectOAuthError( + resolveCloudflareCredential('legacy-token', 'unknown'), + 'temporarily_unavailable', + 429 + ) + expect(error.headers).toEqual({ 'Retry-After': '30' }) + }) + + it('preserves Retry-After from a rate-limited probe', async () => { + mockProbes({ + user: () => + new HttpResponse('rate limited', { status: 429, headers: { 'Retry-After': '17' } }), + accounts: () => new HttpResponse('rate limited', { status: 429 }) + }) + + const error = await expectOAuthError( + resolveCloudflareCredential('legacy-token', 'unknown'), + 'temporarily_unavailable', + 429 + ) + expect(error.headers).toEqual({ 'Retry-After': '17' }) + }) + + it('maps a network failure to server_error', async () => { + vi.spyOn(globalThis, 'fetch').mockRejectedValue(new TypeError('network failed')) + + await expectOAuthError( + resolveCloudflareCredential('legacy-token', 'unknown'), + 'server_error', + 502 + ) + }) +}) diff --git a/tests/auth/oauth-handler.test.ts b/tests/auth/oauth-handler.test.ts index 95aa4c4..f83b942 100644 --- a/tests/auth/oauth-handler.test.ts +++ b/tests/auth/oauth-handler.test.ts @@ -8,32 +8,12 @@ import { http, HttpResponse } from 'msw' import { afterEach, describe, expect, it, vi } from 'vitest' import { - getUserAndAccounts, guardRefreshTokenExchange, handleTokenExchangeCallback } from '../../src/auth/oauth-handler' import { OAuthError } from '../../src/auth/workers-oauth-utils' -import { API_BASE, cfAccountsSuccess, cfSuccess } from '../helpers/cloudflare-api' import { server } from '../setup/msw' -/** Register MSW handlers for the two identity-probe endpoints by path. */ -function mockProbes(opts: { user?: () => Response; accounts?: () => Response }) { - if (opts.user) server.use(http.get(`${API_BASE}/user`, opts.user)) - if (opts.accounts) server.use(http.get(`${API_BASE}/accounts`, opts.accounts)) -} - -// Use minimal retry config so tests don't wait for real backoff delays. This is -// the only mock of our own modules left here: it just tightens retry timing for -// the identity-probe tests; the actual fetch boundary is mocked with MSW. -vi.mock('../../src/utils/fetch-retry', async (importOriginal) => { - const original = await importOriginal() - return { - ...original, - fetchWithRetry: (input: RequestInfo, init?: RequestInit) => - original.fetchWithRetry(input, init, { maxRetries: 0 }) - } -}) - function deferred(): { promise: Promise resolve: (value: T) => void @@ -101,263 +81,6 @@ afterEach(async () => { await Promise.all(keys.map((k) => kv.delete(k.name))) }) -describe('getUserAndAccounts', () => { - it('requests enough accounts for the prompt-list cutoff to be detected', async () => { - let accountsUrl = '' - mockProbes({ - user: () => HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })) - }) - server.use( - http.get(`${API_BASE}/accounts`, ({ request }) => { - accountsUrl = request.url - return HttpResponse.json(cfAccountsSuccess([])) - }) - ) - - await getUserAndAccounts('test-token') - - expect(new URL(accountsUrl).searchParams.get('per_page')).toBe('31') - }) - - it('records the count but omits the records when a user has too many accounts', async () => { - const accounts = Array.from({ length: 31 }, (_, index) => ({ - id: `account-${index + 1}`, - name: `Account ${index + 1}` - })) - mockProbes({ - user: () => HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })), - accounts: () => - HttpResponse.json({ - ...cfSuccess(accounts), - result_info: { page: 1, per_page: 31, count: 31, total_count: 137 } - }) - }) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: { id: 'user-1', email: 'user@example.com' }, - accounts: [], - accountCount: 137 - }) - }) - - it('warns and falls back to page count when total_count is missing', async () => { - const accounts = Array.from({ length: 31 }, (_, index) => ({ - id: `account-${index + 1}`, - name: `Account ${index + 1}` - })) - mockProbes({ - user: () => HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })), - accounts: () => - HttpResponse.json({ - ...cfSuccess(accounts), - result_info: { page: 1, per_page: 31, count: 31 } - }) - }) - const warning = vi.spyOn(console, 'warn').mockImplementation(() => {}) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: { id: 'user-1', email: 'user@example.com' }, - accounts: [], - accountCount: 31 - }) - expect(warning).toHaveBeenCalledWith( - expect.stringContaining('missing a valid result_info.total_count') - ) - }) - - it('keeps the full list when a user is under the cutoff', async () => { - const accounts = Array.from({ length: 25 }, (_, index) => ({ - id: `account-${index + 1}`, - name: `Account ${index + 1}` - })) - mockProbes({ - user: () => HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })), - accounts: () => - HttpResponse.json({ - ...cfSuccess(accounts), - result_info: { page: 1, per_page: 31, count: 25, total_count: 25 } - }) - }) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: { id: 'user-1', email: 'user@example.com' }, - accounts - }) - }) - - it('accepts account-scoped token when /user fails but /accounts succeeds', async () => { - mockProbes({ - user: () => new HttpResponse('Forbidden', { status: 403 }), - accounts: () => - HttpResponse.json(cfAccountsSuccess([{ id: 'acc-1', name: 'Primary Account' }])) - }) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: null, - accounts: [{ id: 'acc-1', name: 'Primary Account' }] - }) - }) - - it('accepts user tokens when /accounts fails but /user succeeds', async () => { - mockProbes({ - user: () => HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })), - accounts: () => new HttpResponse('Forbidden', { status: 403 }) - }) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: { id: 'user-1', email: 'user@example.com' }, - accounts: [] - }) - }) - - it('throws insufficient_scope when both endpoints fail with 403', async () => { - mockProbes({ - user: () => new HttpResponse('Forbidden', { status: 403 }), - accounts: () => new HttpResponse('Forbidden', { status: 403 }) - }) - - await expectOAuthError(getUserAndAccounts('test-token'), 'insufficient_scope', 403) - }) - - it.each([ - { - userStatus: 401, - accountsStatus: 401, - code: 'invalid_token', - statusCode: 401 - }, - { - userStatus: 429, - accountsStatus: 429, - code: 'temporarily_unavailable', - statusCode: 429 - }, - { - userStatus: 500, - accountsStatus: 500, - code: 'server_error', - statusCode: 502 - }, - { - userStatus: 418, - accountsStatus: 418, - code: 'invalid_token', - statusCode: 418 - }, - { - userStatus: 403, - accountsStatus: 500, - code: 'server_error', - statusCode: 502 - } - ])( - 'maps dual endpoint failures to OAuthError for /user=$userStatus /accounts=$accountsStatus', - async ({ userStatus, accountsStatus, code, statusCode }) => { - mockProbes({ - user: () => new HttpResponse('upstream error', { status: userStatus }), - accounts: () => new HttpResponse('upstream error', { status: accountsStatus }) - }) - - await expectOAuthError(getUserAndAccounts('test-token'), code, statusCode) - } - ) - - it('preserves Retry-After from Cloudflare API 429 responses', async () => { - mockProbes({ - user: () => - new HttpResponse('rate limited', { status: 429, headers: { 'Retry-After': '17' } }), - accounts: () => new HttpResponse('rate limited', { status: 429 }) - }) - - const error = await expectOAuthError( - getUserAndAccounts('test-token'), - 'temporarily_unavailable', - 429 - ) - expect(error.headers).toEqual({ 'Retry-After': '17' }) - }) - - it('defaults Retry-After when Cloudflare API 429 responses omit it', async () => { - mockProbes({ - user: () => new HttpResponse('rate limited', { status: 429 }), - accounts: () => new HttpResponse('rate limited', { status: 429 }) - }) - - const error = await expectOAuthError( - getUserAndAccounts('test-token'), - 'temporarily_unavailable', - 429 - ) - expect(error.headers).toEqual({ 'Retry-After': '30' }) - }) - - it('falls back to account-scoped auth when /user is 200 but invalid JSON', async () => { - mockProbes({ - user: () => new HttpResponse('not-json', { status: 200 }), - accounts: () => - HttpResponse.json(cfAccountsSuccess([{ id: 'acc-1', name: 'Primary Account' }])) - }) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: null, - accounts: [{ id: 'acc-1', name: 'Primary Account' }] - }) - }) - - it('falls back to account-scoped auth when /user is 200 with success=false', async () => { - mockProbes({ - user: () => HttpResponse.json({ success: false }), - accounts: () => - HttpResponse.json(cfAccountsSuccess([{ id: 'acc-1', name: 'Primary Account' }])) - }) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: null, - accounts: [{ id: 'acc-1', name: 'Primary Account' }] - }) - }) - - it('keeps user auth when /accounts is 200 but invalid JSON', async () => { - mockProbes({ - user: () => HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })), - accounts: () => new HttpResponse('not-json', { status: 200 }) - }) - - await expect(getUserAndAccounts('test-token')).resolves.toEqual({ - user: { id: 'user-1', email: 'user@example.com' }, - accounts: [] - }) - }) - - it('rejects when /accounts returns empty result and /user fails', async () => { - mockProbes({ - user: () => new HttpResponse('Forbidden', { status: 403 }), - accounts: () => HttpResponse.json(cfAccountsSuccess([])) - }) - - await expectOAuthError(getUserAndAccounts('test-token'), 'invalid_token', 401) - }) - - it('rejects when /accounts payload shape is invalid and /user fails', async () => { - mockProbes({ - user: () => new HttpResponse('Forbidden', { status: 403 }), - accounts: () => HttpResponse.json(cfSuccess([{ id: 'acc-1' }])) - }) - - await expectOAuthError(getUserAndAccounts('test-token'), 'invalid_token', 401) - }) - - it('maps a network failure to server_error', async () => { - // A transport-level failure (fetch rejecting) is BELOW MSW's HTTP - // abstraction — HttpResponse.error() leaks an unhandled rejection through - // @mswjs/interceptors. The fetch primitive is the correct seam for a - // network error, so spy it here. (vi.spyOn restored in afterEach.) - vi.spyOn(globalThis, 'fetch').mockRejectedValue(new TypeError('network failed')) - - await expectOAuthError(getUserAndAccounts('test-token'), 'server_error', 502) - }) -}) - describe('guardRefreshTokenExchange', () => { it('singleflights concurrent refreshes for the same upstream token in one isolate', async () => { const kv = env.OAUTH_KV diff --git a/tests/auth/oauth-routes.test.ts b/tests/auth/oauth-routes.test.ts index 1c5dda7..11935a3 100644 --- a/tests/auth/oauth-routes.test.ts +++ b/tests/auth/oauth-routes.test.ts @@ -20,6 +20,8 @@ import { server } from '../setup/msw' const REDIRECT_URI = 'https://app.example.com/cb' const MCP_ORIGIN = 'https://mcp.cloudflare.com' +const DOWNSTREAM_CODE_VERIFIER = 'test-downstream-code-verifier' +const DOWNSTREAM_CODE_CHALLENGE = 'I4fhllfHqqQsgap17V2SDI0scSei8H7U0e0rZBDIcbo' /** Register a client via the provider's RFC 7591 endpoint; returns its id. */ async function registerClient(): Promise { @@ -62,6 +64,8 @@ async function beginAuthorization(options: { state?: string; scopes?: string } = response_type: 'code', client_id: clientId, redirect_uri: REDIRECT_URI, + code_challenge: DOWNSTREAM_CODE_CHALLENGE, + code_challenge_method: 'S256', scope: 'user:read', ...(options.state === undefined ? {} : { state: options.state }) }) @@ -95,7 +99,12 @@ async function beginAuthorization(options: { state?: string; scopes?: string } = } } -function useCloudflareAuthSuccess(): void { +function useCloudflareAuthSuccess(): { + userCalls: () => number + accountCalls: () => number +} { + let userCalls = 0 + let accountCalls = 0 server.use( http.post('https://dash.cloudflare.com/oauth2/token', () => HttpResponse.json({ @@ -106,13 +115,16 @@ function useCloudflareAuthSuccess(): void { token_type: 'bearer' }) ), - http.get('https://api.cloudflare.com/client/v4/user', () => - HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })) - ), - http.get('https://api.cloudflare.com/client/v4/accounts', () => - HttpResponse.json(cfAccountsSuccess([{ id: 'acc-1', name: 'Account One' }])) - ) + http.get('https://api.cloudflare.com/client/v4/user', () => { + userCalls++ + return HttpResponse.json(cfSuccess({ id: 'user-1', email: 'user@example.com' })) + }), + http.get('https://api.cloudflare.com/client/v4/accounts', () => { + accountCalls++ + return HttpResponse.json(cfAccountsSuccess([{ id: 'acc-1', name: 'Account One' }])) + }) ) + return { userCalls: () => userCalls, accountCalls: () => accountCalls } } /** Collapse a response's Set-Cookie header(s) into a Cookie request header. */ @@ -152,6 +164,8 @@ describe('GET /authorize', () => { response_type: 'code', client_id: clientId, redirect_uri: REDIRECT_URI, + code_challenge: DOWNSTREAM_CODE_CHALLENGE, + code_challenge_method: 'S256', scope: 'user:read' }) ) @@ -191,6 +205,8 @@ describe('GET /authorize', () => { response_type: 'code', client_id: clientId, redirect_uri: REDIRECT_URI, + code_challenge: DOWNSTREAM_CODE_CHALLENGE, + code_challenge_method: 'S256', scope: 'user:read' }) ) @@ -232,7 +248,9 @@ describe('GET /authorize', () => { authorizeUrl({ response_type: 'code', client_id: 'does-not-exist', - redirect_uri: REDIRECT_URI + redirect_uri: REDIRECT_URI, + code_challenge: DOWNSTREAM_CODE_CHALLENGE, + code_challenge_method: 'S256' }) ) ) @@ -286,9 +304,9 @@ describe('GET /oauth/callback', () => { expect(dp.blobs?.[3]).toBeFalsy() }) - it('serves modern MCP from provider-issued authenticated props', async () => { + it('serves modern MCP without externally resolving the provider-issued token', async () => { const { clientId, state, sessionCookie } = await beginAuthorization() - useCloudflareAuthSuccess() + const probes = useCloudflareAuthSuccess() const callback = await exports.default.fetch( new Request(`${MCP_ORIGIN}/oauth/callback?code=authcode&state=${encodeURIComponent(state)}`, { headers: { Cookie: sessionCookie }, @@ -306,17 +324,22 @@ describe('GET /oauth/callback', () => { grant_type: 'authorization_code', code: code!, client_id: clientId, - redirect_uri: REDIRECT_URI + redirect_uri: REDIRECT_URI, + code_verifier: DOWNSTREAM_CODE_VERIFIER }).toString() }) ) expect(tokenResponse.status).toBe(200) const { access_token } = (await tokenResponse.json()) as { access_token: string } + expect(probes.userCalls()).toBe(1) + expect(probes.accountCalls()).toBe(1) const mcpResponse = await exports.default.fetch(modernMcpRequest(access_token, 'tools/list')) const mcpBody = await parseMcpResult(mcpResponse) expect(mcpResponse.status).toBe(200) + expect(probes.userCalls()).toBe(1) + expect(probes.accountCalls()).toBe(1) expect(mcpBody.result?.resultType).toBe('complete') expect(mcpBody.result?.tools?.map((tool) => tool.name)).toEqual(['docs', 'search', 'execute']) }) diff --git a/tests/helpers/cloudflare-api.ts b/tests/helpers/cloudflare-api.ts index 8bebb15..dd8f5ac 100644 --- a/tests/helpers/cloudflare-api.ts +++ b/tests/helpers/cloudflare-api.ts @@ -28,8 +28,8 @@ export function cfError(errors: Array<{ code: number; message: string }>, result } /** - * Register MSW handlers for the API-token identity probe (`/user` + `/accounts`) - * so a direct token resolves through the REAL `getUserAndAccounts` code path. + * Register MSW handlers for legacy/user credential identity probes + * (`/user` + `/accounts`) through the real external-token resolver. * * - `user: null` (default) -> account-scoped token (single account pinned) * - `user` provided -> user token diff --git a/tests/mcp-modern.test.ts b/tests/mcp-modern.test.ts index b57b888..cf40706 100644 --- a/tests/mcp-modern.test.ts +++ b/tests/mcp-modern.test.ts @@ -256,6 +256,7 @@ describe('MCP 2026-07-28 stateless handler', () => { expect(response.headers.get('www-authenticate')).toMatch( /^Bearer .*resource_metadata="https:\/\/mcp\.cloudflare\.com\/\.well-known\/oauth-protected-resource\/mcp"/ ) + expect(await response.text()).toBe('') }) }) diff --git a/tests/worker.test.ts b/tests/worker.test.ts index 1d8f9e2..578196b 100644 --- a/tests/worker.test.ts +++ b/tests/worker.test.ts @@ -1,7 +1,7 @@ import { env } from 'cloudflare:workers' import { http, HttpResponse } from 'msw' import { afterEach, describe, expect, it } from 'vitest' -import { API_BASE, cfSuccess, mockIdentityProbe } from './helpers/cloudflare-api' +import { API_BASE, cfAccountsSuccess, cfSuccess } from './helpers/cloudflare-api' import { clearKv } from './helpers/kv' import { callTool, toolText } from './helpers/mcp' import { server } from './setup/msw' @@ -21,16 +21,23 @@ import { server } from './setup/msw' const ACCOUNT_ID = '00000000000000000000000000000001' -// A direct (non-OAuth) API token: NOT 3 colon-separated parts, so the worker -// treats it as a direct Cloudflare API token rather than an OAuth bearer. -const API_TOKEN = 'test-api-token-e2e' +// A prefixed account-owned Cloudflare API token. The OAuth provider tries its +// own token store first, then delegates the miss to the external token resolver. +const API_TOKEN = 'cfat_test-api-token-e2e' afterEach(() => clearKv(env.OAUTH_KV)) describe('worker: execute tool call', () => { it('runs code in a Worker Loader isolate and returns the mocked API result', async () => { - // Real getUserAndAccounts resolves this token to a single-account token. - mockIdentityProbe({ accounts: [{ id: ACCOUNT_ID, name: 'E2E Test Account' }] }) + // Account prefixes skip /user. MSW fails this test if an unregistered /user + // request occurs, so only register the one expected identity probe. + let accountProbeCalls = 0 + server.use( + http.get(`${API_BASE}/accounts`, () => { + accountProbeCalls++ + return HttpResponse.json(cfAccountsSuccess([{ id: ACCOUNT_ID, name: 'E2E Test Account' }])) + }) + ) // The Cloudflare API call the executed code makes, forwarded by GlobalOutbound. let verifyCalled = false @@ -59,6 +66,7 @@ describe('worker: execute tool call', () => { expect(text).toContain('"status": "active"') expect(text).toContain('token-1') + expect(accountProbeCalls).toBe(1) // The forwarded API call really went through GlobalOutbound -> MSW. expect(verifyCalled).toBe(true) })