-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(auth): implement generalized authentication layer and scope-based authorization #1729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9800cc2
1a833db
3ab1e66
2f985be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- | ||
| '@modelcontextprotocol/core': minor | ||
| '@modelcontextprotocol/server': minor | ||
| --- | ||
|
|
||
| Implement generalized authentication and authorization layer for MCP servers. | ||
|
|
||
| - Added `Authenticator` and `BearerTokenAuthenticator` to `@modelcontextprotocol/server`. | ||
| - Integrated scope-based authorization checks into `McpServer` for tools, resources, and prompts. | ||
| - Fixed asynchronous error propagation in the core `Protocol` class to support proper 401/403 HTTP status mapping in transports. | ||
| - Updated `WebStandardStreamableHTTPServerTransport` to correctly map authentication and authorization failures to their respective HTTP status codes. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # Authentication and Authorization | ||
|
|
||
| The MCP TypeScript SDK provides optional, opt-in support for authentication (AuthN) and authorization (AuthZ). This enables you to protect your MCP server resources, tools, and prompts using industry-standard schemes like OAuth 2.1 Bearer tokens. | ||
|
|
||
| ## Key Concepts | ||
|
|
||
| - **Authenticator**: Responsible for extracting and validating authentication information from an incoming request. | ||
| - **AuthInfo**: A structure containing information about the authenticated entity (e.g., user name, active scopes). | ||
| - **Authorizer**: Used by the MCP server to verify if the authenticated entity has the required scopes to access a specific resource, tool, or prompt. | ||
| - **Scopes**: Optional strings associated with registered items that define the required permissions. | ||
|
|
||
| ## Implementing Authentication | ||
|
|
||
| To enable authentication, provide an `authenticator` in the `ServerOptions` when creating your server. | ||
|
|
||
| ### Using Bearer Token Authentication | ||
|
|
||
| The SDK includes a `BearerTokenAuthenticator` for validating OAuth 2.1 Bearer tokens. | ||
|
|
||
| ```typescript | ||
| import { McpServer, BearerTokenAuthenticator } from "@modelcontextprotocol/server"; | ||
|
|
||
| const server = new McpServer({ | ||
| name: "my-authenticated-server", | ||
| version: "1.0.0", | ||
| }, { | ||
| authenticator: new BearerTokenAuthenticator(async (token) => { | ||
| // Validate the token (e.g., verify with an OAuth provider) | ||
| if (token === "valid-token") { | ||
| return { | ||
| token, | ||
| clientId: "john_doe", | ||
| scopes: ["read:resources", "execute:tools"] | ||
| }; | ||
| } | ||
| return undefined; // Invalid token | ||
| }) | ||
| }); | ||
| ``` | ||
|
|
||
| ## Implementing Authorization | ||
|
|
||
| Authorization is enforced using the `scopes` property when registering tools, resources, or prompts. | ||
|
|
||
| ### Scoped Tools | ||
|
|
||
| ```typescript | ||
| server.tool( | ||
| "secure_tool", | ||
| { | ||
| description: "A tool that requires specific scopes", | ||
| scopes: ["execute:tools"] | ||
| }, | ||
| async (args) => { | ||
| return { content: [{ type: "text", text: "Success!" }] }; | ||
| } | ||
| ); | ||
| ``` | ||
|
|
||
| ### Scoped Resources | ||
|
|
||
| ```typescript | ||
| server.resource( | ||
| "secure_resource", | ||
| "secure://data", | ||
| { scopes: ["read:resources"] }, | ||
| async (uri) => { | ||
| return { contents: [{ uri: uri.href, text: "Top secret data" }] }; | ||
| } | ||
| ); | ||
| ``` | ||
|
|
||
| ## Middleware Support | ||
|
|
||
| For framework-specific integrations, use the provided middleware to pre-authenticate requests. | ||
|
|
||
| ### Express Middleware | ||
|
|
||
| ```typescript | ||
| import express from "express"; | ||
| import { auth } from "@modelcontextprotocol/express"; | ||
|
|
||
| const app = express(); | ||
| app.use(auth({ authenticator })); | ||
|
|
||
| app.post("/mcp", (req, res) => { | ||
| // req.auth is now populated | ||
| transport.handleRequest(req, res); | ||
| }); | ||
| ``` | ||
|
|
||
| ### Hono Middleware | ||
|
|
||
| ```typescript | ||
| import { Hono } from "hono"; | ||
| import { auth } from "@modelcontextprotocol/hono"; | ||
|
|
||
| const app = new Hono(); | ||
| app.use("/mcp/*", auth({ authenticator })); | ||
|
|
||
| app.all("/mcp", async (c) => { | ||
| const authInfo = c.get("mcpAuthInfo"); | ||
| return transport.handleRequest(c.req.raw, { authInfo }); | ||
| }); | ||
| ``` | ||
|
|
||
| ## Error Handling | ||
|
|
||
| - **401 Unauthorized**: Returned when authentication is required but missing or invalid. Includes `WWW-Authenticate: Bearer` header. | ||
| - **403 Forbidden**: Returned when the authenticated entity lacks the required scopes. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -704,16 +704,24 @@ export abstract class Protocol<ContextT extends BaseContext> { | |
| }; | ||
|
|
||
| const _onmessage = this._transport?.onmessage; | ||
| this._transport.onmessage = (message, extra) => { | ||
| _onmessage?.(message, extra); | ||
| if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) { | ||
| this._onresponse(message); | ||
| } else if (isJSONRPCRequest(message)) { | ||
| this._onrequest(message, extra); | ||
| } else if (isJSONRPCNotification(message)) { | ||
| this._onnotification(message); | ||
| } else { | ||
| this._onerror(new Error(`Unknown message type: ${JSON.stringify(message)}`)); | ||
| this._transport.onmessage = async (message, extra) => { | ||
| try { | ||
| if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) { | ||
| await _onmessage?.(message, extra); | ||
| this._onresponse(message); | ||
| } else if (isJSONRPCRequest(message)) { | ||
| await this._onrequest(message, extra); | ||
| } else if (isJSONRPCNotification(message)) { | ||
| await this._onnotification(message); | ||
| } else { | ||
| await _onmessage?.(message, extra); | ||
| this._onerror(new Error(`Unknown message type: ${JSON.stringify(message)}`)); | ||
| } | ||
| } catch (error) { | ||
|
Comment on lines
+707
to
+720
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The PR moves the Extended reasoning...What the bug isIn the original code, this._transport.onmessage = (message, extra) => {
_onmessage?.(message, extra); // Called for ALL messages
if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) {
this._onresponse(message);
} else if (isJSONRPCRequest(message)) {
...The PR restructures this so Why this is an inconsistencyThe Step-by-step proof
Existing test coverageThere is a test at ImpactThe practical impact is low: no production code in the repository sets How to fixRestore this._transport.onmessage = async (message, extra) => {
try {
await _onmessage?.(message, extra); // Restore: called for ALL messages
if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) {
this._onresponse(message);
} else if (isJSONRPCRequest(message)) {
await this._onrequest(message, extra);
} ... |
||
| if (error instanceof ProtocolError && (error.code === ProtocolErrorCode.Unauthorized || error.code === ProtocolErrorCode.Forbidden)) { | ||
| throw error; | ||
| } | ||
| this._onerror(error instanceof Error ? error : new Error(String(error))); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -758,7 +766,7 @@ export abstract class Protocol<ContextT extends BaseContext> { | |
| .catch(error => this._onerror(new Error(`Uncaught error in notification handler: ${error}`))); | ||
| } | ||
|
|
||
| private _onrequest(request: JSONRPCRequest, extra?: MessageExtraInfo): void { | ||
| protected async _onrequest(request: JSONRPCRequest, extra?: MessageExtraInfo): Promise<void> { | ||
| const handler = this._requestHandlers.get(request.method) ?? this.fallbackRequestHandler; | ||
|
|
||
| // Capture the current transport at request time to ensure responses go to the correct client | ||
|
|
@@ -838,7 +846,7 @@ export abstract class Protocol<ContextT extends BaseContext> { | |
| const ctx = this.buildContext(baseCtx, extra); | ||
|
|
||
| // Starting with Promise.resolve() puts any synchronous errors into the monad as well. | ||
| Promise.resolve() | ||
| return Promise.resolve() | ||
| .then(() => { | ||
| // If this request asked for task creation, check capability first | ||
| if (taskCreationParams) { | ||
|
|
@@ -879,6 +887,10 @@ export abstract class Protocol<ContextT extends BaseContext> { | |
| return; | ||
| } | ||
|
|
||
| if (error instanceof ProtocolError && (error.message.includes('Unauthorized') || error.message.includes('Forbidden'))) { | ||
| throw error; | ||
| } | ||
|
|
||
| const errorResponse: JSONRPCErrorResponse = { | ||
| jsonrpc: '2.0', | ||
| id: request.id, | ||
|
|
@@ -903,7 +915,13 @@ export abstract class Protocol<ContextT extends BaseContext> { | |
| : capturedTransport?.send(errorResponse)); | ||
| } | ||
| ) | ||
| .catch(error => this._onerror(new Error(`Failed to send response: ${error}`))) | ||
| .catch(error => { | ||
| if (error instanceof ProtocolError && (error.message.includes('Unauthorized') || error.message.includes('Forbidden'))) { | ||
| throw error; | ||
| } | ||
| // Do not report as protocol error if it's already an auth error we're escaping | ||
| this._onerror(new Error(`Failed to send response: ${error}`)); | ||
| }) | ||
| .finally(() => { | ||
| this._requestHandlerAbortControllers.delete(request.id); | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| export * from './express.js'; | ||
| export { auth } from './middleware/auth.js'; | ||
| export * from './middleware/hostHeaderValidation.js'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import { Request, Response, NextFunction, RequestHandler } from 'express'; | ||
| import { Authenticator, AuthInfo } from '@modelcontextprotocol/server'; | ||
|
|
||
| /** | ||
| * Options for the MCP Express authentication middleware. | ||
| */ | ||
| export interface AuthMiddlewareOptions { | ||
| /** | ||
| * The authenticator to use for validating requests. | ||
| */ | ||
| authenticator: Authenticator; | ||
| } | ||
|
|
||
| /** | ||
| * Creates an Express middleware for MCP authentication. | ||
| * | ||
| * This middleware extracts authentication information from the request using the provided authenticator | ||
| * and attaches it to the request object as `req.auth`. The MCP Express transport will then | ||
| * pick up this information automatically. | ||
| * | ||
| * @param options - Middleware options | ||
| * @returns An Express middleware function | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * const authenticator = new BearerTokenAuthenticator((token) => Promise.resolve({ token, clientId: 'user', scopes: ['read'] })); | ||
| * app.use(auth({ authenticator })); | ||
| * ``` | ||
| */ | ||
| export function auth(options: AuthMiddlewareOptions): RequestHandler { | ||
| return async (req: Request & { auth?: AuthInfo }, res: Response, next: NextFunction) => { | ||
| try { | ||
| const headers: Record<string, string> = {}; | ||
| for (const [key, value] of Object.entries(req.headers)) { | ||
| if (typeof value === 'string') { | ||
| headers[key] = value; | ||
| } else if (Array.isArray(value)) { | ||
| headers[key] = value.join(', '); | ||
| } | ||
| } | ||
|
|
||
| const authInfo = await options.authenticator.authenticate({ | ||
| method: req.method, | ||
| headers, | ||
| }); | ||
| if (authInfo) { | ||
| req.auth = authInfo; | ||
| } | ||
| next(); | ||
| } catch (error) { | ||
| // If authentication fails, we let the MCP server handle it later, | ||
| // or the developer can choose to reject here. | ||
| // By default, we just proceed to allow the MCP server to decide (e.g., if auth is optional). | ||
claude[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| console.error('[MCP Express Auth Middleware] Authentication failed:', error); | ||
| next(); | ||
| } | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| export * from './hono.js'; | ||
| export { auth } from './middleware/auth.js'; | ||
| export * from './middleware/hostHeaderValidation.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Auth errors are detected by string-matching
error.message.includes("Unauthorized")/error.message.includes("Forbidden")in 5+ locations (protocol.ts, mcp.ts, streamableHttp.ts) instead of using dedicated error codes. AnyProtocolErrorwhose message coincidentally contains these words (e.g.,"Forbidden characters in input") would be incorrectly escalated to HTTP 401/403. TheProtocolErrorCodeenum already supports custom codes (e.g.,UrlElicitationRequired = -32_042) -- consider addingUnauthorized/Forbiddencodes instead of string matching.Extended reasoning...
What the bug is
This PR introduces authentication and authorization error handling by throwing
ProtocolErrorinstances withProtocolErrorCode.InvalidRequestand messages like"Unauthorized"or"Forbidden". These errors are then identified downstream viaerror.message.includes("Unauthorized")orerror.message.includes("Forbidden")in at least 5 locations:protocol.tsonmessagecatch block (~line 718)protocol.ts_onrequesterror handler (~line 887)protocol.ts.catchhandler (~line 915)mcp.tstools/callcatch block (~line 228-233)streamableHttp.tsin multiple catch blocks that map to HTTP 401/403Why this is fragile
The string matching operates on the error message, not on a structured error code. This means any
ProtocolErrorwhose message happens to contain the substring"Unauthorized"or"Forbidden"will be incorrectly treated as an auth error. For example, consider a tool handler that throws:Step-by-step proof of false positive
McpServer.registerTool()that validates input and throwsnew ProtocolError(ProtocolErrorCode.InvalidParams, "Forbidden characters in input").tools/callhandler inmcp.tscatches this error. In the PR’s version, the catch block checkserror.message.includes("Forbidden")— this matches, so the error is re-thrown instead of being wrapped in aCallToolResulterror.protocol.ts_onrequest, where the error handler again checkserror.message.includes("Forbidden")and re-throws it instead of sending a normal JSON-RPC error response.streamableHttp.ts, the outer catch block maps anyProtocolErrorcontaining"Forbidden"to an HTTP 403 response.Why existing code doesn’t prevent this
The
ProtocolErrorCodeused for auth errors isInvalidRequest— the same generic code used for many other error conditions. The only distinguishing factor is the message string. There is no structural difference between an auth error and any otherProtocolErrorthat happens to use a message containing these words.Impact
This is a public SDK where users write custom tool/resource/prompt handlers that can throw
ProtocolErrorwith arbitrary messages. A false positive would cause:Additionally, the
mcp.tscatch block at lines ~228-233 is dead code: both branches of theifstatement do the same thing (throw error), suggesting this pattern was applied mechanically without careful consideration.How to fix
Add dedicated error codes to
ProtocolErrorCode, e.g.:Then check
error.codeinstead oferror.messagein all catch blocks. This follows the existing pattern used byUrlElicitationRequiredandResourceNotFound.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dedicated error codes were added (
Unauthorized = 401,Forbidden = 403inProtocolErrorCode) and some locations were updated to use code-based checking (mcp.tscatch block andstreamableHttp.tsSSE/outer catch blocks now checkerror.code). However,protocol.tsitself — the file where this comment was posted — still useserror.message.includes("Unauthorized")/error.message.includes("Forbidden")in all 3 catch blocks (lines ~720, ~890, ~918). The JSON response mode instreamableHttp.ts(lines ~735-740) also still uses string matching.These remaining string-match locations are still vulnerable to the false-positive scenario described in the original comment. They should be updated to check
error.code === ProtocolErrorCode.Unauthorized/error.code === ProtocolErrorCode.Forbiddento match the pattern already applied inmcp.tsand the SSE mode ofstreamableHttp.ts.