From 66dec3d0c8bd2edadc75d9d0b3060e88e4734fb2 Mon Sep 17 00:00:00 2001 From: Codebeast Date: Wed, 15 Apr 2026 09:14:52 -0500 Subject: [PATCH 1/2] fix(oauth): rescue legacy grants via grant.scope fallback in resolveAuth (#29) The C-1a remediation (commit 256ba06, 2026-04-10) removed the hardcoded ['generate','read'] path in resolveAuth and started enforcing the actual scopes threaded through props.scopes. PR #32 fixed the grant-creation side so empty-scope OAuth initiates get routed through the consent page and mint grants with populated scopes. This closes the last gap: the READ side. Any grant minted before C-1a carries empty props.scopes (the pre-C-1a path wrote the grant top-level scope but never populated props), so every authenticated request from that legacy cohort still hits a (none) scopes error at tool dispatch - exactly the UX symptom #29 and #30 describe. resolveAuth now calls a new resolveLegacyGrantScopes helper when oauthProps.scopes is empty. The helper extracts the bearer, calls the OAuth provider's unwrapToken (which denormalizes the top-level grant.scope onto the token record), and returns it as the effective scope set. Successful rescues are logged so we can measure the legacy cohort shrinking over time and know when to retire both this fallback and the future backfill script. What this does NOT rescue: - Grants minted between C-1a (2026-04-10) and #32 (2026-04-11 20:43Z) where grant.scope and props.scopes are both empty. These users must disconnect and reauth through the #32 consent page - nothing exists on the server to fall back to. Architecture: The @cloudflare/workers-oauth-provider library transitively imports cloudflare:* protocol modules that vitest's node ESM loader cannot resolve, so getOAuthApi is dynamically imported inside the fallback helper. Every other unit test that transitively imports gateway.ts continues to work without touching the library. OAuthProvider configuration was extracted to src/oauth-config.ts so gateway.ts can pass the same options to getOAuthApi without a circular import back through index.ts. In-flight safety: deploy this READ fallback first. The backfill script that rewrites stale KV grants comes in a separate PR and must run AFTER this deploy is live - a partial backfill against workers running old code will 500 in-flight sessions on mixed blob versions. Tests (6 new, 131/131 total): - props.scopes present -> uses them directly, never touches unwrapToken - props.scopes empty + grant.scope populated -> fallback rescues, session created with rescued scopes, tool call succeeds - both empty -> scopes stay empty, tool call rejects with (none) error, which is the UX signal pointing users to reauth - unwrapToken returns null -> graceful empty fallback - unwrapToken throws (KV outage) -> caught, logged, empty fallback - no bearer header -> fallback is inert Related: #28 (closed by #32), #29 (this PR), #30 (should resolve after this deploys + users reconnect). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/gateway.ts | 67 +++++++- src/index.ts | 14 +- src/oauth-config.ts | 26 +++ test/gateway-legacy-scope.test.ts | 275 ++++++++++++++++++++++++++++++ 4 files changed, 371 insertions(+), 11 deletions(-) create mode 100644 src/oauth-config.ts create mode 100644 test/gateway-legacy-scope.test.ts diff --git a/src/gateway.ts b/src/gateway.ts index 81f2a2f..fd228fc 100644 --- a/src/gateway.ts +++ b/src/gateway.ts @@ -5,6 +5,7 @@ import type { GatewayEnv, AuthResult, Tier, RiskLevel } from './types.js'; import { extractBearerToken, validateBearerToken, buildWwwAuthenticate } from './auth.js'; +import { OAUTH_PROVIDER_CONFIG } from './oauth-config.js'; import { resolveRoute, getToolRiskLevel, ROUTE_TABLE, type BackendRoute } from './route-table.js'; import { toBackendToolName, buildAggregatedCatalog, validateToolArguments } from './tool-registry.js'; import { type AuditArtifact, generateTraceId, summarizeInput, emitAudit, queueAuditEvent } from './audit.js'; @@ -977,6 +978,53 @@ export async function handleMcpRequest( return jsonResponse({ error: 'Method not allowed', code: 'METHOD_NOT_ALLOWED' }, 405); } +// #29 legacy-scope fallback. Extracts the bearer token from the incoming +// request and calls the OAuth provider's `unwrapToken` helper to read the +// denormalized top-level `grant.scope` off the stored token record. This +// rescues grants minted before the C-1a remediation (commit 256ba06, +// 2026-04-10), which wrote valid scopes at the grant top level but empty +// `props.scopes`. Returns an empty array on any failure — the caller +// treats that as "no scopes" and downstream scope enforcement will reject +// the tool call, which is the correct behavior for the unrescuable cohort +// (grants minted between C-1a and #32 that have empty scopes at every +// level). Any failure mode falls through to the same empty-scopes outcome +// the caller already handled before this fallback existed. +async function resolveLegacyGrantScopes( + request: Request, + env: GatewayEnv, +): Promise { + const bearer = extractBearerToken(request); + if (!bearer) return []; + try { + // Dynamic import: the @cloudflare/workers-oauth-provider library + // transitively imports `cloudflare:*` protocol modules that the node + // ESM loader used by vitest unit tests cannot resolve. Eagerly + // importing `getOAuthApi` at the top of gateway.ts breaks every + // unit test that transitively imports gateway.ts, even tests that + // never exercise the fallback. Lazy-loading here confines the + // workerd-specific dependency to runtime on the fallback path, + // which under vitest is mocked via vi.mock. + const { getOAuthApi } = await import('@cloudflare/workers-oauth-provider'); + const helpers = getOAuthApi(OAUTH_PROVIDER_CONFIG, env); + const token = await helpers.unwrapToken(bearer); + if (!token) return []; + const grantScope = token.grant?.scope ?? []; + if (grantScope.length > 0) { + // Rescue hit — log so we can measure the legacy cohort shrinking + // over time and know when to retire the fallback + backfill script. + console.warn( + `[gateway] legacy-grant scope fallback rescued grant=${token.grantId} scopes=${grantScope.join(',')}`, + ); + } + return grantScope; + } catch (err) { + console.error( + `[gateway] resolveLegacyGrantScopes failed: ${err instanceof Error ? err.message : String(err)}`, + ); + return []; + } +} + // ─── Resolve auth: OAuth props (from OAuthProvider) or Bearer token (API keys/JWTs) ── async function resolveAuth( request: Request, @@ -985,6 +1033,20 @@ async function resolveAuth( ): Promise { // If OAuthProvider already validated the token, use its props if (oauthProps?.userId) { + // #29 legacy-scope fallback: grants minted before the C-1a remediation + // (256ba06, 2026-04-10) carry empty `props.scopes` because the old + // hardcoded path wrote `['generate','read']` at the grant top level but + // never populated props.scopes. Post-#32 grants populate both. Any + // authenticated caller hitting this path with empty props.scopes is + // either a legacy-cohort user or a client that minted a grant between + // C-1a and #32. We try to rescue the first cohort by reading the + // denormalized `grant.scope` off the token record; the second cohort + // has nothing to fall back to and must reauth. + let effectiveScopes = oauthProps.scopes ?? []; + if (effectiveScopes.length === 0) { + effectiveScopes = await resolveLegacyGrantScopes(request, env); + } + // Resolve tenant info from AUTH_SERVICE for proper tier try { const tenant = await env.AUTH_SERVICE.provisionTenant({ @@ -1000,8 +1062,9 @@ async function resolveAuth( // was issued with. Previously hardcoded to ['generate', 'read'], // which silently granted full access to any OAuth-authed caller // regardless of what their token claimed. Now respects the scopes - // passed via completeAuthorization() props. - scopes: oauthProps.scopes ?? [], + // passed via completeAuthorization() props, with a #29 fallback to + // the top-level grant.scope for legacy grants minted pre-C-1a. + scopes: effectiveScopes, }; } catch (err) { // Tenant resolution failed — cannot proceed without a tenantId. diff --git a/src/index.ts b/src/index.ts index aee0f04..9d76393 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,7 +7,7 @@ import OAuthProvider from '@cloudflare/workers-oauth-provider'; import type { GatewayEnv } from './types.js'; import { handleMcpRequest } from './gateway.js'; -import oauthHandler from './oauth-handler.js'; +import { OAUTH_PROVIDER_CONFIG } from './oauth-config.js'; const CORS_HEADERS: Record = { 'Access-Control-Allow-Origin': '*', @@ -26,7 +26,10 @@ function addCorsHeaders(response: Response): Response { } const oauthProvider = new OAuthProvider({ - apiRoute: '/mcp', + ...OAUTH_PROVIDER_CONFIG, + // Override the config's stub apiHandler with the real one that + // routes authenticated requests into handleMcpRequest. See + // src/oauth-config.ts for why the base config carries a stub. apiHandler: { fetch: async (request: Request, env: GatewayEnv, ctx: ExecutionContext) => { // OAuthProvider validates the token and sets ctx.props with the @@ -39,13 +42,6 @@ const oauthProvider = new OAuthProvider({ return addCorsHeaders(response); }, }, - defaultHandler: oauthHandler, - authorizeEndpoint: '/authorize', - tokenEndpoint: '/token', - clientRegistrationEndpoint: '/register', - scopesSupported: ['generate', 'read'], - accessTokenTTL: 3600, - refreshTokenTTL: 2592000, }); export default { diff --git a/src/oauth-config.ts b/src/oauth-config.ts new file mode 100644 index 0000000..5b4c2ad --- /dev/null +++ b/src/oauth-config.ts @@ -0,0 +1,26 @@ +// ─── Shared OAuthProvider configuration ────────────────────── +// Extracted into its own module so gateway.ts can call getOAuthApi() +// for the #29 legacy-scope fallback in resolveAuth without creating a +// circular import with index.ts (which imports handleMcpRequest from +// gateway.ts). The real apiHandler.fetch is attached in index.ts at +// provider-instantiation time — the stub below exists only to satisfy +// the OAuthProviderOptions shape for read-side helpers calls like +// unwrapToken, which never traverse apiHandler. + +import type { OAuthProviderOptions } from '@cloudflare/workers-oauth-provider'; +import type { GatewayEnv } from './types.js'; +import oauthHandler from './oauth-handler.js'; + +export const OAUTH_PROVIDER_CONFIG: OAuthProviderOptions = { + apiRoute: '/mcp', + apiHandler: { + fetch: async () => new Response('api handler not attached', { status: 500 }), + }, + defaultHandler: oauthHandler, + authorizeEndpoint: '/authorize', + tokenEndpoint: '/token', + clientRegistrationEndpoint: '/register', + scopesSupported: ['generate', 'read'], + accessTokenTTL: 3600, + refreshTokenTTL: 2592000, +}; diff --git a/test/gateway-legacy-scope.test.ts b/test/gateway-legacy-scope.test.ts new file mode 100644 index 0000000..cd3043c --- /dev/null +++ b/test/gateway-legacy-scope.test.ts @@ -0,0 +1,275 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock @cloudflare/workers-oauth-provider so the #29 fallback path in +// resolveAuth can be exercised deterministically without standing up a +// real OAuthProvider + KV token store. We cannot use vi.importActual +// here because the library imports `cloudflare:*` modules that are only +// resolvable inside workerd — under node's default ESM loader that +// protocol is unsupported and importActual fails. gateway.ts only uses +// `getOAuthApi` from this module, so a minimal stub shape is sufficient. +vi.mock('@cloudflare/workers-oauth-provider', () => ({ + default: class {}, + getOAuthApi: vi.fn(), +})); + +import { getOAuthApi } from '@cloudflare/workers-oauth-provider'; +import { handleMcpRequest } from '../src/gateway.js'; +import type { GatewayEnv, AuthServiceRpc } from '../src/types.js'; + +// ─── Shared mocks ───────────────────────────────────────────── +function mockAuthService(): AuthServiceRpc { + return { + validateApiKey: async () => ({ valid: false }), + validateJwt: async () => ({ valid: false }), + authenticateUser: async () => ({ valid: false }), + registerUser: async () => ({ valid: false }), + provisionTenant: async () => ({ + tenantId: 'tenant-oauth-1', + userId: 'user-oauth-1', + tier: 'pro', + delinquent: false, + createdAt: '2026-04-11T00:00:00Z', + }), + exchangeSocialCode: async () => ({ valid: false }), + }; +} + +function mockFetcher(responseBody: unknown = {}, status = 200): Fetcher { + return { + fetch: async () => new Response(JSON.stringify(responseBody), { + status, + headers: { 'Content-Type': 'application/json' }, + }), + connect: () => { throw new Error('not implemented'); }, + } as unknown as Fetcher; +} + +function mockKV(): KVNamespace { + const store = new Map(); + return { + get: async (key: string) => store.get(key) ?? null, + put: async (key: string, value: string) => { store.set(key, value); }, + delete: async (key: string) => { store.delete(key); }, + list: async () => ({ keys: [], list_complete: true, cacheStatus: null }), + getWithMetadata: async () => ({ value: null, metadata: null, cacheStatus: null }), + } as unknown as KVNamespace; +} + +function makeEnv(): GatewayEnv { + return { + AUTH_SERVICE: mockAuthService(), + TAROTSCRIPT: mockFetcher({ verified: true, createdAt: '2026-04-11' }), + IMG_FORGE: mockFetcher(), + OAUTH_PROVIDER: {} as any, + OAUTH_KV: mockKV(), + PLATFORM_EVENTS_QUEUE: { send: async () => {} } as unknown as Queue, + SERVICE_BINDING_SECRET: 'test-secret', + API_BASE_URL: 'https://mcp.stackbilt.dev', + }; +} + +// Bearer tokens issued by the OAuth provider take the format +// `{userId}:{grantId}:{secret}`; our fake token matches that shape so +// resolveLegacyGrantScopes' extractBearerToken + unwrapToken path +// doesn't short-circuit before reaching the mock. +const FAKE_OAUTH_BEARER = 'user-oauth-1:grant-abc:secret-xyz'; + +function rpcRequest( + method: string, + params?: unknown, + headers?: Record, +): Request { + return new Request('https://mcp.stackbilt.dev/mcp', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', + Authorization: `Bearer ${FAKE_OAUTH_BEARER}`, + ...headers, + }, + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method, params }), + }); +} + +// Helper: stub getOAuthApi to return a helpers object whose unwrapToken +// resolves to a TokenSummary with the given grant scope. +function stubUnwrapToken(grantScope: string[] | null) { + (getOAuthApi as unknown as ReturnType).mockReturnValue({ + unwrapToken: async () => { + if (grantScope === null) return null; + return { + id: 'tok-hash', + grantId: 'grant-abc', + userId: 'user-oauth-1', + createdAt: Math.floor(Date.now() / 1000), + expiresAt: Math.floor(Date.now() / 1000) + 3600, + audience: undefined, + scope: grantScope, + grant: { + clientId: 'client-xyz', + scope: grantScope, + props: { userId: 'user-oauth-1' }, + }, + }; + }, + }); +} + +// ─── Tests ──────────────────────────────────────────────────── +describe('#29 legacy-scope fallback in resolveAuth', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('uses props.scopes directly when populated — no fallback call', async () => { + const unwrap = vi.fn(); + (getOAuthApi as unknown as ReturnType).mockReturnValue({ unwrapToken: unwrap }); + + const env = makeEnv(); + const oauthProps = { userId: 'user-oauth-1', scopes: ['read', 'generate'] }; + + const initReq = rpcRequest('initialize', { + protocolVersion: '2025-03-26', + capabilities: {}, + clientInfo: { name: 'test' }, + }); + const initRes = await handleMcpRequest(initReq, env, oauthProps); + + expect(initRes.status).toBe(200); + expect(unwrap).not.toHaveBeenCalled(); + + const body = await initRes.json() as { result: { serverInfo: { metadata: { session: { scopes: string[] } } } } }; + expect(body.result.serverInfo.metadata.session.scopes).toEqual(['read', 'generate']); + }); + + it('rescues legacy grant when props.scopes is empty but grant.scope is populated', async () => { + stubUnwrapToken(['read', 'generate']); + + const env = makeEnv(); + const oauthProps = { userId: 'user-oauth-1', scopes: [] }; + + const initReq = rpcRequest('initialize', { + protocolVersion: '2025-03-26', + capabilities: {}, + clientInfo: { name: 'test' }, + }); + const initRes = await handleMcpRequest(initReq, env, oauthProps); + expect(initRes.status).toBe(200); + const sessionId = initRes.headers.get('MCP-Session-Id')!; + expect(sessionId).toBeTruthy(); + + // Session should be created with rescued scopes. + const initBody = await initRes.json() as { result: { serverInfo: { metadata: { session: { scopes: string[] } } } } }; + expect(initBody.result.serverInfo.metadata.session.scopes).toEqual(['read', 'generate']); + + // And a tools/call with the rescued read scope should succeed on a READ_ONLY tool. + const callReq = rpcRequest( + 'tools/call', + { name: 'flow_status', arguments: { hash: 'abc' } }, + { 'MCP-Session-Id': sessionId }, + ); + const callRes = await handleMcpRequest(callReq, env, oauthProps); + const callBody = await callRes.json() as { result?: unknown; error?: unknown }; + expect(callBody.error).toBeFalsy(); + expect(callBody.result).toBeTruthy(); + }); + + it('leaves scopes empty when both props.scopes and grant.scope are empty', async () => { + // This is the between-C-1a-and-#32 cohort: nothing to fall back to. + // The caller must reauth through the #32 consent screen path. + stubUnwrapToken([]); + + const env = makeEnv(); + const oauthProps = { userId: 'user-oauth-1', scopes: [] }; + + const initReq = rpcRequest('initialize', { + protocolVersion: '2025-03-26', + capabilities: {}, + clientInfo: { name: 'test' }, + }); + const initRes = await handleMcpRequest(initReq, env, oauthProps); + expect(initRes.status).toBe(200); + const sessionId = initRes.headers.get('MCP-Session-Id')!; + + const initBody = await initRes.json() as { result: { serverInfo: { metadata: { session: { scopes: string[] } } } } }; + expect(initBody.result.serverInfo.metadata.session.scopes).toEqual([]); + + // tools/call should fail with the (none) scopes message — same shape the + // reporter hit, and the UX signal that points the user to reauth. + const callReq = rpcRequest( + 'tools/call', + { name: 'flow_status', arguments: { hash: 'abc' } }, + { 'MCP-Session-Id': sessionId }, + ); + const callRes = await handleMcpRequest(callReq, env, oauthProps); + const callBody = await callRes.json() as { error?: { code: number; message: string } }; + expect(callBody.error).toBeTruthy(); + expect(callBody.error?.code).toBe(-32600); + expect(callBody.error?.message).toContain('(none)'); + }); + + it('gracefully handles unwrapToken returning null (unknown token)', async () => { + stubUnwrapToken(null); + + const env = makeEnv(); + const oauthProps = { userId: 'user-oauth-1', scopes: [] }; + + const initReq = rpcRequest('initialize', { + protocolVersion: '2025-03-26', + capabilities: {}, + clientInfo: { name: 'test' }, + }); + const initRes = await handleMcpRequest(initReq, env, oauthProps); + + expect(initRes.status).toBe(200); + const body = await initRes.json() as { result: { serverInfo: { metadata: { session: { scopes: string[] } } } } }; + expect(body.result.serverInfo.metadata.session.scopes).toEqual([]); + }); + + it('gracefully handles unwrapToken throwing (KV outage, malformed record)', async () => { + (getOAuthApi as unknown as ReturnType).mockReturnValue({ + unwrapToken: async () => { throw new Error('kv unavailable'); }, + }); + + const env = makeEnv(); + const oauthProps = { userId: 'user-oauth-1', scopes: [] }; + + const initReq = rpcRequest('initialize', { + protocolVersion: '2025-03-26', + capabilities: {}, + clientInfo: { name: 'test' }, + }); + const initRes = await handleMcpRequest(initReq, env, oauthProps); + + // Fallback error must not break auth resolution — we return empty scopes + // and let downstream scope enforcement reject the tool call. + expect(initRes.status).toBe(200); + const body = await initRes.json() as { result: { serverInfo: { metadata: { session: { scopes: string[] } } } } }; + expect(body.result.serverInfo.metadata.session.scopes).toEqual([]); + }); + + it('skips fallback entirely when no bearer token is present', async () => { + const unwrap = vi.fn(); + (getOAuthApi as unknown as ReturnType).mockReturnValue({ unwrapToken: unwrap }); + + const env = makeEnv(); + const oauthProps = { userId: 'user-oauth-1', scopes: [] }; + + // Request with no Authorization header — the OAuth layer above the gateway + // would normally block this, but the fallback should be inert anyway. + const req = new Request('https://mcp.stackbilt.dev/mcp', { + method: 'POST', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-03-26', capabilities: {}, clientInfo: { name: 'test' } }, + }), + }); + + const res = await handleMcpRequest(req, env, oauthProps); + expect(res.status).toBe(200); + expect(unwrap).not.toHaveBeenCalled(); + }); +}); From a688fbb06d2073fdbc8193d9a4a227f0342e7c06 Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Fri, 17 Apr 2026 09:06:15 -0500 Subject: [PATCH 2/2] ci: refresh for current main (post-#40)