Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 45 additions & 3 deletions src/oauth-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `
Expand Down Expand Up @@ -469,6 +481,27 @@ async function handleGetAuthorize(request: Request, env: GatewayEnv): Promise<Re
headers: { 'Content-Type': 'text/html' },
});
}
// #28 fix: if the client omitted scope from the OAuth initiate, do
// NOT auto-approve. Render the consent page with DEFAULT_CONSENT_SCOPES
// so the user explicitly approves. Clients that DID request scope still
// hit the zero-latency auto-approve path below — this exception adds
// one consent hop only for empty-scope initiates, which is acceptable
// compared to the alternative (silent grant with no usable scope).
if (!oauthReqInfo.scope?.length) {
const clientInfo = await env.OAUTH_PROVIDER.lookupClient(oauthReqInfo.clientId);
const clientName = clientInfo?.clientName ?? oauthReqInfo.clientId;
return new Response(
renderConsentPage(
clientName,
DEFAULT_CONSENT_SCOPES,
oauthParams,
identityToken,
identity.email,
),
{ headers: { 'Content-Type': 'text/html' } },
);
}

// Auto-approve: skip consent screen to reduce OAuth hop count.
// The user already authenticated (login/signup/social) and we control
// all scopes. Showing a consent page adds latency that can cause
Expand Down Expand Up @@ -562,11 +595,20 @@ async function handlePostAuthorize(request: Request, env: GatewayEnv): Promise<R
return Response.redirect(redirectUrl.toString(), 302);
}

// Approve -- complete the OAuth authorization
// Approve -- complete the OAuth authorization.
// #28 fix: if the original OAuth initiate omitted scope, use the consent-
// screen defaults that the user just explicitly approved on the rendered
// page. This is NOT a silent upgrade — the consent page showed exactly
// these scopes and the user clicked approve. Clients that requested scope
// explicitly keep whatever they requested.
const effectiveScope = oauthReqInfo.scope?.length
? oauthReqInfo.scope
: DEFAULT_CONSENT_SCOPES;

const { redirectTo } = await env.OAUTH_PROVIDER.completeAuthorization({
request: oauthReqInfo,
userId: identity.userId,
scope: oauthReqInfo.scope,
scope: effectiveScope,
metadata: {
authorizedAt: new Date().toISOString(),
userEmail: identity.email,
Expand All @@ -577,7 +619,7 @@ async function handlePostAuthorize(request: Request, env: GatewayEnv): Promise<R
name: identity.name,
// C-1a remediation: thread the actual OAuth scope grant through
// to the gateway's apiHandler so resolveAuth can enforce it.
scopes: oauthReqInfo.scope,
scopes: effectiveScope,
},
});

Expand Down
169 changes: 169 additions & 0 deletions test/oauth-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,175 @@ describe('OAuth handler — scope labels', () => {
});
});

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<string> {
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, {
Expand Down
Loading