From 69bc37f745844794353d476891dfeaf2e0ace474 Mon Sep 17 00:00:00 2001 From: Time Attakc <89218912+time-attack@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:28:42 -0700 Subject: [PATCH] fix(auth): admin-gated rescope surface for DCR clients stuck on default scope (#3299) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(auth): admin-gated rescope surface for DCR clients stuck on default scope (#1914) DCR clients self-register with source_id='default' + federated_read=['default'] and the registration comment promised 'rescope via the CLI later' — but no rescope surface existed. Adds: - GBrainOAuthProvider.rescopeClient(clientId, { sourceId?, federatedRead? }): single-statement COALESCE update, canonical source-id validation (assertValidSourceId), FK-backed existence check on the write source, friendly errors for pre-v60/v61 schemas and unknown clients. Takes effect for already-issued tokens because verifyAccessToken re-reads oauth_clients. - gbrain auth rescope-client [--source S] [--federated-read a,b] (trusted local CLI). - POST /admin/api/rescope-client (requireAdmin), mirroring the existing register-client / revoke-client admin endpoints. Deliberately does NOT let clients self-widen scope (options a/b from the issue) — fail-closed trust invariant. Fixes #1914 Co-Authored-By: Claude Fable 5 * fix(auth): rescope-client admin endpoint returns 400 (not 500) for nonexistent write source The FK-translated 'Source "x" does not exist' error is a client error; map it to 400 like the sibling validation failures. ('No OAuth client found' is matched first, so the 404 path is unaffected.) Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Garry Tan Co-authored-by: Claude Fable 5 --- src/commands/auth.ts | 60 ++++++++++++++++++++++++++++++ src/commands/serve-http.ts | 32 ++++++++++++++++ src/core/oauth-provider.ts | 61 ++++++++++++++++++++++++++++++ test/oauth.test.ts | 76 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+) diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 6fefd6964..af759b576 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -515,6 +515,60 @@ async function registerClient(name: string, args: string[]) { } } +/** + * v0.42.x (#1914): rescope an existing OAuth client's write source and/or + * federated read scope. This is the operator surface the DCR registration + * comment promised ("rescope via the CLI later") — DCR clients land with + * source_id='default' / federated_read=['default'] and must not self-widen, + * so widening happens here (trusted local CLI) or via the requireAdmin + * /admin/api/rescope-client endpoint. + */ +async function rescopeClient(clientId: string, args: string[]) { + const usage = 'Usage: auth rescope-client [--source SOURCE] [--federated-read SRC1,SRC2,...]'; + if (!clientId) { + console.error(usage); + process.exit(1); + } + let sourceId: string | undefined; + let federatedRead: string[] | undefined; + for (let i = 0; i < args.length; i += 2) { + const flag = args[i]; + const value = args[i + 1]; + if (value === undefined || value.startsWith('--')) { + console.error(`Error: ${flag} requires a value`); + console.error(usage); + process.exit(1); + } + if (flag === '--source') sourceId = value; + else if (flag === '--federated-read') { + federatedRead = value.split(',').map(s => s.trim()).filter(Boolean); + } else { + console.error(`Error: Unknown flag: ${flag}`); + console.error(usage); + process.exit(1); + } + } + if (sourceId === undefined && federatedRead === undefined) { + console.error('Error: pass --source and/or --federated-read'); + console.error(usage); + process.exit(1); + } + try { + await withConfiguredSql(async (sql) => { + const { GBrainOAuthProvider } = await import('../core/oauth-provider.ts'); + const provider = new GBrainOAuthProvider({ sql }); + const result = await provider.rescopeClient(clientId, { sourceId, federatedRead }); + console.log(`OAuth client rescoped: "${result.clientName}" (${result.clientId})\n`); + console.log(` Write source: ${result.sourceId}`); + console.log(` Federated reads: ${result.federatedRead.join(', ') || ''}`); + console.log('\nTakes effect on the client\'s next request (existing tokens included).'); + }); + } catch (e: any) { + console.error('Error:', e.message); + process.exit(1); + } +} + /** * Entry point for the `gbrain auth` CLI subcommand. Also reused by the * direct-script path (see bottom of file) so `bun run src/commands/auth.ts` @@ -556,6 +610,7 @@ export async function runAuth(args: string[]): Promise { return; } case 'register-client': await registerClient(rest[0], rest.slice(1)); return; + case 'rescope-client': await rescopeClient(rest[0], rest.slice(1)); return; case 'revoke-client': await revokeClient(rest[0]); return; case 'test': { const tokenIdx = rest.indexOf('--token'); @@ -593,6 +648,11 @@ Usage: --bound-slug-prefixes Bind submit_agent writes to slug prefixes --bound-max-concurrent Bound submit_agent concurrency (default: 1) --budget-usd-per-day Bound submit_agent daily spend cap + gbrain auth rescope-client [options] Change an existing client's source scope (e.g. a DCR + client stuck on the 'default' source). Only the flags + you pass change; the other axis is left as-is. + --source New write source + --federated-read New read-scope source list gbrain auth revoke-client Hard-delete an OAuth 2.1 client (cascades to tokens + codes) gbrain auth test --token Smoke-test a remote MCP server `); diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index 994d9acfa..60c4f2ee2 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -1567,6 +1567,38 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption } }); + // v0.42.x (#1914): rescope an OAuth client's write source / federated read + // scope. Admin-gated on purpose — DCR clients must never self-widen their + // scope (fail-closed trust); only the operator rescopes, here or via + // `gbrain auth rescope-client`. Source ids are validated by the canonical + // validator inside rescopeClient. + app.post('/admin/api/rescope-client', requireAdmin, express.json(), async (req: Request, res: Response) => { + try { + const { clientId, sourceId, federatedRead } = req.body ?? {}; + if (!clientId || typeof clientId !== 'string') { + res.status(400).json({ error: 'clientId required' }); + return; + } + if (federatedRead !== undefined && + !(Array.isArray(federatedRead) && federatedRead.every((s: unknown) => typeof s === 'string'))) { + res.status(400).json({ error: 'federatedRead must be an array of source id strings' }); + return; + } + if (sourceId !== undefined && typeof sourceId !== 'string') { + res.status(400).json({ error: 'sourceId must be a string' }); + return; + } + const result = await oauthProvider.rescopeClient(clientId, { sourceId, federatedRead }); + res.json(result); + } catch (e) { + const message = e instanceof Error ? e.message : 'Rescope failed'; + const status = /No OAuth client found/.test(message) ? 404 + : /Invalid source_id|requires --source|cannot be empty|does not exist/.test(message) ? 400 + : 500; + res.status(status).json({ error: message }); + } + }); + // Revoke OAuth client app.post('/admin/api/revoke-client', requireAdmin, express.json(), async (req: Request, res: Response) => { try { diff --git a/src/core/oauth-provider.ts b/src/core/oauth-provider.ts index 46c2b16c7..17f383a7b 100644 --- a/src/core/oauth-provider.ts +++ b/src/core/oauth-provider.ts @@ -24,6 +24,7 @@ import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/serv import type { AuthInfo as SdkAuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; import { InvalidTokenError, InvalidClientMetadataError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; import { hashToken, generateToken, isUndefinedColumnError } from './utils.ts'; +import { assertValidSourceId } from './source-id.ts'; import { hasScope, assertAllowedScopes, parseScopeString, InvalidScopeError } from './scope.ts'; import type { AuthInfo as CoreAuthInfo } from './operations.ts'; import { parseLegacyTokenScope } from './legacy-token-scope.ts'; @@ -1006,6 +1007,66 @@ export class GBrainOAuthProvider implements OAuthServerProvider { return { clientId, clientSecret }; } + /** + * v0.42.x (#1914): admin-gated rescope for an existing OAuth client. + * + * DCR clients self-register with source_id='default' + + * federated_read=['default'] and MUST NOT be able to widen their own + * scope (fail-closed trust). This is the trusted-operator surface that + * changes it afterward: `gbrain auth rescope-client` (local CLI) and + * POST /admin/api/rescope-client (requireAdmin) both route here. + * + * Omitted fields are left untouched (COALESCE). Takes effect on the + * client's NEXT request even for already-issued tokens, because + * verifyAccessToken re-reads oauth_clients on every verification. + */ + async rescopeClient( + clientId: string, + opts: { sourceId?: string; federatedRead?: string[] }, + ): Promise<{ clientId: string; clientName: string; sourceId: string; federatedRead: string[] }> { + const { sourceId, federatedRead } = opts; + if (sourceId === undefined && federatedRead === undefined) { + throw new Error('rescope-client requires --source and/or --federated-read'); + } + if (sourceId !== undefined) assertValidSourceId(sourceId); + if (federatedRead !== undefined) { + if (federatedRead.length === 0) { + throw new Error('--federated-read cannot be empty (pass at least one source id)'); + } + for (const s of federatedRead) assertValidSourceId(s); + } + let rows: Record[]; + try { + rows = await this.sql` + UPDATE oauth_clients + SET source_id = COALESCE(${sourceId ?? null}::text, source_id), + federated_read = COALESCE(${federatedRead ? pgArray(federatedRead) : null}::text[], federated_read) + WHERE client_id = ${clientId} + RETURNING client_id, client_name, source_id, federated_read + `; + } catch (err) { + if (isUndefinedColumnError(err, 'source_id') || isUndefinedColumnError(err, 'federated_read')) { + throw new Error('rescope-client requires an up-to-date OAuth schema; run `gbrain apply-migrations --yes` and retry.'); + } + // FK oauth_clients.source_id → sources(id): translate the raw 23503 + // into an actionable message. + if ((err as { code?: string })?.code === '23503') { + throw new Error(`Source "${sourceId}" does not exist. Create it first: gbrain sources add ${sourceId} ...`); + } + throw err; + } + if (rows.length === 0) { + throw new Error(`No OAuth client found with id "${clientId}"`); + } + const row = rows[0]; + return { + clientId: row.client_id as string, + clientName: (row.client_name as string | null) ?? '', + sourceId: (row.source_id as string | null) ?? 'default', + federatedRead: Array.isArray(row.federated_read) ? (row.federated_read as string[]) : [], + }; + } + // ------------------------------------------------------------------------- // Internal: Issue access + optional refresh tokens // ------------------------------------------------------------------------- diff --git a/test/oauth.test.ts b/test/oauth.test.ts index 43c484462..66eb7d3ee 100644 --- a/test/oauth.test.ts +++ b/test/oauth.test.ts @@ -166,6 +166,82 @@ describe('client registration', () => { }); }); +// --------------------------------------------------------------------------- +// rescopeClient (#1914) — admin-gated rescope of a DCR-defaulted client +// --------------------------------------------------------------------------- + +describe('rescopeClient', () => { + beforeAll(async () => { + // oauth_clients.source_id has FK → sources(id); create the targets. + for (const id of ['wiki', 'essays', 'alpha', 'gamma']) { + await sql`INSERT INTO sources (id, name) VALUES (${id}, ${id}) ON CONFLICT (id) DO NOTHING`; + } + }); + + test('DCR client stuck on default gets rescoped; existing tokens pick it up', async () => { + // Simulate the DCR path: self-registered client lands with + // source_id='default', federated_read=['default']. client_credentials + // over DCR needs the explicit --enable-dcr-insecure opt-in, so build a + // provider with that flag just for this registration. + const dcrProvider = new GBrainOAuthProvider({ sql, tokenTtl: 60, allowClientCredentialsDcr: true }); + const dcr = await dcrProvider.clientsStore.registerClient!({ + client_name: 'dcr-stuck-client', + redirect_uris: [], + grant_types: ['client_credentials'], + scope: 'read', + token_endpoint_auth_method: 'client_secret_post', + } as any); + const clientId = dcr.client_id; + const [before] = await sql`SELECT source_id, federated_read FROM oauth_clients WHERE client_id = ${clientId}`; + expect(before.source_id).toBe('default'); + expect(before.federated_read).toEqual(['default']); + + // Issue a token BEFORE the rescope — it must see the new scope after. + const tokens = await provider.exchangeClientCredentials(clientId, dcr.client_secret!, 'read'); + + const result = await provider.rescopeClient(clientId, { + sourceId: 'wiki', + federatedRead: ['wiki', 'essays'], + }); + expect(result.sourceId).toBe('wiki'); + expect(result.federatedRead).toEqual(['wiki', 'essays']); + + const authInfo = await provider.verifyAccessToken(tokens.access_token) as unknown as CoreAuthInfo; + expect(authInfo.sourceId).toBe('wiki'); + expect(authInfo.allowedSources).toEqual(['wiki', 'essays']); + }); + + test('partial rescope leaves the other axis untouched', async () => { + const { clientId } = await provider.registerClientManual( + 'partial-rescope', ['client_credentials'], 'read', [], 'alpha', ['alpha', 'beta'], + ); + const result = await provider.rescopeClient(clientId, { federatedRead: ['beta'] }); + expect(result.sourceId).toBe('alpha'); // untouched + expect(result.federatedRead).toEqual(['beta']); + + const result2 = await provider.rescopeClient(clientId, { sourceId: 'gamma' }); + expect(result2.sourceId).toBe('gamma'); + expect(result2.federatedRead).toEqual(['beta']); // untouched + }); + + test('rejects invalid source ids, empty federated list, no-op calls, unknown client', async () => { + const { clientId } = await provider.registerClientManual( + 'rescope-validation', ['client_credentials'], 'read', + ); + await expect(provider.rescopeClient(clientId, { sourceId: '../etc' })).rejects.toThrow('Invalid source_id'); + await expect(provider.rescopeClient(clientId, { federatedRead: ['ok', 'Not Valid!'] })).rejects.toThrow('Invalid source_id'); + await expect(provider.rescopeClient(clientId, { federatedRead: [] })).rejects.toThrow('cannot be empty'); + await expect(provider.rescopeClient(clientId, {})).rejects.toThrow('requires --source and/or --federated-read'); + await expect(provider.rescopeClient('gbrain_cl_nonexistent', { sourceId: 'wiki' })).rejects.toThrow('No OAuth client found'); + // FK: write source must exist in sources(id). + await expect(provider.rescopeClient(clientId, { sourceId: 'no-such-source' })).rejects.toThrow('does not exist'); + + // Validation failures must not have mutated the row. + const [row] = await sql`SELECT source_id FROM oauth_clients WHERE client_id = ${clientId}`; + expect(row.source_id).toBe('default'); + }); +}); + // --------------------------------------------------------------------------- // Client Credentials Exchange // ---------------------------------------------------------------------------