mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 11:22:34 +00:00
fix(security): default dynamic-registration clients to authorization_code
Self-registered DCR clients (the unauthenticated network registration path) previously defaulted to the client_credentials grant, which bypasses the /authorize consent screen. They now default to authorization_code; an explicit client_credentials request is rejected with invalid_client_metadata unless the operator opts in with the new --enable-dcr-insecure flag. A loud stderr WARNING prints at startup whenever DCR is enabled (#1353). Manual CLI/admin client registration is unchanged (operator-trusted). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
213dfa8bc0
commit
07dd05b31c
+2
-1
@@ -2325,7 +2325,8 @@ ADMIN
|
||||
serve MCP server (stdio)
|
||||
serve --http [--port N] HTTP MCP server with OAuth 2.1
|
||||
--token-ttl N Access token TTL in seconds (default: 3600)
|
||||
--enable-dcr Enable Dynamic Client Registration
|
||||
--enable-dcr Enable Dynamic Client Registration (DCR clients default to authorization_code)
|
||||
--enable-dcr-insecure Also allow the consent-bypassing client_credentials grant on DCR (implies --enable-dcr)
|
||||
--public-url URL Public issuer URL (required behind proxy/tunnel)
|
||||
connect <mcp-url> --token <t> Wire Claude Code to a remote gbrain (bearer token)
|
||||
[--install] [--json] Print the paste-ready command, or --install to run it
|
||||
|
||||
@@ -259,6 +259,11 @@ interface ServeHttpOptions {
|
||||
port: number;
|
||||
tokenTtl: number;
|
||||
enableDcr: boolean;
|
||||
/**
|
||||
* #1353: allow the consent-bypassing client_credentials grant on the DCR path.
|
||||
* Off by default; DCR clients default to authorization_code. Implies enableDcr.
|
||||
*/
|
||||
enableDcrInsecure?: boolean;
|
||||
/**
|
||||
* Public URL the server is reachable at (e.g., https://brain.example.com).
|
||||
* Used as the OAuth issuer in discovery metadata. Defaults to
|
||||
@@ -396,7 +401,7 @@ export function skillPublishStatus(publishSkills: boolean): { bannerValue: strin
|
||||
}
|
||||
|
||||
export async function runServeHttp(engine: BrainEngine, options: ServeHttpOptions) {
|
||||
const { port, tokenTtl, enableDcr, publicUrl, logFullParams } = options;
|
||||
const { port, tokenTtl, enableDcr, enableDcrInsecure, publicUrl, logFullParams } = options;
|
||||
// v0.34.1 (#864, D11): default bind flipped from 0.0.0.0 to 127.0.0.1.
|
||||
// gbrain's primary use case is a personal-knowledge brain on a laptop;
|
||||
// the pre-v0.34 default exposed brains on every interface. Server
|
||||
@@ -479,8 +484,28 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
sql,
|
||||
tokenTtl,
|
||||
dcrDisabled: !enableDcr,
|
||||
allowClientCredentialsDcr: enableDcrInsecure === true,
|
||||
});
|
||||
|
||||
// #1353: loud stderr security WARN when DCR is enabled. DCR is an
|
||||
// unauthenticated network registration endpoint; surface the posture change
|
||||
// (and the extra blast radius of --enable-dcr-insecure) so it's visible in
|
||||
// logs, not buried in the neutral "DCR: enabled" banner line.
|
||||
if (enableDcr) {
|
||||
console.error(
|
||||
'SECURITY WARNING: Dynamic Client Registration (--enable-dcr) is ON. ' +
|
||||
'Any network caller can self-register an OAuth client. DCR clients default ' +
|
||||
'to the authorization_code (consent-bearing) grant. See SECURITY.md.',
|
||||
);
|
||||
if (enableDcrInsecure) {
|
||||
console.error(
|
||||
'SECURITY WARNING: --enable-dcr-insecure is ON — self-registered DCR ' +
|
||||
'clients may request the client_credentials grant, which BYPASSES the ' +
|
||||
'/authorize consent screen. Only use this on a trusted network.',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Sweep expired tokens on startup (non-blocking)
|
||||
try {
|
||||
const swept = await oauthProvider.sweepExpiredTokens();
|
||||
@@ -2133,7 +2158,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
║ Engine: ${(config.engine || 'pglite').padEnd(40)}║
|
||||
║ Issuer: ${issuerUrl.origin.padEnd(40)}║
|
||||
║ Clients: ${String((clientCount[0] as any).count).padEnd(40)}║
|
||||
║ DCR: ${(enableDcr ? 'enabled' : 'disabled').padEnd(40)}║
|
||||
║ DCR: ${(enableDcr ? (enableDcrInsecure ? 'enabled (INSECURE: client_credentials)' : 'enabled') : 'disabled').padEnd(40)}║
|
||||
║ Skills: ${skillStatus.bannerValue.padEnd(40)}║
|
||||
║ Token TTL: ${(tokenTtl + 's').padEnd(40)}║
|
||||
╠══════════════════════════════════════════════════════╣
|
||||
|
||||
@@ -85,7 +85,12 @@ export async function runServe(
|
||||
const ttlIdx = args.indexOf('--token-ttl');
|
||||
const tokenTtl = ttlIdx >= 0 ? parseInt(args[ttlIdx + 1]) || 3600 : 3600;
|
||||
|
||||
const enableDcr = args.includes('--enable-dcr');
|
||||
// #1353: --enable-dcr-insecure opts into the consent-bypassing
|
||||
// client_credentials grant on the DCR path. It implies --enable-dcr (you
|
||||
// can't allow insecure DCR clients without DCR). Plain --enable-dcr keeps
|
||||
// the secure default: DCR clients are authorization_code (consent-bearing).
|
||||
const enableDcrInsecure = args.includes('--enable-dcr-insecure');
|
||||
const enableDcr = args.includes('--enable-dcr') || enableDcrInsecure;
|
||||
|
||||
const publicUrlIdx = args.indexOf('--public-url');
|
||||
const publicUrl = publicUrlIdx >= 0 ? args[publicUrlIdx + 1] : undefined;
|
||||
@@ -114,7 +119,7 @@ export async function runServe(
|
||||
const suppressBootstrapToken = args.includes('--suppress-bootstrap-token');
|
||||
|
||||
const { runServeHttp } = await import('./serve-http.ts');
|
||||
await runServeHttp(engine, { port, tokenTtl, enableDcr, publicUrl, logFullParams, bind, suppressBootstrapToken });
|
||||
await runServeHttp(engine, { port, tokenTtl, enableDcr, enableDcrInsecure, publicUrl, logFullParams, bind, suppressBootstrapToken });
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -22,7 +22,7 @@ import type {
|
||||
import type { OAuthServerProvider, AuthorizationParams } from '@modelcontextprotocol/sdk/server/auth/provider.js';
|
||||
import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/server/auth/clients.js';
|
||||
import type { AuthInfo as SdkAuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js';
|
||||
import { InvalidTokenError } from '@modelcontextprotocol/sdk/server/auth/errors.js';
|
||||
import { InvalidTokenError, InvalidClientMetadataError } from '@modelcontextprotocol/sdk/server/auth/errors.js';
|
||||
import { hashToken, generateToken, isUndefinedColumnError } from './utils.ts';
|
||||
import { hasScope, assertAllowedScopes, parseScopeString, InvalidScopeError } from './scope.ts';
|
||||
import type { AuthInfo as CoreAuthInfo } from './operations.ts';
|
||||
@@ -183,6 +183,16 @@ interface GBrainOAuthProviderOptions {
|
||||
* before mcpAuthRouter ran).
|
||||
*/
|
||||
dcrDisabled?: boolean;
|
||||
/**
|
||||
* Allow the consent-bypassing `client_credentials` grant on the unauthenticated
|
||||
* Dynamic Client Registration path. Default false (#1353): a self-registered
|
||||
* DCR client defaults to `authorization_code` (which goes through /authorize
|
||||
* consent), and an explicit `client_credentials` request is rejected. Operators
|
||||
* who genuinely need machine-to-machine DCR clients opt in via
|
||||
* `--enable-dcr-insecure`. Manual CLI / admin registration is unaffected
|
||||
* (operator-trusted, registers grants directly).
|
||||
*/
|
||||
allowClientCredentialsDcr?: boolean;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -190,7 +200,7 @@ interface GBrainOAuthProviderOptions {
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
constructor(private sql: SqlQuery) {}
|
||||
constructor(private sql: SqlQuery, private allowClientCredentialsDcr = false) {}
|
||||
|
||||
async getClient(clientId: string): Promise<OAuthClientInformationFull | undefined> {
|
||||
const rows = await this.sql`
|
||||
@@ -243,6 +253,24 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
// registration entry points share one allow-list.
|
||||
const authMethod = validateTokenEndpointAuthMethod(client.token_endpoint_auth_method);
|
||||
|
||||
// v0.42 (#1353): the DCR path is the unauthenticated network entry point.
|
||||
// `client_credentials` skips /authorize consent entirely, so a self-
|
||||
// registered DCR client must NOT get it by default. Default the grant to
|
||||
// `authorization_code` (the consent-bearing flow) when unspecified, and
|
||||
// reject an explicit `client_credentials` request unless the operator opted
|
||||
// in via `--enable-dcr-insecure`. Manual CLI/admin registration bypasses
|
||||
// this store method, so operators can still mint machine clients directly.
|
||||
const grantTypes = (client.grant_types && client.grant_types.length > 0)
|
||||
? client.grant_types
|
||||
: ['authorization_code'];
|
||||
if (!this.allowClientCredentialsDcr && grantTypes.includes('client_credentials')) {
|
||||
throw new InvalidClientMetadataError(
|
||||
'client_credentials grant is not permitted via dynamic client registration; ' +
|
||||
'restart the server with --enable-dcr-insecure to allow it, or register the ' +
|
||||
'client via the gbrain CLI / admin API.',
|
||||
);
|
||||
}
|
||||
|
||||
const clientId = generateToken('gbrain_cl_');
|
||||
// v0.34.1 (#909): RFC 7591 §2 — clients that authenticate at the token
|
||||
// endpoint via PKCE alone declare `token_endpoint_auth_method: "none"`.
|
||||
@@ -273,7 +301,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
client_id_issued_at, source_id, federated_read)
|
||||
VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'},
|
||||
${pgArray((client.redirect_uris || []).map(String))},
|
||||
${pgArray(client.grant_types || ['client_credentials'])},
|
||||
${pgArray(grantTypes)},
|
||||
${client.scope || ''}, ${authMethod},
|
||||
${now}, ${'default'}, ${pgArray(['default'])})
|
||||
`;
|
||||
@@ -286,7 +314,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
client_id_issued_at, source_id)
|
||||
VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'},
|
||||
${pgArray((client.redirect_uris || []).map(String))},
|
||||
${pgArray(client.grant_types || ['client_credentials'])},
|
||||
${pgArray(grantTypes)},
|
||||
${client.scope || ''}, ${authMethod},
|
||||
${now}, ${'default'})
|
||||
`;
|
||||
@@ -298,7 +326,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
client_id_issued_at)
|
||||
VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'},
|
||||
${pgArray((client.redirect_uris || []).map(String))},
|
||||
${pgArray(client.grant_types || ['client_credentials'])},
|
||||
${pgArray(grantTypes)},
|
||||
${client.scope || ''}, ${authMethod},
|
||||
${now})
|
||||
`;
|
||||
@@ -313,7 +341,7 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
client_id_issued_at)
|
||||
VALUES (${clientId}, ${secretHash}, ${client.client_name || 'unnamed'},
|
||||
${pgArray((client.redirect_uris || []).map(String))},
|
||||
${pgArray(client.grant_types || ['client_credentials'])},
|
||||
${pgArray(grantTypes)},
|
||||
${client.scope || ''}, ${authMethod},
|
||||
${now})
|
||||
`;
|
||||
@@ -350,7 +378,7 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
|
||||
constructor(options: GBrainOAuthProviderOptions) {
|
||||
this.sql = options.sql;
|
||||
this._clientsStore = new GBrainClientsStore(this.sql);
|
||||
this._clientsStore = new GBrainClientsStore(this.sql, options.allowClientCredentialsDcr === true);
|
||||
this.dcrDisabled = options.dcrDisabled === true;
|
||||
this.tokenTtl = options.tokenTtl || 3600;
|
||||
this.refreshTtl = options.refreshTtl || 30 * 24 * 3600;
|
||||
|
||||
+41
-3
@@ -11,7 +11,7 @@ import {
|
||||
} from '../src/core/oauth-provider.ts';
|
||||
import { hashToken, generateToken } from '../src/core/utils.ts';
|
||||
import { PGLITE_SCHEMA_SQL } from '../src/core/pglite-schema.ts';
|
||||
import { InvalidTokenError } from '@modelcontextprotocol/sdk/server/auth/errors.js';
|
||||
import { InvalidTokenError, InvalidClientMetadataError } from '@modelcontextprotocol/sdk/server/auth/errors.js';
|
||||
import type { AuthInfo as CoreAuthInfo } from '../src/core/operations.ts';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -1489,12 +1489,50 @@ describe('v0.41.3 DCR validator (T5)', () => {
|
||||
test('DCR accepts "client_secret_basic" — codex F3 regression', async () => {
|
||||
const reg = await provider.clientsStore.registerClient!({
|
||||
client_name: 'dcr-basic-test',
|
||||
grant_types: ['client_credentials'],
|
||||
grant_types: ['authorization_code'],
|
||||
scope: 'read',
|
||||
redirect_uris: [],
|
||||
redirect_uris: ['https://example.test/cb'],
|
||||
token_endpoint_auth_method: 'client_secret_basic',
|
||||
} as any);
|
||||
expect(reg.client_id).toStartWith('gbrain_cl_');
|
||||
expect(reg.client_secret).toStartWith('gbrain_cs_');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#1353 DCR default-grant hardening', () => {
|
||||
test('DCR rejects explicit client_credentials by default', async () => {
|
||||
await expect(
|
||||
provider.clientsStore.registerClient!({
|
||||
client_name: 'cc-default-test',
|
||||
grant_types: ['client_credentials'],
|
||||
scope: 'read',
|
||||
redirect_uris: [],
|
||||
token_endpoint_auth_method: 'client_secret_post',
|
||||
} as any),
|
||||
).rejects.toThrow(InvalidClientMetadataError);
|
||||
});
|
||||
|
||||
test('DCR defaults to authorization_code when grant_types unspecified', async () => {
|
||||
const reg = await provider.clientsStore.registerClient!({
|
||||
client_name: 'no-grant-test',
|
||||
scope: 'read',
|
||||
redirect_uris: ['https://example.test/cb'],
|
||||
token_endpoint_auth_method: 'none',
|
||||
} as any);
|
||||
const stored = await provider.clientsStore.getClient(reg.client_id);
|
||||
expect(stored?.grant_types).toEqual(['authorization_code']);
|
||||
});
|
||||
|
||||
test('--enable-dcr-insecure (allowClientCredentialsDcr) permits client_credentials', async () => {
|
||||
const insecure = new GBrainOAuthProvider({ sql, allowClientCredentialsDcr: true });
|
||||
const reg = await insecure.clientsStore.registerClient!({
|
||||
client_name: 'cc-allowed-test',
|
||||
grant_types: ['client_credentials'],
|
||||
scope: 'read',
|
||||
redirect_uris: [],
|
||||
token_endpoint_auth_method: 'client_secret_post',
|
||||
} as any);
|
||||
const stored = await insecure.clientsStore.getClient(reg.client_id);
|
||||
expect(stored?.grant_types).toEqual(['client_credentials']);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user