mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
feat(oauth): accept token_ttl_seconds at Dynamic Client Registration, clamped to admin policy (#2179)
POST /register now accepts an optional token_ttl_seconds field (RFC 7591 extension). The server clamps it into an admin-configured window (oauth.dcr_ttl_min_seconds / oauth.dcr_ttl_max_seconds config keys; defaults 300s..7d), persists it as the client's per-client TTL override (oauth_clients.token_ttl), and echoes the EFFECTIVE value back as token_ttl_seconds in the registration response. Fail-safe posture: absent/malformed -> server default; out-of-range -> clamped, never rejected; pre-migration schemas without the token_ttl column keep registering (no echo). The MCP SDK's /register handler strips unknown body members before they reach the clients store, so serve-http parses the raw body in a middleware and carries the value through an AsyncLocalStorage context. Root-cause follow-through: the per-client token_ttl lookup moved from exchangeClientCredentials into issueTokens, so authorization_code (the DCR default grant) and refresh issuance honor the override too — previously only client_credentials did. Tests: clamp boundaries (below/at/in/at/above, floor, inverted window), store-level persistence + echo + no-context back-compat, cross-grant TTL enforcement (test/oauth-dcr-ttl.test.ts), and a DB-gated wire-level e2e in test/e2e/serve-http-oauth.test.ts. Docs: docs/mcp/DEPLOY.md. Reported-by: @asabirov (#2179) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Claude Fable 5
parent
032af6e5f7
commit
d2a432dfd7
@@ -675,6 +675,54 @@ describeE2E('serve-http OAuth 2.1 E2E (v0.26.1 + v0.26.2 + v0.26.3)', () => {
|
||||
}
|
||||
}, 15_000);
|
||||
|
||||
// =========================================================================
|
||||
// #2179: DCR token_ttl_seconds — wire-level clamp + echo
|
||||
// =========================================================================
|
||||
//
|
||||
// The unit tests in test/oauth-dcr-ttl.test.ts prove the store-level clamp;
|
||||
// this is the HTTP seam: the MCP SDK's /register handler STRIPS unknown
|
||||
// body members, so the field only works if serve-http's middleware carries
|
||||
// it through dcrRegistrationContext. A request above the default max (7d)
|
||||
// must come back clamped, not rejected — and the minted token must match.
|
||||
|
||||
test('DCR /register accepts token_ttl_seconds, clamps to policy, echoes effective value (#2179)', async () => {
|
||||
const res = await fetch(`${BASE}/register`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
client_name: 'e2e-dcr-ttl',
|
||||
redirect_uris: ['https://example.com/cb'],
|
||||
grant_types: ['authorization_code'],
|
||||
token_endpoint_auth_method: 'client_secret_basic',
|
||||
scope: 'read',
|
||||
token_ttl_seconds: 365 * 24 * 3600, // way above the 7d default max
|
||||
}),
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
const body = await res.json() as any;
|
||||
if (body.client_id) dcrClientIds.push(body.client_id);
|
||||
|
||||
// Echoed effective value = clamped to the default max (7 days).
|
||||
expect(body.token_ttl_seconds).toBe(7 * 24 * 3600);
|
||||
|
||||
// And a client that omits the field gets no echo (backward compatible).
|
||||
const res2 = await fetch(`${BASE}/register`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
client_name: 'e2e-dcr-no-ttl',
|
||||
redirect_uris: ['https://example.com/cb'],
|
||||
grant_types: ['authorization_code'],
|
||||
token_endpoint_auth_method: 'client_secret_basic',
|
||||
scope: 'read',
|
||||
}),
|
||||
});
|
||||
expect(res2.ok).toBe(true);
|
||||
const body2 = await res2.json() as any;
|
||||
if (body2.client_id) dcrClientIds.push(body2.client_id);
|
||||
expect(body2.token_ttl_seconds).toBeUndefined();
|
||||
}, 15_000);
|
||||
|
||||
// =========================================================================
|
||||
// v0.26.2: revoke-client CLI subprocess test
|
||||
// =========================================================================
|
||||
|
||||
@@ -0,0 +1,209 @@
|
||||
/**
|
||||
* #2179 — DCR `token_ttl_seconds`: clamp boundaries, persistence, response
|
||||
* echo, and per-client TTL enforcement across grant paths.
|
||||
*
|
||||
* The wire path (serve-http /register middleware → SDK handler → store) is
|
||||
* exercised at the store boundary here: the middleware's only job is to put
|
||||
* the parsed number into `dcrRegistrationContext`, which these tests do
|
||||
* directly. Setup mirrors test/oauth.test.ts (in-memory PGLite).
|
||||
*/
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGlite } from '@electric-sql/pglite';
|
||||
import { vector } from '@electric-sql/pglite/vector';
|
||||
import { pg_trgm } from '@electric-sql/pglite/contrib/pg_trgm';
|
||||
import {
|
||||
GBrainOAuthProvider,
|
||||
clampDcrTokenTtl,
|
||||
dcrRegistrationContext,
|
||||
DEFAULT_DCR_TTL_MIN_SECONDS,
|
||||
DEFAULT_DCR_TTL_MAX_SECONDS,
|
||||
} from '../src/core/oauth-provider.ts';
|
||||
import { PGLITE_SCHEMA_SQL } from '../src/core/pglite-schema.ts';
|
||||
|
||||
let db: PGlite;
|
||||
let sql: (strings: TemplateStringsArray, ...values: unknown[]) => Promise<any>;
|
||||
|
||||
beforeAll(async () => {
|
||||
db = new PGlite({ extensions: { vector, pg_trgm } });
|
||||
await db.exec(PGLITE_SCHEMA_SQL);
|
||||
sql = async (strings: TemplateStringsArray, ...values: unknown[]) => {
|
||||
const query = strings.reduce((acc, str, i) => acc + str + (i < values.length ? `$${i + 1}` : ''), '');
|
||||
const result = await db.query(query, values as any[]);
|
||||
return result.rows;
|
||||
};
|
||||
}, 30_000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (db) await db.close();
|
||||
}, 15_000);
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// clampDcrTokenTtl — pure clamp boundaries
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('clampDcrTokenTtl', () => {
|
||||
test('below min clamps up to min', () => {
|
||||
expect(clampDcrTokenTtl(10, 300, 600)).toBe(300);
|
||||
});
|
||||
|
||||
test('exactly min passes through', () => {
|
||||
expect(clampDcrTokenTtl(300, 300, 600)).toBe(300);
|
||||
});
|
||||
|
||||
test('in-range passes through', () => {
|
||||
expect(clampDcrTokenTtl(450, 300, 600)).toBe(450);
|
||||
});
|
||||
|
||||
test('exactly max passes through', () => {
|
||||
expect(clampDcrTokenTtl(600, 300, 600)).toBe(600);
|
||||
});
|
||||
|
||||
test('above max clamps down to max', () => {
|
||||
expect(clampDcrTokenTtl(999_999, 300, 600)).toBe(600);
|
||||
});
|
||||
|
||||
test('zero and negative clamp up to min (never reject)', () => {
|
||||
expect(clampDcrTokenTtl(0, 300, 600)).toBe(300);
|
||||
expect(clampDcrTokenTtl(-5, 300, 600)).toBe(300);
|
||||
});
|
||||
|
||||
test('non-integer request floors before clamping', () => {
|
||||
expect(clampDcrTokenTtl(450.9, 300, 600)).toBe(450);
|
||||
});
|
||||
|
||||
test('inverted window collapses to min bound', () => {
|
||||
expect(clampDcrTokenTtl(500, 600, 300)).toBe(600);
|
||||
});
|
||||
|
||||
test('non-positive min is floored to 1', () => {
|
||||
expect(clampDcrTokenTtl(0, 0, 600)).toBe(1);
|
||||
});
|
||||
|
||||
test('defaults are 300s..7d', () => {
|
||||
expect(clampDcrTokenTtl(1)).toBe(DEFAULT_DCR_TTL_MIN_SECONDS);
|
||||
expect(clampDcrTokenTtl(Number.MAX_SAFE_INTEGER)).toBe(DEFAULT_DCR_TTL_MAX_SECONDS);
|
||||
expect(DEFAULT_DCR_TTL_MIN_SECONDS).toBe(300);
|
||||
expect(DEFAULT_DCR_TTL_MAX_SECONDS).toBe(7 * 24 * 3600);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// registerClient — persistence + response echo through dcrRegistrationContext
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
function makeProvider(bounds?: { min?: number; max?: number }) {
|
||||
return new GBrainOAuthProvider({
|
||||
sql,
|
||||
tokenTtl: 60,
|
||||
allowClientCredentialsDcr: true,
|
||||
dcrTtlMinSeconds: bounds?.min,
|
||||
dcrTtlMaxSeconds: bounds?.max,
|
||||
});
|
||||
}
|
||||
|
||||
const DCR_METADATA = {
|
||||
client_name: 'dcr-ttl-test',
|
||||
redirect_uris: [],
|
||||
grant_types: ['client_credentials'],
|
||||
scope: 'read',
|
||||
token_endpoint_auth_method: 'client_secret_post',
|
||||
} as any;
|
||||
|
||||
function registerWithTtl(provider: GBrainOAuthProvider, tokenTtlSeconds?: number) {
|
||||
return dcrRegistrationContext.run({ tokenTtlSeconds }, () =>
|
||||
provider.clientsStore.registerClient!({ ...DCR_METADATA }),
|
||||
);
|
||||
}
|
||||
|
||||
describe('DCR registration with token_ttl_seconds (#2179)', () => {
|
||||
test('in-range request persists and echoes verbatim; /token honors it', async () => {
|
||||
const provider = makeProvider({ min: 120, max: 600 });
|
||||
const info = await registerWithTtl(provider, 300);
|
||||
expect((info as any).token_ttl_seconds).toBe(300);
|
||||
|
||||
const [row] = await sql`SELECT token_ttl FROM oauth_clients WHERE client_id = ${info.client_id}`;
|
||||
expect(Number(row.token_ttl)).toBe(300);
|
||||
|
||||
const tokens = await provider.exchangeClientCredentials(info.client_id, info.client_secret!, 'read');
|
||||
expect(tokens.expires_in).toBe(300);
|
||||
});
|
||||
|
||||
test('below-min request clamps up, is echoed clamped, never rejected', async () => {
|
||||
const provider = makeProvider({ min: 120, max: 600 });
|
||||
const info = await registerWithTtl(provider, 10);
|
||||
expect((info as any).token_ttl_seconds).toBe(120);
|
||||
|
||||
const [row] = await sql`SELECT token_ttl FROM oauth_clients WHERE client_id = ${info.client_id}`;
|
||||
expect(Number(row.token_ttl)).toBe(120);
|
||||
});
|
||||
|
||||
test('above-max request clamps down, is echoed clamped, never rejected', async () => {
|
||||
const provider = makeProvider({ min: 120, max: 600 });
|
||||
const info = await registerWithTtl(provider, 86_400);
|
||||
expect((info as any).token_ttl_seconds).toBe(600);
|
||||
|
||||
const tokens = await provider.exchangeClientCredentials(info.client_id, info.client_secret!, 'read');
|
||||
expect(tokens.expires_in).toBe(600);
|
||||
});
|
||||
|
||||
test('absent request → no echo, server default TTL applies', async () => {
|
||||
const provider = makeProvider({ min: 120, max: 600 });
|
||||
const info = await registerWithTtl(provider, undefined);
|
||||
expect((info as any).token_ttl_seconds).toBeUndefined();
|
||||
|
||||
const [row] = await sql`SELECT token_ttl FROM oauth_clients WHERE client_id = ${info.client_id}`;
|
||||
expect(row.token_ttl).toBeNull();
|
||||
|
||||
const tokens = await provider.exchangeClientCredentials(info.client_id, info.client_secret!, 'read');
|
||||
expect(tokens.expires_in).toBe(60); // provider default
|
||||
});
|
||||
|
||||
test('registration outside any DCR context behaves exactly as before', async () => {
|
||||
const provider = makeProvider({ min: 120, max: 600 });
|
||||
const info = await provider.clientsStore.registerClient!({ ...DCR_METADATA });
|
||||
expect((info as any).token_ttl_seconds).toBeUndefined();
|
||||
const [row] = await sql`SELECT token_ttl FROM oauth_clients WHERE client_id = ${info.client_id}`;
|
||||
expect(row.token_ttl).toBeNull();
|
||||
});
|
||||
|
||||
test('unset bounds use the exported defaults', async () => {
|
||||
const provider = makeProvider();
|
||||
const info = await registerWithTtl(provider, 1);
|
||||
expect((info as any).token_ttl_seconds).toBe(DEFAULT_DCR_TTL_MIN_SECONDS);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// issueTokens — per-client token_ttl honored on the authorization_code path
|
||||
// (DCR clients default to authorization_code, so the override must not be
|
||||
// client_credentials-only)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('per-client token_ttl across grant paths (#2179)', () => {
|
||||
test('authorization_code exchange honors oauth_clients.token_ttl', async () => {
|
||||
const provider = makeProvider();
|
||||
const { clientId } = await provider.registerClientManual(
|
||||
'dcr-ttl-authcode', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
await sql`UPDATE oauth_clients SET token_ttl = ${222} WHERE client_id = ${clientId}`;
|
||||
const client = (await provider.clientsStore.getClient(clientId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(client, {
|
||||
codeChallenge: 'test-challenge-hash',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
state: 'ttl-state',
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
|
||||
const tokens = await provider.exchangeAuthorizationCode(client, code);
|
||||
expect(tokens.expires_in).toBe(222);
|
||||
|
||||
// Refresh issuance honors it too.
|
||||
const refreshed = await provider.exchangeRefreshToken(client, tokens.refresh_token!);
|
||||
expect(refreshed.expires_in).toBe(222);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user