From 575bc010a1bf2c4eb715d4738a33b0f1fda5cb00 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 15 Jun 2026 08:12:50 -0700 Subject: [PATCH 1/2] Wire RFC 9728 resource_metadata discovery into Bearer 401 + observability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bearer middleware 401s on /mcp now advertise the protected-resource metadata document on WWW-Authenticate via `resource_metadata=`, wiring the previously-dead `unauthorizedWithDiscovery` helper at src/oauth/handlers.ts:626 into all three failure sites: empty token, the four typed JWT errors (invalid signature, expired, aud mismatch, malformed), and the unknown-error fallthrough. The discovery endpoint already exists on this server, so no new endpoint work — this is pure substitution at the call sites. Adds three `console.warn` lines (`[oauth] bearer_failure reason=…`) matching the existing log shape (`[oauth] ip=`), with the catch-block reason discriminated by `instanceof` so operators can tell signature drift from clock skew from aud-mismatch (RFC 8707) from a malformed token. The `unauthorized()` helper is preserved — `error="invalid_token"` moves from WWW-Authenticate to the JSON body, mirroring what `unauthorizedWithDiscovery` was always emitting. Updates the two pre-existing tests that asserted `error="invalid_token"` on the header. Spec: https://datatracker.ietf.org/doc/html/rfc9728 --- src/__tests__/oauth-e2e.test.ts | 7 +- src/__tests__/oauth-handlers.test.ts | 122 ++++++++++++++++++++++++++- src/oauth/handlers.ts | 27 +++++- 3 files changed, 151 insertions(+), 5 deletions(-) diff --git a/src/__tests__/oauth-e2e.test.ts b/src/__tests__/oauth-e2e.test.ts index 3331147..63103eb 100644 --- a/src/__tests__/oauth-e2e.test.ts +++ b/src/__tests__/oauth-e2e.test.ts @@ -383,7 +383,12 @@ describe("OAuth 2.1 end-to-end ceremonial flow", () => { expect(res.status).toBe(401); const www = res.headers.get("www-authenticate"); expect(www).toContain('Bearer realm="mcp"'); - expect(www).toContain('error="invalid_token"'); + // RFC 9728: discovery URL advertised on WWW-Authenticate so the client + // can fetch the protected-resource metadata document. The + // `error="invalid_token"` code now lives in the JSON response body. + expect(www).toContain("resource_metadata="); + const body = (await res.json()) as { error?: string }; + expect(body.error).toBe("invalid_token"); }); // ────────────────────────────────────────────────────────────────── diff --git a/src/__tests__/oauth-handlers.test.ts b/src/__tests__/oauth-handlers.test.ts index 9822566..ee9efff 100644 --- a/src/__tests__/oauth-handlers.test.ts +++ b/src/__tests__/oauth-handlers.test.ts @@ -1511,9 +1511,13 @@ describe("bearerMiddleware", () => { "WWW-Authenticate", expect.stringContaining('Bearer realm="mcp"'), ); + // RFC 9728: `WWW-Authenticate` advertises `resource_metadata=` so + // clients can discover the protected-resource document. The + // `error="invalid_token"` code now lives in the JSON body (see + // `unauthorizedWithDiscovery`). expect(res.setHeader).toHaveBeenCalledWith( "WWW-Authenticate", - expect.stringContaining('error="invalid_token"'), + expect.stringContaining("resource_metadata="), ); expect(next).not.toHaveBeenCalled(); }); @@ -1577,6 +1581,122 @@ describe("bearerMiddleware", () => { expect(res.status).toHaveBeenCalledWith(401); expect(next).not.toHaveBeenCalled(); }); + + // ────────────────────────────────────────────────────────────────────── + // RFC 9728 — every Bearer 401 must include `resource_metadata=` on + // `WWW-Authenticate` so clients can discover the protected-resource + // metadata document. The helper at handlers.ts:626 constructs the URL + // from `originOf(req)` + "/.well-known/oauth-protected-resource"; tests + // mirror that construction to detect drift. + // ────────────────────────────────────────────────────────────────────── + describe("RFC 9728 resource_metadata discovery on 401", () => { + const expectedResourceMetadata = + "https://mcp.example.com/.well-known/oauth-protected-resource"; + + function assertDiscoveryHeader( + res: ReturnType, + next: ReturnType, + ): void { + expect(res.status).toHaveBeenCalledWith(401); + expect(next).not.toHaveBeenCalled(); + expect(res.setHeader).toHaveBeenCalledWith( + "WWW-Authenticate", + expect.stringContaining("Bearer"), + ); + expect(res.setHeader).toHaveBeenCalledWith( + "WWW-Authenticate", + expect.stringContaining( + `resource_metadata="${expectedResourceMetadata}"`, + ), + ); + } + + it("empty Bearer token → 401 + resource_metadata", () => { + const req = mockReq({ + headers: { + host: "mcp.example.com", + "x-forwarded-proto": "https", + authorization: "Bearer ", + }, + }); + const res = mockRes(); + const next = vi.fn(); + bearerMiddleware(req as never, res as never, next); + assertDiscoveryHeader(res, next); + }); + + it("invalid signature → 401 + resource_metadata", () => { + const now = Math.floor(Date.now() / 1000); + const token = signJWT( + { + sub: "x", + aud: "https://mcp.example.com", + iat: now, + exp: now + 3600, + }, + "wrong-secret-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + ); + const req = mockReq({ + headers: { + host: "mcp.example.com", + "x-forwarded-proto": "https", + authorization: `Bearer ${token}`, + }, + }); + const res = mockRes(); + const next = vi.fn(); + bearerMiddleware(req as never, res as never, next); + assertDiscoveryHeader(res, next); + }); + + it("expired token → 401 + resource_metadata", () => { + const now = Math.floor(Date.now() / 1000); + const token = signJWT( + { + sub: "x", + aud: "https://mcp.example.com", + iat: now - 7200, + exp: now - 3600, + }, + "a".repeat(64), + ); + const req = mockReq({ + headers: { + host: "mcp.example.com", + "x-forwarded-proto": "https", + authorization: `Bearer ${token}`, + }, + }); + const res = mockRes(); + const next = vi.fn(); + bearerMiddleware(req as never, res as never, next); + assertDiscoveryHeader(res, next); + }); + + it("aud mismatch → 401 + resource_metadata", () => { + const now = Math.floor(Date.now() / 1000); + const token = signJWT( + { + sub: "x", + aud: "https://other.example", + iat: now, + exp: now + 3600, + }, + "a".repeat(64), + ); + const req = mockReq({ + headers: { + host: "mcp.example.com", + "x-forwarded-proto": "https", + authorization: `Bearer ${token}`, + }, + }); + const res = mockRes(); + const next = vi.fn(); + bearerMiddleware(req as never, res as never, next); + assertDiscoveryHeader(res, next); + }); + }); }); // Keep module-level state from leaking across files diff --git a/src/oauth/handlers.ts b/src/oauth/handlers.ts index 888c89a..ae6af91 100644 --- a/src/oauth/handlers.ts +++ b/src/oauth/handlers.ts @@ -585,7 +585,10 @@ export function bearerMiddleware( } const token = trimmed.slice("Bearer".length).trim(); if (!token) { - unauthorized(res, "invalid_token"); + console.warn( + `[oauth] bearer_failure reason=empty_token ip=${oauthClientIp(req)}`, + ); + unauthorizedWithDiscovery(req, res); return; } @@ -610,11 +613,29 @@ export function bearerMiddleware( err instanceof InvalidAudience || err instanceof MalformedToken ) { - unauthorized(res, "invalid_token"); + // The catch block fans in four distinct JWT failure modes; tag the + // observability log with the actual reason so operators can tell + // signature drift apart from clock skew apart from aud mismatch + // (RFC 8707 resource binding) apart from a malformed token. + const reason = + err instanceof InvalidSignature + ? "invalid_signature" + : err instanceof TokenExpired + ? "expired" + : err instanceof InvalidAudience + ? "aud_mismatch" + : "malformed_token"; + console.warn( + `[oauth] bearer_failure reason=${reason} ip=${oauthClientIp(req)}`, + ); + unauthorizedWithDiscovery(req, res); return; } // Unknown error — fail closed - unauthorized(res, "invalid_token"); + console.warn( + `[oauth] bearer_failure reason=unknown ip=${oauthClientIp(req)}`, + ); + unauthorizedWithDiscovery(req, res); } } From 852e6496942f51e0a2246012ce07d0e125fc0f30 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 15 Jun 2026 08:26:16 -0700 Subject: [PATCH 2/2] =?UTF-8?q?Keep=20error=3Dinvalid=5Ftoken=20in=20WWW-A?= =?UTF-8?q?uthenticate=20per=20RFC=206750=20=C2=A73.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/__tests__/oauth-e2e.test.ts | 6 ++++-- src/__tests__/oauth-handlers.test.ts | 17 ++++++++++++++--- src/oauth/handlers.ts | 2 +- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/__tests__/oauth-e2e.test.ts b/src/__tests__/oauth-e2e.test.ts index 63103eb..a41623c 100644 --- a/src/__tests__/oauth-e2e.test.ts +++ b/src/__tests__/oauth-e2e.test.ts @@ -384,9 +384,11 @@ describe("OAuth 2.1 end-to-end ceremonial flow", () => { const www = res.headers.get("www-authenticate"); expect(www).toContain('Bearer realm="mcp"'); // RFC 9728: discovery URL advertised on WWW-Authenticate so the client - // can fetch the protected-resource metadata document. The - // `error="invalid_token"` code now lives in the JSON response body. + // can fetch the protected-resource metadata document. Per RFC 6750 §3.1, + // the `error="invalid_token"` attribute MUST also be in the header (it + // is additionally returned in the JSON response body). expect(www).toContain("resource_metadata="); + expect(www).toContain('error="invalid_token"'); const body = (await res.json()) as { error?: string }; expect(body.error).toBe("invalid_token"); }); diff --git a/src/__tests__/oauth-handlers.test.ts b/src/__tests__/oauth-handlers.test.ts index ee9efff..20942f9 100644 --- a/src/__tests__/oauth-handlers.test.ts +++ b/src/__tests__/oauth-handlers.test.ts @@ -1512,13 +1512,18 @@ describe("bearerMiddleware", () => { expect.stringContaining('Bearer realm="mcp"'), ); // RFC 9728: `WWW-Authenticate` advertises `resource_metadata=` so - // clients can discover the protected-resource document. The - // `error="invalid_token"` code now lives in the JSON body (see - // `unauthorizedWithDiscovery`). + // clients can discover the protected-resource document. Per RFC 6750 + // §3.1, when a Bearer token is present-but-invalid the `error=` + // attribute MUST also be in the `WWW-Authenticate` header (it is + // additionally returned in the JSON body). expect(res.setHeader).toHaveBeenCalledWith( "WWW-Authenticate", expect.stringContaining("resource_metadata="), ); + expect(res.setHeader).toHaveBeenCalledWith( + "WWW-Authenticate", + expect.stringContaining('error="invalid_token"'), + ); expect(next).not.toHaveBeenCalled(); }); @@ -1609,6 +1614,12 @@ describe("bearerMiddleware", () => { `resource_metadata="${expectedResourceMetadata}"`, ), ); + // RFC 6750 §3.1 — when a Bearer token is present-but-invalid the + // `error=` attribute MUST appear in `WWW-Authenticate`. + expect(res.setHeader).toHaveBeenCalledWith( + "WWW-Authenticate", + expect.stringContaining('error="invalid_token"'), + ); } it("empty Bearer token → 401 + resource_metadata", () => { diff --git a/src/oauth/handlers.ts b/src/oauth/handlers.ts index ae6af91..76986f1 100644 --- a/src/oauth/handlers.ts +++ b/src/oauth/handlers.ts @@ -649,7 +649,7 @@ function unauthorizedWithDiscovery(req: Request, res: Response): void { const resourceMetadata = `${origin}/.well-known/oauth-protected-resource`; res.setHeader( "WWW-Authenticate", - `Bearer realm="mcp", resource_metadata="${resourceMetadata}", scope="${TOKEN_SCOPE}"`, + `Bearer realm="mcp", resource_metadata="${resourceMetadata}", scope="${TOKEN_SCOPE}", error="invalid_token"`, ); res.status(401).json({ error: "invalid_token" }); }