mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(auth): admin-gated rescope surface for DCR clients stuck on default scope (#3299)
* 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 <client_id> [--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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Garry Tan
Claude Fable 5
parent
4b38724aa2
commit
69bc37f745
@@ -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 <client_id> [--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(', ') || '<none>'}`);
|
||||
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<void> {
|
||||
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 <prefix1,prefix2> Bind submit_agent writes to slug prefixes
|
||||
--bound-max-concurrent <n> Bound submit_agent concurrency (default: 1)
|
||||
--budget-usd-per-day <usd> Bound submit_agent daily spend cap
|
||||
gbrain auth rescope-client <client_id> [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 <id> New write source
|
||||
--federated-read <id1,id2,...> New read-scope source list
|
||||
gbrain auth revoke-client <client_id> Hard-delete an OAuth 2.1 client (cascades to tokens + codes)
|
||||
gbrain auth test <url> --token <token> Smoke-test a remote MCP server
|
||||
`);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<string, unknown>[];
|
||||
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
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user