From 40f61d88a0e02f0e710cf19c053c562bf47de54f Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 17 Jun 2025 17:05:34 +0100 Subject: [PATCH] show how to enable strict resource checking in mcp server --- .../server/demoInMemoryOAuthProvider.ts | 3 +- src/examples/server/simpleStreamableHttp.ts | 18 ++++++++++- src/server/auth/middleware/bearerAuth.test.ts | 31 +++++++++++++------ src/server/auth/middleware/bearerAuth.ts | 8 ++++- src/server/auth/provider.ts | 2 +- 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/examples/server/demoInMemoryOAuthProvider.ts b/src/examples/server/demoInMemoryOAuthProvider.ts index 3133e455..500f59b8 100644 --- a/src/examples/server/demoInMemoryOAuthProvider.ts +++ b/src/examples/server/demoInMemoryOAuthProvider.ts @@ -41,7 +41,8 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider { if (mcpServerUrl) { const expectedResource = resourceUrlFromServerUrl(mcpServerUrl); this.validateResource = (resource?: URL) => { - return !resource || resource.toString() !== expectedResource.toString(); + if (!resource) return false; + return resource.toString() === expectedResource.toString(); }; } } diff --git a/src/examples/server/simpleStreamableHttp.ts b/src/examples/server/simpleStreamableHttp.ts index da5e740a..a97fdf5a 100644 --- a/src/examples/server/simpleStreamableHttp.ts +++ b/src/examples/server/simpleStreamableHttp.ts @@ -12,6 +12,13 @@ import { OAuthMetadata } from 'src/shared/auth.js'; // Check for OAuth flag const useOAuth = process.argv.includes('--oauth'); +// Resource Indicator the OAuth tokens are checked against (RFC8707). +const expectedOAuthResource = (iArg => iArg < 0 ? undefined: process.argv[iArg + 1])(process.argv.indexOf('--oauth-resource')); +// Requires Resource Indicator check (implies protocol more recent than 2025-03-26) +const strictOAuthResourceCheck = process.argv.includes('--oauth-resource-strict'); +if (strictOAuthResourceCheck && !expectedOAuthResource) { + throw new Error(`Strict resource indicator checking requires passing the expected resource with --oauth-resource https://...`); +} // Create an MCP server with implementation details const getServer = () => { @@ -285,7 +292,7 @@ if (useOAuth) { const oauthMetadata: OAuthMetadata = setupAuthServer(authServerUrl, mcpServerUrl); const tokenVerifier = { - verifyAccessToken: async (token: string) => { + verifyAccessToken: async (token: string, protocolVersion: string) => { const endpoint = oauthMetadata.introspection_endpoint; if (!endpoint) { @@ -308,6 +315,15 @@ if (useOAuth) { } const data = await response.json(); + + if (expectedOAuthResource) { + if (strictOAuthResourceCheck && !data.resource) { + throw new Error('Resource Indicator (RFC8707) missing'); + } + if (data.resource && data.resource !== expectedOAuthResource) { + throw new Error(`Expected resource indicator ${expectedOAuthResource}, got: ${data.resource}`); + } + } // Convert the response to AuthInfo format return { diff --git a/src/server/auth/middleware/bearerAuth.test.ts b/src/server/auth/middleware/bearerAuth.test.ts index b8953e5c..cae054d5 100644 --- a/src/server/auth/middleware/bearerAuth.test.ts +++ b/src/server/auth/middleware/bearerAuth.test.ts @@ -3,6 +3,7 @@ import { requireBearerAuth } from "./bearerAuth.js"; import { AuthInfo } from "../types.js"; import { InsufficientScopeError, InvalidTokenError, OAuthError, ServerError } from "../errors.js"; import { OAuthTokenVerifier } from "../provider.js"; +import { LATEST_PROTOCOL_VERSION } from '../../../types.js'; // Mock verifier const mockVerifyAccessToken = jest.fn(); @@ -42,12 +43,13 @@ describe("requireBearerAuth middleware", () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockRequest.auth).toEqual(validAuthInfo); expect(nextFunction).toHaveBeenCalled(); expect(mockResponse.status).not.toHaveBeenCalled(); @@ -65,12 +67,13 @@ describe("requireBearerAuth middleware", () => { mockRequest.headers = { authorization: "Bearer expired-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("expired-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("expired-token", LATEST_PROTOCOL_VERSION); expect(mockResponse.status).toHaveBeenCalledWith(401); expect(mockResponse.set).toHaveBeenCalledWith( "WWW-Authenticate", @@ -93,12 +96,13 @@ describe("requireBearerAuth middleware", () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockRequest.auth).toEqual(nonExpiredAuthInfo); expect(nextFunction).toHaveBeenCalled(); expect(mockResponse.status).not.toHaveBeenCalled(); @@ -115,6 +119,7 @@ describe("requireBearerAuth middleware", () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; const middleware = requireBearerAuth({ @@ -124,7 +129,7 @@ describe("requireBearerAuth middleware", () => { await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockResponse.status).toHaveBeenCalledWith(403); expect(mockResponse.set).toHaveBeenCalledWith( "WWW-Authenticate", @@ -146,6 +151,7 @@ describe("requireBearerAuth middleware", () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; const middleware = requireBearerAuth({ @@ -155,7 +161,7 @@ describe("requireBearerAuth middleware", () => { await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockRequest.auth).toEqual(authInfo); expect(nextFunction).toHaveBeenCalled(); expect(mockResponse.status).not.toHaveBeenCalled(); @@ -204,6 +210,7 @@ describe("requireBearerAuth middleware", () => { it("should return 401 when token verification fails with InvalidTokenError", async () => { mockRequest.headers = { authorization: "Bearer invalid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; mockVerifyAccessToken.mockRejectedValue(new InvalidTokenError("Token expired")); @@ -211,7 +218,7 @@ describe("requireBearerAuth middleware", () => { const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("invalid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("invalid-token", LATEST_PROTOCOL_VERSION); expect(mockResponse.status).toHaveBeenCalledWith(401); expect(mockResponse.set).toHaveBeenCalledWith( "WWW-Authenticate", @@ -226,6 +233,7 @@ describe("requireBearerAuth middleware", () => { it("should return 403 when access token has insufficient scopes", async () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; mockVerifyAccessToken.mockRejectedValue(new InsufficientScopeError("Required scopes: read, write")); @@ -233,7 +241,7 @@ describe("requireBearerAuth middleware", () => { const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockResponse.status).toHaveBeenCalledWith(403); expect(mockResponse.set).toHaveBeenCalledWith( "WWW-Authenticate", @@ -248,6 +256,7 @@ describe("requireBearerAuth middleware", () => { it("should return 500 when a ServerError occurs", async () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; mockVerifyAccessToken.mockRejectedValue(new ServerError("Internal server issue")); @@ -255,7 +264,7 @@ describe("requireBearerAuth middleware", () => { const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockResponse.status).toHaveBeenCalledWith(500); expect(mockResponse.json).toHaveBeenCalledWith( expect.objectContaining({ error: "server_error", error_description: "Internal server issue" }) @@ -266,6 +275,7 @@ describe("requireBearerAuth middleware", () => { it("should return 400 for generic OAuthError", async () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; mockVerifyAccessToken.mockRejectedValue(new OAuthError("custom_error", "Some OAuth error")); @@ -273,7 +283,7 @@ describe("requireBearerAuth middleware", () => { const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockResponse.status).toHaveBeenCalledWith(400); expect(mockResponse.json).toHaveBeenCalledWith( expect.objectContaining({ error: "custom_error", error_description: "Some OAuth error" }) @@ -284,6 +294,7 @@ describe("requireBearerAuth middleware", () => { it("should return 500 when unexpected error occurs", async () => { mockRequest.headers = { authorization: "Bearer valid-token", + 'mcp-protocol-version': LATEST_PROTOCOL_VERSION, }; mockVerifyAccessToken.mockRejectedValue(new Error("Unexpected error")); @@ -291,7 +302,7 @@ describe("requireBearerAuth middleware", () => { const middleware = requireBearerAuth({ verifier: mockVerifier }); await middleware(mockRequest as Request, mockResponse as Response, nextFunction); - expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token", LATEST_PROTOCOL_VERSION); expect(mockResponse.status).toHaveBeenCalledWith(500); expect(mockResponse.json).toHaveBeenCalledWith( expect.objectContaining({ error: "server_error", error_description: "Internal Server Error" }) diff --git a/src/server/auth/middleware/bearerAuth.ts b/src/server/auth/middleware/bearerAuth.ts index fd96055a..4674089f 100644 --- a/src/server/auth/middleware/bearerAuth.ts +++ b/src/server/auth/middleware/bearerAuth.ts @@ -2,6 +2,7 @@ import { RequestHandler } from "express"; import { InsufficientScopeError, InvalidTokenError, OAuthError, ServerError } from "../errors.js"; import { OAuthTokenVerifier } from "../provider.js"; import { AuthInfo } from "../types.js"; +import { DEFAULT_NEGOTIATED_PROTOCOL_VERSION } from "../../../types.js"; export type BearerAuthMiddlewareOptions = { /** @@ -50,7 +51,12 @@ export function requireBearerAuth({ verifier, requiredScopes = [], resourceMetad throw new InvalidTokenError("Invalid Authorization header format, expected 'Bearer TOKEN'"); } - const authInfo = await verifier.verifyAccessToken(token); + let protocolVersion = req.headers["mcp-protocol-version"] ?? DEFAULT_NEGOTIATED_PROTOCOL_VERSION; + if (Array.isArray(protocolVersion)) { + protocolVersion = protocolVersion[protocolVersion.length - 1]; + } + + const authInfo = await verifier.verifyAccessToken(token, protocolVersion); // Check if token has the required scopes (if any) if (requiredScopes.length > 0) { diff --git a/src/server/auth/provider.ts b/src/server/auth/provider.ts index 18beb216..409e9dae 100644 --- a/src/server/auth/provider.ts +++ b/src/server/auth/provider.ts @@ -80,5 +80,5 @@ export interface OAuthTokenVerifier { /** * Verifies an access token and returns information about it. */ - verifyAccessToken(token: string): Promise; + verifyAccessToken(token: string, protocolVersion: string): Promise; }