From 773566c7fa85162d302b70626618b12dfc58e38e Mon Sep 17 00:00:00 2001 From: Suleiman Shahbari Date: Fri, 26 Jun 2026 22:55:40 +0300 Subject: [PATCH] refactor(mcp): harden OAuth empty-token, chain resolver errors, sharpen docs Code quality + docs pass for @gemstack/mcp: - OAuth: reject an empty bearer token ("Authorization: Bearer " with no value) up front with 401 invalid_token instead of forwarding an empty string to verifyToken. Neutralized the framework-specific wording in the oauth2 core docs (any JWT library / introspection endpoint). - handle-deps: chain the original error via { cause } when a @Handle dependency fails to resolve, preserving the stack. - Documented McpResponse.text/json/error and when to prefer error() over throwing. - README: completed the OAuth 2.1 section (real jose-based verifyToken; spelled out that oauth2McpMiddleware AND registerOAuth2Metadata must both be wired, with the discovery rationale); softened the origin framing. Added an empty-token test. Build + 105 tests green. --- .changeset/mcp-quality-pass.md | 10 +++++++ packages/mcp/README.md | 39 ++++++++++++++++++------- packages/mcp/src/McpResponse.ts | 12 ++++++++ packages/mcp/src/auth/oauth2.ts | 17 +++++++---- packages/mcp/src/index.test.ts | 17 ++++++++++- packages/mcp/src/runtime/handle-deps.ts | 1 + 6 files changed, 79 insertions(+), 17 deletions(-) create mode 100644 .changeset/mcp-quality-pass.md diff --git a/.changeset/mcp-quality-pass.md b/.changeset/mcp-quality-pass.md new file mode 100644 index 0000000..ea76560 --- /dev/null +++ b/.changeset/mcp-quality-pass.md @@ -0,0 +1,10 @@ +--- +"@gemstack/mcp": patch +--- + +Quality + docs pass for mcp: + +- OAuth: reject an empty bearer token (`Authorization: Bearer ` with no value) up front with a `401 invalid_token` instead of forwarding an empty string to `verifyToken`. +- Errors thrown when a `@Handle` dependency fails to resolve now chain the original via `{ cause }`. +- Documented `McpResponse.text/json/error` (and when to prefer `error()` over throwing); neutralized framework-specific wording in the OAuth core docs. +- README: completed the OAuth 2.1 section (a real `jose`-based `verifyToken`, and that `oauth2McpMiddleware` + `registerOAuth2Metadata` must both be wired), softened the origin framing. diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 80a5cc9..aa0583b 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -2,7 +2,7 @@ An agent-agnostic framework for **authoring Model Context Protocol (MCP) servers** in TypeScript: declare tools, resources, and prompts as classes; serve them over a framework-neutral HTTP handler or stdio; protect them with OAuth 2.1. No framework required. -This is the graduation of the mature `@rudderjs/mcp` server framework into a standalone, dependency-light package. Its only runtime dependencies are `@modelcontextprotocol/sdk`, `zod`, and `reflect-metadata`. +It is standalone and dependency-light: its only runtime dependencies are `@modelcontextprotocol/sdk`, `zod`, and `reflect-metadata`. (It graduated from the mature `@rudderjs/mcp` server framework, re-versioned under the GemStack umbrella.) ## Which MCP package do I want? @@ -70,7 +70,7 @@ app.all('/mcp', (c) => handler(c.req.raw)) For a CLI/stdio server, use `startStdio` from the same subpath. -> **Runnable example:** [`examples/mcp-quickstart`](../../examples/mcp-quickstart) is a complete, framework-neutral server (tool + resource + prompt, `@Handle` DI, OAuth 2.1) served over both `node:http` and Hono, with a CI smoke test, and **zero `@rudderjs/*` packages**. +> **Runnable example:** [`examples/mcp-quickstart`](../../examples/mcp-quickstart) is a complete, framework-neutral server (tool + resource + prompt, `@Handle` DI, OAuth 2.1) served over both `node:http` and Hono, with a CI smoke test and **zero framework dependencies**. ### Resources and prompts @@ -131,21 +131,40 @@ If a `@Handle` method requests a dependency and no resolver is provided — or t ## OAuth 2.1 -Protect a web endpoint with bearer tokens. The core is auth-agnostic: you supply a `verifyToken` that validates the JWT (signature, expiry, revocation) and returns its claims, or `null`/throws when invalid. +Protect a web endpoint with bearer tokens. The core is auth-agnostic: you supply a `verifyToken` that validates the JWT (signature, expiry, revocation) and returns its claims, or `null`/throws when invalid. Back it with any JWT library (`jose` shown here), a token-introspection endpoint, or a framework's auth integration. + +Two pieces work together, and you need **both**: + +1. `oauth2McpMiddleware('/mcp', ...)` guards the MCP endpoint and, on failure, returns an RFC 9728 `WWW-Authenticate` challenge. +2. `registerOAuth2Metadata(router, '/mcp', ...)` serves the protected-resource metadata document at `/.well-known/oauth-protected-resource/mcp` that the challenge points clients to. Without it, compliant clients can't discover the authorization server. ```ts -import { oauth2McpMiddleware } from '@gemstack/mcp' +import { oauth2McpMiddleware, registerOAuth2Metadata } from '@gemstack/mcp' +import { createRemoteJWKSet, jwtVerify } from 'jose' + +const JWKS = createRemoteJWKSet(new URL('https://issuer.example.com/.well-known/jwks.json')) -const mw = oauth2McpMiddleware('/mcp', { +const options = { scopes: ['mcp.read'], - verifyToken: async (jwt) => { - // validate however you like; return claims or null - return { sub: 'user-1', scopes: ['mcp.read'] } + scopesSupported: ['mcp.read', 'mcp.write'], + authorizationServers: ['https://issuer.example.com'], + verifyToken: async (jwt: string) => { + try { + const { payload } = await jwtVerify(jwt, JWKS, { audience: 'https://api.example.com/mcp' }) + // map your token's claims onto { sub?, scopes? } + return { sub: payload.sub, scopes: String(payload['scope'] ?? '').split(' ').filter(Boolean) } + } catch { + return null // invalid/expired -> 401 + } }, -}) +} + +// Express/Connect-style wiring: +app.use('/mcp', oauth2McpMiddleware('/mcp', options)) +registerOAuth2Metadata(app, '/mcp', options) ``` -On success the verified claims are attached to the request as `req.mcpAuth`. `registerOAuth2Metadata(...)` emits the RFC 9728 protected-resource metadata document. +On success the verified claims are attached to the request as `req.mcpAuth` (`{ sub?, scopes?, claims }`). Missing required `scopes` yields a `403 insufficient_scope`; a missing/invalid token yields `401 invalid_token`. ## Testing diff --git a/packages/mcp/src/McpResponse.ts b/packages/mcp/src/McpResponse.ts index 1c06c72..6cabd45 100644 --- a/packages/mcp/src/McpResponse.ts +++ b/packages/mcp/src/McpResponse.ts @@ -1,14 +1,26 @@ import type { McpToolResult } from './McpTool.js' +/** + * Helpers for building a tool's {@link McpToolResult}. Return one of these from + * a tool's `handle()`. + */ export class McpResponse { + /** A plain-text result. */ static text(content: string): McpToolResult { return { content: [{ type: 'text', text: content }] } } + /** A structured result, serialized as pretty-printed JSON text. */ static json(data: unknown): McpToolResult { return { content: [{ type: 'text', text: JSON.stringify(data, null, 2) }] } } + /** + * An error result (`isError: true`), prefixed with `Error: `. The client sees + * a failed tool call rather than a thrown exception, so prefer this for + * expected, user-facing failures (validation, not-found) and reserve throwing + * for unexpected faults. + */ static error(message: string): McpToolResult { return { content: [{ type: 'text', text: `Error: ${message}` }], isError: true } } diff --git a/packages/mcp/src/auth/oauth2.ts b/packages/mcp/src/auth/oauth2.ts index b9d2d42..57631f1 100644 --- a/packages/mcp/src/auth/oauth2.ts +++ b/packages/mcp/src/auth/oauth2.ts @@ -1,12 +1,12 @@ /** * OAuth 2.1 bearer-token protection for an MCP web endpoint, framework-agnostic. * - * The core does NOT know how to verify a token — that is the binding's / app's - * job. Supply a {@link VerifyToken} via {@link OAuth2McpOptions.verifyToken}: it - * validates the JWT (signature, expiry, revocation — whatever your authorization - * server requires) and returns the token's claims, or `null` / throws when the - * token is invalid. The Rudder binding wires `@rudderjs/passport` here; a - * non-Rudder app supplies its own verifier. + * The core does NOT know how to verify a token — that is the app's job. Supply + * a {@link VerifyToken} via {@link OAuth2McpOptions.verifyToken}: it validates + * the JWT (signature, expiry, revocation — whatever your authorization server + * requires) and returns the token's claims, or `null` / throws when the token + * is invalid. Back it with any JWT library (e.g. `jose`), a hosted introspection + * endpoint, or a framework's auth integration. * * On failure the middleware adds an RFC 9728 `WWW-Authenticate` header pointing * clients at the protected-resource metadata document (see @@ -93,6 +93,11 @@ export function oauth2McpMiddleware(mcpPath: string, options: OAuth2McpOptions = } const jwt = authHeader.slice(7).trim() + if (!jwt) { + challenge(res, metadataUrl, 'invalid_token', 'Bearer token required.') + return + } + let claims: VerifiedToken | null try { claims = await verifyToken(jwt) diff --git a/packages/mcp/src/index.test.ts b/packages/mcp/src/index.test.ts index fa2cb05..4bca42a 100644 --- a/packages/mcp/src/index.test.ts +++ b/packages/mcp/src/index.test.ts @@ -1251,7 +1251,7 @@ describe('oauth2McpMiddleware — happy paths', () => { } } - // A stand-in for the binding's verifier (the Rudder binding wires passport here). + // A stand-in for an app-supplied verifier (real ones validate the JWT). function verifier(opts: { scopes?: string[]; sub?: string } = {}): VerifyToken { return async () => ({ sub: opts.sub ?? 'user-1', @@ -1327,6 +1327,21 @@ describe('oauth2McpMiddleware — happy paths', () => { assert.equal(calls.status, 401) assert.ok(calls.headers['www-authenticate']?.includes('invalid_token')) }) + + it('an empty bearer token ("Bearer ") is rejected without calling the verifier', async () => { + const { oauth2McpMiddleware } = await import('./auth/oauth2.js') + let verifierCalled = false + const mw = oauth2McpMiddleware('/mcp/secure', { + verifyToken: async () => { verifierCalled = true; return { sub: 'x' } }, + }) + const { res, calls } = mockRes() + let nextCalled = false + await mw(mockReq('Bearer ') as never, res as never, async () => { nextCalled = true }) + assert.equal(nextCalled, false) + assert.equal(verifierCalled, false) + assert.equal(calls.status, 401) + assert.ok(calls.headers['www-authenticate']?.includes('invalid_token')) + }) }) describe('registerOAuth2Metadata', () => { diff --git a/packages/mcp/src/runtime/handle-deps.ts b/packages/mcp/src/runtime/handle-deps.ts index cdc108d..31ae045 100644 --- a/packages/mcp/src/runtime/handle-deps.ts +++ b/packages/mcp/src/runtime/handle-deps.ts @@ -63,6 +63,7 @@ export function resolveHandleDeps( const msg = err instanceof Error ? err.message : String(err) throw new Error( `[gemstack/mcp] failed to resolve dependency ${describeToken(token)} for ${member}: ${msg}`, + { cause: err }, ) } if (resolved === undefined) {