mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
v0.26.1 fix(oauth): client_credentials tokens rejected by MCP bearer auth (#577)
* fix(oauth): client_credentials tokens rejected by MCP bearer auth
Three bugs found in production when connecting Claude Code via Tailscale:
1. Token validation fails with 'Token has no expiration time'
- Root cause: postgres driver with prepare:false returns expires_at as
string, but MCP SDK's bearerAuth middleware checks typeof === 'number'
- Fix: Number(row.expires_at) in verifyAccessToken
2. OAuth metadata missing client_credentials grant type
- Root cause: MCP SDK hardcodes ['authorization_code', 'refresh_token']
in mcpAuthRouter's .well-known endpoint
- Fix: middleware intercepts metadata response and appends
'client_credentials' before it reaches the client
- Claude Code's native OAuth auto-discovery now finds the CC flow
3. Express 5 compatibility fixes
- trust proxy: 'loopback' for reverse proxy deployments (Caddy/Tailscale)
without this, express-rate-limit throws ERR_ERL_UNEXPECTED_X_FORWARDED_FOR
- /admin/* wildcard → /admin/{*path} (Express 5 named param syntax)
* test(oauth): add regression tests for v0.26.1 fixes
Unit test (oauth.test.ts):
- expiresAt is always a number, not string — SDK bearerAuth compat
Integration tests (serve-http-oauth.test.ts, 7 cases):
- client_credentials token accepted at /mcp (the actual regression)
- token expires_in matches server TTL
- OAuth metadata includes client_credentials grant type
- token endpoint discoverable from metadata
- admin dashboard serves SPA (Express 5 wildcard fix)
- X-Forwarded-For doesn't crash rate limiter (trust proxy fix)
- read-only token cannot call write operations (scope enforcement)
42 tests, 0 failures, 172 assertions.
* test(e2e): full E2E suite for serve-http OAuth 2.1 (15 cases)
Spins up a real gbrain serve --http against real Postgres, registers an
OAuth client, mints tokens via client_credentials, and exercises the full
MCP JSON-RPC pipeline end-to-end.
E2E cases (test/e2e/serve-http-oauth.test.ts):
- mint token via client_credentials grant
- minted token accepted at /mcp — tools/list returns tools
- minted token works for tools/call — search executes
- expired/invalid token rejected at /mcp
- missing Authorization header returns 401
- OAuth metadata includes all three grant types
- OAuth metadata issuer matches public URL
- admin dashboard serves SPA (Express 5 wildcard fix)
- admin sub-routes serve SPA fallback
- X-Forwarded-For doesn't crash rate limiter
- read-only token rejected for write operations
- write-scoped token can call read operations
- health endpoint works without auth
- multiple tokens work independently
- wrong client_secret rejected at token endpoint
Unit test addition (test/oauth.test.ts):
- expiresAt is always typeof number (SDK bearerAuth compat)
Total: 50 tests, 0 failures, 201 assertions.
---------
Co-authored-by: Wintermute <wintermute@garrytan.com>
This commit is contained in:
co-authored by
Wintermute
parent
3c032d79ec
commit
d01a921e01
@@ -82,6 +82,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
|
||||
// Express 5 app
|
||||
const app = express();
|
||||
app.set('trust proxy', 'loopback'); // Caddy/Tailscale reverse proxy on localhost
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Cookie parsing — required for /admin auth (express 5 has no built-in)
|
||||
@@ -155,7 +156,25 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
};
|
||||
}
|
||||
|
||||
app.use(mcpAuthRouter(authRouterOptions));
|
||||
const authRouter = mcpAuthRouter(authRouterOptions);
|
||||
|
||||
// Patch the SDK's OAuth metadata to include client_credentials grant type.
|
||||
// The SDK hardcodes ['authorization_code', 'refresh_token'] — we intercept
|
||||
// the response and add client_credentials before it reaches the client.
|
||||
app.use((req, res, next) => {
|
||||
if (req.path === '/.well-known/oauth-authorization-server' && req.method === 'GET') {
|
||||
const origJson = res.json.bind(res);
|
||||
(res as any).json = (body: any) => {
|
||||
if (body?.grant_types_supported && !body.grant_types_supported.includes('client_credentials')) {
|
||||
body.grant_types_supported.push('client_credentials');
|
||||
}
|
||||
return origJson(body);
|
||||
};
|
||||
}
|
||||
next();
|
||||
});
|
||||
|
||||
app.use(authRouter);
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Health check
|
||||
@@ -326,7 +345,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
if (fs.existsSync(adminDistPath)) {
|
||||
app.use('/admin', express.static(adminDistPath));
|
||||
// SPA fallback: serve index.html for all unmatched /admin/* routes
|
||||
app.get('/admin/*', (req: Request, res: Response, next: NextFunction) => {
|
||||
app.get('/admin/{*path}', (req: Request, res: Response, next: NextFunction) => {
|
||||
// Skip API and events routes
|
||||
if (req.path.startsWith('/admin/api/') || req.path === '/admin/events' || req.path === '/admin/login') {
|
||||
return next();
|
||||
|
||||
@@ -300,7 +300,7 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
token,
|
||||
clientId: row.client_id as string,
|
||||
scopes: (row.scopes as string[]) || [],
|
||||
expiresAt: row.expires_at as number,
|
||||
expiresAt: Number(row.expires_at),
|
||||
resource: row.resource ? new URL(row.resource as string) : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -0,0 +1,291 @@
|
||||
/**
|
||||
* E2E tests for serve-http.ts OAuth 2.1 fixes (v0.26.1).
|
||||
*
|
||||
* Spins up a real `gbrain serve --http` against real Postgres, registers an
|
||||
* OAuth client, mints tokens, and exercises the full MCP JSON-RPC pipeline
|
||||
* end-to-end. Catches the three bugs fixed in v0.26.1:
|
||||
*
|
||||
* 1. client_credentials tokens rejected at /mcp (expiresAt string vs number)
|
||||
* 2. OAuth metadata missing client_credentials grant type
|
||||
* 3. Express 5 trust proxy + admin SPA wildcard
|
||||
*
|
||||
* Run: GBRAIN_DATABASE_URL=... bun test test/e2e/serve-http-oauth.test.ts
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { hasDatabase } from './helpers.ts';
|
||||
|
||||
const skip = !hasDatabase();
|
||||
const describeE2E = skip ? describe.skip : describe;
|
||||
|
||||
if (skip) {
|
||||
console.log('Skipping E2E serve-http-oauth tests (DATABASE_URL not set)');
|
||||
}
|
||||
|
||||
const PORT = 19131; // Avoid collision with production 3131
|
||||
const BASE = `http://localhost:${PORT}`;
|
||||
|
||||
describeE2E('serve-http OAuth 2.1 E2E (v0.26.1)', () => {
|
||||
let serverProcess: ReturnType<typeof import('child_process').spawn> | null = null;
|
||||
let clientId: string;
|
||||
let clientSecret: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
const { execSync, spawn } = await import('child_process');
|
||||
|
||||
// Register a test OAuth client via CLI
|
||||
const regOutput = execSync(
|
||||
'bun run src/cli.ts auth register-client e2e-oauth-test --grant-types client_credentials --scopes "read write"',
|
||||
{ cwd: process.cwd(), encoding: 'utf8' }
|
||||
);
|
||||
const idMatch = regOutput.match(/Client ID:\s+(gbrain_cl_\S+)/);
|
||||
const secretMatch = regOutput.match(/Client Secret:\s+(gbrain_cs_\S+)/);
|
||||
if (!idMatch || !secretMatch) throw new Error('Failed to register test client:\n' + regOutput);
|
||||
clientId = idMatch[1];
|
||||
clientSecret = secretMatch[1];
|
||||
|
||||
// Start the HTTP server
|
||||
serverProcess = spawn('bun', [
|
||||
'run', 'src/cli.ts', 'serve', '--http',
|
||||
'--port', String(PORT),
|
||||
'--public-url', `http://localhost:${PORT}`,
|
||||
], {
|
||||
cwd: process.cwd(),
|
||||
env: process.env,
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
});
|
||||
|
||||
// Collect stderr for debugging failures
|
||||
let stderr = '';
|
||||
serverProcess.stderr?.on('data', (d: Buffer) => { stderr += d.toString(); });
|
||||
|
||||
// Wait for server to be ready (up to 15s)
|
||||
let ready = false;
|
||||
for (let i = 0; i < 30; i++) {
|
||||
try {
|
||||
const res = await fetch(`${BASE}/health`);
|
||||
if (res.ok) { ready = true; break; }
|
||||
} catch {}
|
||||
await new Promise(r => setTimeout(r, 500));
|
||||
}
|
||||
if (!ready) throw new Error('Server failed to start within 15s.\nstderr: ' + stderr.slice(-500));
|
||||
}, 30_000);
|
||||
|
||||
afterAll(async () => {
|
||||
// Kill server
|
||||
if (serverProcess) {
|
||||
serverProcess.kill('SIGTERM');
|
||||
await new Promise(r => setTimeout(r, 1000));
|
||||
if (!serverProcess.killed) serverProcess.kill('SIGKILL');
|
||||
}
|
||||
// Revoke test client
|
||||
try {
|
||||
const { execSync } = await import('child_process');
|
||||
execSync(`bun run src/cli.ts auth revoke-client "${clientId}"`,
|
||||
{ cwd: process.cwd(), encoding: 'utf8', stdio: 'pipe' });
|
||||
} catch {}
|
||||
});
|
||||
|
||||
// Helper: mint a token with given scopes
|
||||
async function mintToken(scope = 'read write'): Promise<{ access_token: string; expires_in: number; scope: string }> {
|
||||
const res = await fetch(`${BASE}/token`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
|
||||
body: `grant_type=client_credentials&client_id=${clientId}&client_secret=${clientSecret}&scope=${encodeURIComponent(scope)}`,
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
return res.json() as any;
|
||||
}
|
||||
|
||||
// Helper: call MCP JSON-RPC with a bearer token
|
||||
async function mcpCall(token: string, method: string, params?: any): Promise<Response> {
|
||||
return fetch(`${BASE}/mcp`, {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'Authorization': `Bearer ${token}`,
|
||||
'Content-Type': 'application/json',
|
||||
'Accept': 'application/json, text/event-stream',
|
||||
},
|
||||
body: JSON.stringify({ jsonrpc: '2.0', id: 1, method, ...(params ? { params } : {}) }),
|
||||
});
|
||||
}
|
||||
|
||||
// =========================================================================
|
||||
// Fix 1: client_credentials tokens validate at /mcp
|
||||
// =========================================================================
|
||||
|
||||
test('mint token via client_credentials grant', async () => {
|
||||
const data = await mintToken('read write');
|
||||
expect(data.access_token).toMatch(/^gbrain_at_/);
|
||||
expect(data.expires_in).toBe(3600);
|
||||
expect(data.scope).toContain('read');
|
||||
});
|
||||
|
||||
test('minted token is accepted at /mcp — tools/list returns tools', async () => {
|
||||
const { access_token } = await mintToken('read');
|
||||
const res = await mcpCall(access_token, 'tools/list');
|
||||
|
||||
// Before v0.26.1 fix: 401 {"error":"invalid_token","error_description":"Token has no expiration time"}
|
||||
expect(res.status).not.toBe(401);
|
||||
|
||||
const body = await res.text();
|
||||
expect(body).toContain('tools');
|
||||
expect(body).toContain('search'); // search tool should be in the list
|
||||
expect(body).toContain('query'); // query tool too
|
||||
}, 15_000);
|
||||
|
||||
test('minted token works for tools/call — search executes', async () => {
|
||||
const { access_token } = await mintToken('read');
|
||||
const res = await mcpCall(access_token, 'tools/call', {
|
||||
name: 'search',
|
||||
arguments: { query: 'gbrain', limit: 1 },
|
||||
});
|
||||
|
||||
expect(res.status).not.toBe(401);
|
||||
const body = await res.text();
|
||||
// Should contain search results, not an auth error
|
||||
expect(body).not.toContain('invalid_token');
|
||||
expect(body).toContain('result');
|
||||
}, 15_000);
|
||||
|
||||
test('expired/invalid token is rejected at /mcp', async () => {
|
||||
const res = await mcpCall('gbrain_at_totally_fake_token', 'tools/list');
|
||||
// Invalid tokens should not return 200 with tool results
|
||||
const body = await res.text();
|
||||
expect(body).not.toContain('"tools"');
|
||||
// Should be an error status (401, 403, or 500 depending on SDK error mapping)
|
||||
expect(res.status).toBeGreaterThanOrEqual(400);
|
||||
});
|
||||
|
||||
test('missing Authorization header returns 401', async () => {
|
||||
const res = await fetch(`${BASE}/mcp`, {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
'Accept': 'application/json, text/event-stream',
|
||||
},
|
||||
body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list' }),
|
||||
});
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
// Fix 2: OAuth metadata includes client_credentials
|
||||
// =========================================================================
|
||||
|
||||
test('OAuth AS metadata includes all three grant types', async () => {
|
||||
const res = await fetch(`${BASE}/.well-known/oauth-authorization-server`);
|
||||
expect(res.ok).toBe(true);
|
||||
const meta = await res.json() as any;
|
||||
expect(meta.grant_types_supported).toContain('authorization_code');
|
||||
expect(meta.grant_types_supported).toContain('refresh_token');
|
||||
expect(meta.grant_types_supported).toContain('client_credentials');
|
||||
});
|
||||
|
||||
test('OAuth metadata issuer matches public URL', async () => {
|
||||
const res = await fetch(`${BASE}/.well-known/oauth-authorization-server`);
|
||||
const meta = await res.json() as any;
|
||||
expect(meta.issuer).toBe(`http://localhost:${PORT}/`);
|
||||
expect(meta.token_endpoint).toContain('/token');
|
||||
expect(meta.scopes_supported).toContain('read');
|
||||
expect(meta.scopes_supported).toContain('write');
|
||||
expect(meta.scopes_supported).toContain('admin');
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
// Fix 3: Express 5 compatibility
|
||||
// =========================================================================
|
||||
|
||||
test('admin dashboard serves SPA index.html (not Express error)', async () => {
|
||||
const res = await fetch(`${BASE}/admin/`);
|
||||
const html = await res.text();
|
||||
expect(html).toContain('GBrain Admin');
|
||||
expect(html).not.toContain('<pre>Cannot GET');
|
||||
});
|
||||
|
||||
test('admin sub-routes serve SPA fallback', async () => {
|
||||
const res = await fetch(`${BASE}/admin/agents`);
|
||||
const html = await res.text();
|
||||
expect(html).toContain('GBrain Admin');
|
||||
});
|
||||
|
||||
test('X-Forwarded-For header does not crash server', async () => {
|
||||
const res = await fetch(`${BASE}/health`, {
|
||||
headers: { 'X-Forwarded-For': '10.0.0.1, 172.16.0.1' },
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
const data = await res.json() as any;
|
||||
expect(data.status).toBe('ok');
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
// Scope enforcement
|
||||
// =========================================================================
|
||||
|
||||
test('read-only token is rejected for write operations', async () => {
|
||||
const { access_token } = await mintToken('read');
|
||||
const res = await mcpCall(access_token, 'tools/call', {
|
||||
name: 'put_page',
|
||||
arguments: { slug: 'e2e-scope-test', content: '---\ntitle: test\n---\ntest' },
|
||||
});
|
||||
|
||||
const body = await res.text();
|
||||
// Should be rejected via scope check (403 or JSON-RPC error with scope message)
|
||||
expect(res.status === 403 || body.includes('scope') || body.includes('Insufficient')).toBe(true);
|
||||
}, 15_000);
|
||||
|
||||
test('write-scoped token can call read operations', async () => {
|
||||
const { access_token } = await mintToken('read write');
|
||||
const res = await mcpCall(access_token, 'tools/call', {
|
||||
name: 'search',
|
||||
arguments: { query: 'test', limit: 1 },
|
||||
});
|
||||
|
||||
expect(res.status).not.toBe(401);
|
||||
expect(res.status).not.toBe(403);
|
||||
const body = await res.text();
|
||||
// Should get a result, not an auth error
|
||||
expect(body).not.toContain('invalid_token');
|
||||
expect(body).not.toContain('insufficient_scope');
|
||||
}, 15_000);
|
||||
|
||||
// =========================================================================
|
||||
// Health endpoint (no auth required)
|
||||
// =========================================================================
|
||||
|
||||
test('health endpoint returns OK without auth', async () => {
|
||||
const res = await fetch(`${BASE}/health`);
|
||||
expect(res.ok).toBe(true);
|
||||
const data = await res.json() as any;
|
||||
expect(data.status).toBe('ok');
|
||||
expect(data.version).toBeDefined();
|
||||
expect(data.page_count).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
// Token lifecycle
|
||||
// =========================================================================
|
||||
|
||||
test('multiple tokens can be minted and used independently', async () => {
|
||||
const t1 = await mintToken('read');
|
||||
const t2 = await mintToken('read write');
|
||||
|
||||
// Both should work
|
||||
const r1 = await mcpCall(t1.access_token, 'tools/list');
|
||||
const r2 = await mcpCall(t2.access_token, 'tools/list');
|
||||
|
||||
expect(r1.status).not.toBe(401);
|
||||
expect(r2.status).not.toBe(401);
|
||||
}, 15_000);
|
||||
|
||||
test('wrong client_secret is rejected at token endpoint', async () => {
|
||||
const res = await fetch(`${BASE}/token`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
|
||||
body: `grant_type=client_credentials&client_id=${clientId}&client_secret=gbrain_cs_wrong_secret&scope=read`,
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
const data = await res.json() as any;
|
||||
expect(data.error).toBe('invalid_grant');
|
||||
});
|
||||
});
|
||||
@@ -180,6 +180,21 @@ describe('verifyAccessToken', () => {
|
||||
await expect(provider.verifyAccessToken('nonexistent-token')).rejects.toThrow('Invalid token');
|
||||
});
|
||||
|
||||
test('expiresAt is always a number (not string) — SDK bearerAuth compat', async () => {
|
||||
// Regression: postgres driver with prepare:false returns integers as strings.
|
||||
// MCP SDK's bearerAuth middleware checks typeof === 'number' and rejects strings.
|
||||
// verifyAccessToken must cast to Number() before returning.
|
||||
const { clientId, clientSecret } = await provider.registerClientManual(
|
||||
'typeof-test', ['client_credentials'], 'read',
|
||||
);
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret, 'read');
|
||||
const authInfo = await provider.verifyAccessToken(tokens.access_token);
|
||||
|
||||
expect(typeof authInfo.expiresAt).toBe('number');
|
||||
expect(Number.isNaN(authInfo.expiresAt)).toBe(false);
|
||||
expect(authInfo.expiresAt).toBeGreaterThan(Math.floor(Date.now() / 1000));
|
||||
});
|
||||
|
||||
test('legacy access_tokens fallback works', async () => {
|
||||
// Insert a legacy bearer token
|
||||
const legacyToken = generateToken('gbrain_');
|
||||
|
||||
Reference in New Issue
Block a user