diff --git a/src/__tests__/oauth-e2e.test.ts b/src/__tests__/oauth-e2e.test.ts index 3331147..a41623c 100644 --- a/src/__tests__/oauth-e2e.test.ts +++ b/src/__tests__/oauth-e2e.test.ts @@ -383,7 +383,14 @@ 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"'); + // RFC 9728: discovery URL advertised on WWW-Authenticate so the client + // 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 9822566..20942f9 100644 --- a/src/__tests__/oauth-handlers.test.ts +++ b/src/__tests__/oauth-handlers.test.ts @@ -1511,6 +1511,15 @@ describe("bearerMiddleware", () => { "WWW-Authenticate", expect.stringContaining('Bearer realm="mcp"'), ); + // RFC 9728: `WWW-Authenticate` advertises `resource_metadata=` so + // 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"'), @@ -1577,6 +1586,128 @@ 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}"`, + ), + ); + // 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", () => { + 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..76986f1 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); } } @@ -628,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" }); }