diff --git a/src/oauth-handler.ts b/src/oauth-handler.ts index e360688..49f5431 100644 --- a/src/oauth-handler.ts +++ b/src/oauth-handler.ts @@ -8,6 +8,18 @@ import type { AuthRequest } from '@cloudflare/workers-oauth-provider'; // ── Feature gate: flip to true when ready to accept public signups ── const PUBLIC_SIGNUPS_ENABLED = true; +// #28 fix: default scopes shown on the consent screen when an OAuth client +// initiates without requesting any scope. The C-1a remediation (commit 256ba06, +// 2026-04-10) correctly removed the hardcoded ['generate','read'] grant — any +// caller with an OAuth token was inheriting those scopes regardless of what +// their token claimed — but left no path for clients (notably Claude Code / +// Claude.ai MCP clients) that don't include scope in their OAuth initiate. +// Those requests end up creating grants that are empty at every level +// (grant.scope AND encryptedProps.scopes), silently blocking every subsequent +// tool call. These defaults are NEVER silently applied: they're rendered on +// the consent page and only enter the grant after explicit user approval. +const DEFAULT_CONSENT_SCOPES = ['read', 'generate']; + // --- Shared styles --- const SHARED_STYLES = ` @@ -469,6 +481,27 @@ async function handleGetAuthorize(request: Request, env: GatewayEnv): Promise { }); }); +describe('OAuth handler — #28 empty-scope consent flow', () => { + // When the OAuth client's initiate omits `scope` entirely, C-1a's + // remediation made the gateway silently mint an empty-scope grant — + // every subsequent tool call failed with "(none)". The fix: route + // empty-scope initiates through the consent page with defaults so + // the user explicitly approves. Auto-approve still works for every + // initiate that includes explicit scope (regression guard below). + + async function makeIdentityToken(clientId: string, redirectUri: string): Promise { + return signIdentityToken(TEST_SECRET, { + userId: 'u1', + email: 'test@test.com', + name: 'Test', + clientId, + redirectUri, + }); + } + + it('renders the consent page when oauthReqInfo.scope is empty', async () => { + const emptyScopeAuthRequest: AuthRequest = { ...MOCK_AUTH_REQUEST, scope: [] }; + const provider = mockOAuthProvider({ + parseAuthRequest: vi.fn(async () => emptyScopeAuthRequest), + }); + const env = makeEnv({ + OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'], + }); + + const identityToken = await makeIdentityToken( + MOCK_AUTH_REQUEST.clientId, + MOCK_AUTH_REQUEST.redirectUri, + ); + const req = getRequest(`/authorize?identity_token=${encodeURIComponent(identityToken)}`); + const res = await callHandler(req, env); + + // Consent page rendered — NOT a redirect to completeAuthorization. + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toContain('text/html'); + const body = await res.text(); + expect(body).toContain('Authorize access'); + expect(body).toContain('read'); + expect(body).toContain('generate'); + // The consent page carries forward the oauth_params + identity_token + // so the POST-back to /authorize has everything it needs. + expect(body).toContain('identity_token'); + expect(body).toContain('action'); + + // Crucial: no empty grant gets minted at this step. + expect(provider.completeAuthorization).not.toHaveBeenCalled(); + }); + + it('auto-approves (skips consent) when the client explicitly requests scope', async () => { + // Regression guard for the zero-latency path that Claude Code depends on. + const provider = mockOAuthProvider(); // default scope ['generate', 'read'] + const env = makeEnv({ + OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'], + }); + + const identityToken = await makeIdentityToken( + MOCK_AUTH_REQUEST.clientId, + MOCK_AUTH_REQUEST.redirectUri, + ); + const req = getRequest(`/authorize?identity_token=${encodeURIComponent(identityToken)}`); + const res = await callHandler(req, env); + + // Redirect back to client — auto-approve path, no consent hop. + expect(res.status).toBe(302); + expect(provider.completeAuthorization).toHaveBeenCalledOnce(); + // The grant uses the client-requested scope verbatim (not the defaults). + expect(provider.completeAuthorization).toHaveBeenCalledWith( + expect.objectContaining({ + scope: ['generate', 'read'], + props: expect.objectContaining({ scopes: ['generate', 'read'] }), + }), + ); + }); + + it('handlePostAuthorize injects consent defaults when the original initiate was empty-scope', async () => { + const emptyScopeAuthRequest: AuthRequest = { ...MOCK_AUTH_REQUEST, scope: [] }; + const emptyScopeOauthParams = btoa(JSON.stringify(emptyScopeAuthRequest)); + + const provider = mockOAuthProvider({ + parseAuthRequest: vi.fn(async () => emptyScopeAuthRequest), + }); + const env = makeEnv({ + OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'], + }); + + const identityToken = await makeIdentityToken( + MOCK_AUTH_REQUEST.clientId, + MOCK_AUTH_REQUEST.redirectUri, + ); + + const req = formRequest('/authorize', { + action: 'approve', + oauth_params: emptyScopeOauthParams, + identity_token: identityToken, + }); + const res = await callHandler(req, env); + + expect(res.status).toBe(302); + // The grant was stored with the consent defaults — NOT empty. + expect(provider.completeAuthorization).toHaveBeenCalledWith( + expect.objectContaining({ + scope: ['read', 'generate'], + props: expect.objectContaining({ scopes: ['read', 'generate'] }), + }), + ); + }); + + it('handlePostAuthorize preserves the client-requested scope when it is non-empty', async () => { + // Regression guard — explicit scopes are never overwritten. + const oauthParams = makeOAuthParamsB64(); // scope: ['generate', 'read'] + const provider = mockOAuthProvider(); + const env = makeEnv({ + OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'], + }); + + const identityToken = await makeIdentityToken( + MOCK_AUTH_REQUEST.clientId, + MOCK_AUTH_REQUEST.redirectUri, + ); + + const req = formRequest('/authorize', { + action: 'approve', + oauth_params: oauthParams, + identity_token: identityToken, + }); + const res = await callHandler(req, env); + + expect(res.status).toBe(302); + expect(provider.completeAuthorization).toHaveBeenCalledWith( + expect.objectContaining({ + scope: ['generate', 'read'], + props: expect.objectContaining({ scopes: ['generate', 'read'] }), + }), + ); + }); + + it('deny action still works for empty-scope initiates (no grant created)', async () => { + const emptyScopeAuthRequest: AuthRequest = { ...MOCK_AUTH_REQUEST, scope: [] }; + const emptyScopeOauthParams = btoa(JSON.stringify(emptyScopeAuthRequest)); + + const provider = mockOAuthProvider({ + parseAuthRequest: vi.fn(async () => emptyScopeAuthRequest), + }); + const env = makeEnv({ + OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'], + }); + + const identityToken = await makeIdentityToken( + MOCK_AUTH_REQUEST.clientId, + MOCK_AUTH_REQUEST.redirectUri, + ); + + const req = formRequest('/authorize', { + action: 'deny', + oauth_params: emptyScopeOauthParams, + identity_token: identityToken, + }); + const res = await callHandler(req, env); + + // 302 back to client with access_denied — and crucially, no grant. + expect(res.status).toBe(302); + const location = res.headers.get('Location'); + expect(location).toContain('error=access_denied'); + expect(provider.completeAuthorization).not.toHaveBeenCalled(); + }); +}); + describe('OAuth handler — security invariants', () => { it('identity tokens use HMAC-SHA256 (not plaintext)', async () => { const token = await signIdentityToken(TEST_SECRET, {