Skip to content

Commit 2dd29e5

Browse files
stackbilt-adminCodebeastclaude
authored
fix(oauth): route empty-scope initiates through consent screen with defaults (#28) (#32)
The C-1a remediation (commit 256ba06, 2026-04-10) correctly removed the hardcoded ['generate','read'] grant from completeAuthorization() calls — before C-1a, any OAuth-authed caller silently inherited full access regardless of what their token claimed, which was the C-1 privilege escalation bug. Post-C-1a, grant scopes come from oauthReqInfo.scope. The gap: MCP clients like Claude Code and Claude.ai don't include scope in their OAuth initiate. Post-fix, those initiates produced grants with empty scope at every level (top-level grant.scope AND encryptedProps.scopes), silently blocking every subsequent tool call with a "(none)" scopes error. #28/#30 reported the symptom; this commit closes #28's creation-side bug. Design (Option C from the #28 issue body): - Add DEFAULT_CONSENT_SCOPES = ['read', 'generate'] as an explicit constant. - In handleGetAuthorize auto-approve branch: when oauthReqInfo.scope is empty AND the identity token is valid, DO NOT auto-approve. Render the existing renderConsentPage (which was previously defined but never reachable) with DEFAULT_CONSENT_SCOPES, so the user explicitly approves. - In handlePostAuthorize approve branch: substitute DEFAULT_CONSENT_SCOPES when the POSTed oauthReqInfo.scope is empty, matching what the consent page just showed the user. Non-empty scopes are preserved verbatim. Rejected alternatives (explicit so they don't come back): - Client allowlist keyed on clientId (Option A from #28): REJECTED. clientId is attacker-controlled at OAuth client registration. An attacker registers a client with clientId: "claude-code" and inherits the allowlist's hardcoded scopes — same class as C-1, reskinned. - Silent injection of default scopes at auto-approve time: REJECTED. Loses the "user explicitly consented" property that was the whole point of C-1a. One extra hop for empty-scope initiates is an acceptable tradeoff. What this does NOT fix: - Legacy grants with stale encryptedProps.scopes from pre-2026-04-10 code paths (where the old hardcoded path wrote correct top-level grant.scope but no props.scopes). That's #29's read-side fallback in gateway.ts resolveAuth, which lands in a separate PR. - Static-bearer (ea_*) API key path — covered by the separate fix(auth): accept ea_* prefix PR, which is the Phase 0 unblock. Tests (5 new cases, 125/125 total): - Renders consent page when oauthReqInfo.scope is empty (no completeAuthorization call fires) - Auto-approves when scope is non-empty (regression guard for the zero-latency path Claude Code depends on) - handlePostAuthorize injects DEFAULT_CONSENT_SCOPES when approving an empty-scope grant - handlePostAuthorize preserves client-requested scopes verbatim (regression guard) - Deny action still works for empty-scope initiates (no grant minted) Related: #28, #29, #30 on this repo; PR for the ea_* patch lands separately as a Phase 0 unblock. Co-authored-by: Codebeast <codebeast@stackbilt.dev> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a2c0fc9 commit 2dd29e5

2 files changed

Lines changed: 214 additions & 3 deletions

File tree

src/oauth-handler.ts

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@ import type { AuthRequest } from '@cloudflare/workers-oauth-provider';
88
// ── Feature gate: flip to true when ready to accept public signups ──
99
const PUBLIC_SIGNUPS_ENABLED = true;
1010

11+
// #28 fix: default scopes shown on the consent screen when an OAuth client
12+
// initiates without requesting any scope. The C-1a remediation (commit 256ba06,
13+
// 2026-04-10) correctly removed the hardcoded ['generate','read'] grant — any
14+
// caller with an OAuth token was inheriting those scopes regardless of what
15+
// their token claimed — but left no path for clients (notably Claude Code /
16+
// Claude.ai MCP clients) that don't include scope in their OAuth initiate.
17+
// Those requests end up creating grants that are empty at every level
18+
// (grant.scope AND encryptedProps.scopes), silently blocking every subsequent
19+
// tool call. These defaults are NEVER silently applied: they're rendered on
20+
// the consent page and only enter the grant after explicit user approval.
21+
const DEFAULT_CONSENT_SCOPES = ['read', 'generate'];
22+
1123
// --- Shared styles ---
1224

1325
const SHARED_STYLES = `
@@ -469,6 +481,27 @@ async function handleGetAuthorize(request: Request, env: GatewayEnv): Promise<Re
469481
headers: { 'Content-Type': 'text/html' },
470482
});
471483
}
484+
// #28 fix: if the client omitted scope from the OAuth initiate, do
485+
// NOT auto-approve. Render the consent page with DEFAULT_CONSENT_SCOPES
486+
// so the user explicitly approves. Clients that DID request scope still
487+
// hit the zero-latency auto-approve path below — this exception adds
488+
// one consent hop only for empty-scope initiates, which is acceptable
489+
// compared to the alternative (silent grant with no usable scope).
490+
if (!oauthReqInfo.scope?.length) {
491+
const clientInfo = await env.OAUTH_PROVIDER.lookupClient(oauthReqInfo.clientId);
492+
const clientName = clientInfo?.clientName ?? oauthReqInfo.clientId;
493+
return new Response(
494+
renderConsentPage(
495+
clientName,
496+
DEFAULT_CONSENT_SCOPES,
497+
oauthParams,
498+
identityToken,
499+
identity.email,
500+
),
501+
{ headers: { 'Content-Type': 'text/html' } },
502+
);
503+
}
504+
472505
// Auto-approve: skip consent screen to reduce OAuth hop count.
473506
// The user already authenticated (login/signup/social) and we control
474507
// all scopes. Showing a consent page adds latency that can cause
@@ -562,11 +595,20 @@ async function handlePostAuthorize(request: Request, env: GatewayEnv): Promise<R
562595
return Response.redirect(redirectUrl.toString(), 302);
563596
}
564597

565-
// Approve -- complete the OAuth authorization
598+
// Approve -- complete the OAuth authorization.
599+
// #28 fix: if the original OAuth initiate omitted scope, use the consent-
600+
// screen defaults that the user just explicitly approved on the rendered
601+
// page. This is NOT a silent upgrade — the consent page showed exactly
602+
// these scopes and the user clicked approve. Clients that requested scope
603+
// explicitly keep whatever they requested.
604+
const effectiveScope = oauthReqInfo.scope?.length
605+
? oauthReqInfo.scope
606+
: DEFAULT_CONSENT_SCOPES;
607+
566608
const { redirectTo } = await env.OAUTH_PROVIDER.completeAuthorization({
567609
request: oauthReqInfo,
568610
userId: identity.userId,
569-
scope: oauthReqInfo.scope,
611+
scope: effectiveScope,
570612
metadata: {
571613
authorizedAt: new Date().toISOString(),
572614
userEmail: identity.email,
@@ -577,7 +619,7 @@ async function handlePostAuthorize(request: Request, env: GatewayEnv): Promise<R
577619
name: identity.name,
578620
// C-1a remediation: thread the actual OAuth scope grant through
579621
// to the gateway's apiHandler so resolveAuth can enforce it.
580-
scopes: oauthReqInfo.scope,
622+
scopes: effectiveScope,
581623
},
582624
});
583625

test/oauth-handler.test.ts

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,175 @@ describe('OAuth handler — scope labels', () => {
413413
});
414414
});
415415

416+
describe('OAuth handler — #28 empty-scope consent flow', () => {
417+
// When the OAuth client's initiate omits `scope` entirely, C-1a's
418+
// remediation made the gateway silently mint an empty-scope grant —
419+
// every subsequent tool call failed with "(none)". The fix: route
420+
// empty-scope initiates through the consent page with defaults so
421+
// the user explicitly approves. Auto-approve still works for every
422+
// initiate that includes explicit scope (regression guard below).
423+
424+
async function makeIdentityToken(clientId: string, redirectUri: string): Promise<string> {
425+
return signIdentityToken(TEST_SECRET, {
426+
userId: 'u1',
427+
email: 'test@test.com',
428+
name: 'Test',
429+
clientId,
430+
redirectUri,
431+
});
432+
}
433+
434+
it('renders the consent page when oauthReqInfo.scope is empty', async () => {
435+
const emptyScopeAuthRequest: AuthRequest = { ...MOCK_AUTH_REQUEST, scope: [] };
436+
const provider = mockOAuthProvider({
437+
parseAuthRequest: vi.fn(async () => emptyScopeAuthRequest),
438+
});
439+
const env = makeEnv({
440+
OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'],
441+
});
442+
443+
const identityToken = await makeIdentityToken(
444+
MOCK_AUTH_REQUEST.clientId,
445+
MOCK_AUTH_REQUEST.redirectUri,
446+
);
447+
const req = getRequest(`/authorize?identity_token=${encodeURIComponent(identityToken)}`);
448+
const res = await callHandler(req, env);
449+
450+
// Consent page rendered — NOT a redirect to completeAuthorization.
451+
expect(res.status).toBe(200);
452+
expect(res.headers.get('Content-Type')).toContain('text/html');
453+
const body = await res.text();
454+
expect(body).toContain('Authorize access');
455+
expect(body).toContain('read');
456+
expect(body).toContain('generate');
457+
// The consent page carries forward the oauth_params + identity_token
458+
// so the POST-back to /authorize has everything it needs.
459+
expect(body).toContain('identity_token');
460+
expect(body).toContain('action');
461+
462+
// Crucial: no empty grant gets minted at this step.
463+
expect(provider.completeAuthorization).not.toHaveBeenCalled();
464+
});
465+
466+
it('auto-approves (skips consent) when the client explicitly requests scope', async () => {
467+
// Regression guard for the zero-latency path that Claude Code depends on.
468+
const provider = mockOAuthProvider(); // default scope ['generate', 'read']
469+
const env = makeEnv({
470+
OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'],
471+
});
472+
473+
const identityToken = await makeIdentityToken(
474+
MOCK_AUTH_REQUEST.clientId,
475+
MOCK_AUTH_REQUEST.redirectUri,
476+
);
477+
const req = getRequest(`/authorize?identity_token=${encodeURIComponent(identityToken)}`);
478+
const res = await callHandler(req, env);
479+
480+
// Redirect back to client — auto-approve path, no consent hop.
481+
expect(res.status).toBe(302);
482+
expect(provider.completeAuthorization).toHaveBeenCalledOnce();
483+
// The grant uses the client-requested scope verbatim (not the defaults).
484+
expect(provider.completeAuthorization).toHaveBeenCalledWith(
485+
expect.objectContaining({
486+
scope: ['generate', 'read'],
487+
props: expect.objectContaining({ scopes: ['generate', 'read'] }),
488+
}),
489+
);
490+
});
491+
492+
it('handlePostAuthorize injects consent defaults when the original initiate was empty-scope', async () => {
493+
const emptyScopeAuthRequest: AuthRequest = { ...MOCK_AUTH_REQUEST, scope: [] };
494+
const emptyScopeOauthParams = btoa(JSON.stringify(emptyScopeAuthRequest));
495+
496+
const provider = mockOAuthProvider({
497+
parseAuthRequest: vi.fn(async () => emptyScopeAuthRequest),
498+
});
499+
const env = makeEnv({
500+
OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'],
501+
});
502+
503+
const identityToken = await makeIdentityToken(
504+
MOCK_AUTH_REQUEST.clientId,
505+
MOCK_AUTH_REQUEST.redirectUri,
506+
);
507+
508+
const req = formRequest('/authorize', {
509+
action: 'approve',
510+
oauth_params: emptyScopeOauthParams,
511+
identity_token: identityToken,
512+
});
513+
const res = await callHandler(req, env);
514+
515+
expect(res.status).toBe(302);
516+
// The grant was stored with the consent defaults — NOT empty.
517+
expect(provider.completeAuthorization).toHaveBeenCalledWith(
518+
expect.objectContaining({
519+
scope: ['read', 'generate'],
520+
props: expect.objectContaining({ scopes: ['read', 'generate'] }),
521+
}),
522+
);
523+
});
524+
525+
it('handlePostAuthorize preserves the client-requested scope when it is non-empty', async () => {
526+
// Regression guard — explicit scopes are never overwritten.
527+
const oauthParams = makeOAuthParamsB64(); // scope: ['generate', 'read']
528+
const provider = mockOAuthProvider();
529+
const env = makeEnv({
530+
OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'],
531+
});
532+
533+
const identityToken = await makeIdentityToken(
534+
MOCK_AUTH_REQUEST.clientId,
535+
MOCK_AUTH_REQUEST.redirectUri,
536+
);
537+
538+
const req = formRequest('/authorize', {
539+
action: 'approve',
540+
oauth_params: oauthParams,
541+
identity_token: identityToken,
542+
});
543+
const res = await callHandler(req, env);
544+
545+
expect(res.status).toBe(302);
546+
expect(provider.completeAuthorization).toHaveBeenCalledWith(
547+
expect.objectContaining({
548+
scope: ['generate', 'read'],
549+
props: expect.objectContaining({ scopes: ['generate', 'read'] }),
550+
}),
551+
);
552+
});
553+
554+
it('deny action still works for empty-scope initiates (no grant created)', async () => {
555+
const emptyScopeAuthRequest: AuthRequest = { ...MOCK_AUTH_REQUEST, scope: [] };
556+
const emptyScopeOauthParams = btoa(JSON.stringify(emptyScopeAuthRequest));
557+
558+
const provider = mockOAuthProvider({
559+
parseAuthRequest: vi.fn(async () => emptyScopeAuthRequest),
560+
});
561+
const env = makeEnv({
562+
OAUTH_PROVIDER: provider as unknown as GatewayEnv['OAUTH_PROVIDER'],
563+
});
564+
565+
const identityToken = await makeIdentityToken(
566+
MOCK_AUTH_REQUEST.clientId,
567+
MOCK_AUTH_REQUEST.redirectUri,
568+
);
569+
570+
const req = formRequest('/authorize', {
571+
action: 'deny',
572+
oauth_params: emptyScopeOauthParams,
573+
identity_token: identityToken,
574+
});
575+
const res = await callHandler(req, env);
576+
577+
// 302 back to client with access_denied — and crucially, no grant.
578+
expect(res.status).toBe(302);
579+
const location = res.headers.get('Location');
580+
expect(location).toContain('error=access_denied');
581+
expect(provider.completeAuthorization).not.toHaveBeenCalled();
582+
});
583+
});
584+
416585
describe('OAuth handler — security invariants', () => {
417586
it('identity tokens use HMAC-SHA256 (not plaintext)', async () => {
418587
const token = await signIdentityToken(TEST_SECRET, {

0 commit comments

Comments
 (0)