diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..8e084134 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,25 @@ +# MCP TypeScript SDK Guide + +## Build & Test Commands +``` +npm run build # Build ESM and CJS versions +npm run lint # Run ESLint +npm test # Run all tests +npx jest path/to/file.test.ts # Run specific test file +npx jest -t "test name" # Run tests matching pattern +``` + +## Code Style Guidelines +- **TypeScript**: Strict type checking, ES modules, explicit return types +- **Naming**: PascalCase for classes/types, camelCase for functions/variables +- **Files**: Lowercase with hyphens, test files with `.test.ts` suffix +- **Imports**: ES module style, include `.js` extension, group imports logically +- **Error Handling**: Use TypeScript's strict mode, explicit error checking in tests +- **Formatting**: 2-space indentation, semicolons required, single quotes preferred +- **Testing**: Co-locate tests with source files, use descriptive test names +- **Comments**: JSDoc for public APIs, inline comments for complex logic + +## Project Structure +- `/src`: Source code with client, server, and shared modules +- Tests alongside source files with `.test.ts` suffix +- Node.js >= 18 required \ No newline at end of file diff --git a/package.json b/package.json index 8e3771f2..21ed2505 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.5.0", + "version": "1.6.0", "description": "Model Context Protocol implementation for TypeScript", "license": "MIT", "author": "Anthropic, PBC (https://anthropic.com)", diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 97c9e14a..02026c4f 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -43,6 +43,64 @@ describe("OAuth Authorization", () => { }); }); + it("returns metadata when first fetch fails but second without MCP header succeeds", async () => { + // Set up a counter to control behavior + let callCount = 0; + + // Mock implementation that changes behavior based on call count + mockFetch.mockImplementation((_url, _options) => { + callCount++; + + if (callCount === 1) { + // First call with MCP header - fail with TypeError (simulating CORS error) + // We need to use TypeError specifically because that's what the implementation checks for + return Promise.reject(new TypeError("Network error")); + } else { + // Second call without header - succeed + return Promise.resolve({ + ok: true, + status: 200, + json: async () => validMetadata + }); + } + }); + + // Should succeed with the second call + const metadata = await discoverOAuthMetadata("https://auth.example.com"); + expect(metadata).toEqual(validMetadata); + + // Verify both calls were made + expect(mockFetch).toHaveBeenCalledTimes(2); + + // Verify first call had MCP header + expect(mockFetch.mock.calls[0][1]?.headers).toHaveProperty("MCP-Protocol-Version"); + }); + + it("throws an error when all fetch attempts fail", async () => { + // Set up a counter to control behavior + let callCount = 0; + + // Mock implementation that changes behavior based on call count + mockFetch.mockImplementation((_url, _options) => { + callCount++; + + if (callCount === 1) { + // First call - fail with TypeError + return Promise.reject(new TypeError("First failure")); + } else { + // Second call - fail with different error + return Promise.reject(new Error("Second failure")); + } + }); + + // Should fail with the second error + await expect(discoverOAuthMetadata("https://auth.example.com")) + .rejects.toThrow("Second failure"); + + // Verify both calls were made + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + it("returns undefined when discovery endpoint returns 404", async () => { mockFetch.mockResolvedValueOnce({ ok: false, diff --git a/src/client/auth.ts b/src/client/auth.ts index c0bac9e8..c7799429 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -163,11 +163,21 @@ export async function discoverOAuthMetadata( opts?: { protocolVersion?: string }, ): Promise { const url = new URL("/.well-known/oauth-authorization-server", serverUrl); - const response = await fetch(url, { - headers: { - "MCP-Protocol-Version": opts?.protocolVersion ?? LATEST_PROTOCOL_VERSION + let response: Response; + try { + response = await fetch(url, { + headers: { + "MCP-Protocol-Version": opts?.protocolVersion ?? LATEST_PROTOCOL_VERSION + } + }); + } catch (error) { + // CORS errors come back as TypeError + if (error instanceof TypeError) { + response = await fetch(url); + } else { + throw error; } - }); + } if (response.status === 404) { return undefined; diff --git a/src/server/auth/handlers/register.test.ts b/src/server/auth/handlers/register.test.ts index 5faf1a4a..a961f654 100644 --- a/src/server/auth/handlers/register.test.ts +++ b/src/server/auth/handlers/register.test.ts @@ -141,6 +141,37 @@ describe('Client Registration Handler', () => { expect(response.status).toBe(201); expect(response.body.client_secret).toBeUndefined(); + expect(response.body.client_secret_expires_at).toBeUndefined(); + }); + + it('sets client_secret_expires_at for public clients only', async () => { + // Test for public client (token_endpoint_auth_method not 'none') + const publicClientMetadata: OAuthClientMetadata = { + redirect_uris: ['https://example.com/callback'], + token_endpoint_auth_method: 'client_secret_basic' + }; + + const publicResponse = await supertest(app) + .post('/register') + .send(publicClientMetadata); + + expect(publicResponse.status).toBe(201); + expect(publicResponse.body.client_secret).toBeDefined(); + expect(publicResponse.body.client_secret_expires_at).toBeDefined(); + + // Test for non-public client (token_endpoint_auth_method is 'none') + const nonPublicClientMetadata: OAuthClientMetadata = { + redirect_uris: ['https://example.com/callback'], + token_endpoint_auth_method: 'none' + }; + + const nonPublicResponse = await supertest(app) + .post('/register') + .send(nonPublicClientMetadata); + + expect(nonPublicResponse.status).toBe(201); + expect(nonPublicResponse.body.client_secret).toBeUndefined(); + expect(nonPublicResponse.body.client_secret_expires_at).toBeUndefined(); }); it('sets expiry based on clientSecretExpirySeconds', async () => { diff --git a/src/server/auth/handlers/register.ts b/src/server/auth/handlers/register.ts index 675e8733..30b7cdf8 100644 --- a/src/server/auth/handlers/register.ts +++ b/src/server/auth/handlers/register.ts @@ -75,20 +75,26 @@ export function clientRegistrationHandler({ } const clientMetadata = parseResult.data; + const isPublicClient = clientMetadata.token_endpoint_auth_method === 'none' // Generate client credentials const clientId = crypto.randomUUID(); - const clientSecret = clientMetadata.token_endpoint_auth_method !== 'none' - ? crypto.randomBytes(32).toString('hex') - : undefined; + const clientSecret = isPublicClient + ? undefined + : crypto.randomBytes(32).toString('hex'); const clientIdIssuedAt = Math.floor(Date.now() / 1000); + // Calculate client secret expiry time + const clientsDoExpire = clientSecretExpirySeconds > 0 + const secretExpiryTime = clientsDoExpire ? clientIdIssuedAt + clientSecretExpirySeconds : 0 + const clientSecretExpiresAt = isPublicClient ? undefined : secretExpiryTime + let clientInfo: OAuthClientInformationFull = { ...clientMetadata, client_id: clientId, client_secret: clientSecret, client_id_issued_at: clientIdIssuedAt, - client_secret_expires_at: clientSecretExpirySeconds > 0 ? clientIdIssuedAt + clientSecretExpirySeconds : 0 + client_secret_expires_at: clientSecretExpiresAt, }; clientInfo = await clientsStore.registerClient!(clientInfo); diff --git a/src/server/auth/middleware/bearerAuth.test.ts b/src/server/auth/middleware/bearerAuth.test.ts index 8c0b595e..da2f5838 100644 --- a/src/server/auth/middleware/bearerAuth.test.ts +++ b/src/server/auth/middleware/bearerAuth.test.ts @@ -55,6 +55,57 @@ describe("requireBearerAuth middleware", () => { expect(mockResponse.status).not.toHaveBeenCalled(); expect(mockResponse.json).not.toHaveBeenCalled(); }); + + it("should reject expired tokens", async () => { + const expiredAuthInfo: AuthInfo = { + token: "expired-token", + clientId: "client-123", + scopes: ["read", "write"], + expiresAt: Math.floor(Date.now() / 1000) - 100, // Token expired 100 seconds ago + }; + mockVerifyAccessToken.mockResolvedValue(expiredAuthInfo); + + mockRequest.headers = { + authorization: "Bearer expired-token", + }; + + const middleware = requireBearerAuth({ provider: mockProvider }); + await middleware(mockRequest as Request, mockResponse as Response, nextFunction); + + expect(mockVerifyAccessToken).toHaveBeenCalledWith("expired-token"); + expect(mockResponse.status).toHaveBeenCalledWith(401); + expect(mockResponse.set).toHaveBeenCalledWith( + "WWW-Authenticate", + expect.stringContaining('Bearer error="invalid_token"') + ); + expect(mockResponse.json).toHaveBeenCalledWith( + expect.objectContaining({ error: "invalid_token", error_description: "Token has expired" }) + ); + expect(nextFunction).not.toHaveBeenCalled(); + }); + + it("should accept non-expired tokens", async () => { + const nonExpiredAuthInfo: AuthInfo = { + token: "valid-token", + clientId: "client-123", + scopes: ["read", "write"], + expiresAt: Math.floor(Date.now() / 1000) + 3600, // Token expires in an hour + }; + mockVerifyAccessToken.mockResolvedValue(nonExpiredAuthInfo); + + mockRequest.headers = { + authorization: "Bearer valid-token", + }; + + const middleware = requireBearerAuth({ provider: mockProvider }); + await middleware(mockRequest as Request, mockResponse as Response, nextFunction); + + expect(mockVerifyAccessToken).toHaveBeenCalledWith("valid-token"); + expect(mockRequest.auth).toEqual(nonExpiredAuthInfo); + expect(nextFunction).toHaveBeenCalled(); + expect(mockResponse.status).not.toHaveBeenCalled(); + expect(mockResponse.json).not.toHaveBeenCalled(); + }); it("should require specific scopes when configured", async () => { const authInfo: AuthInfo = { diff --git a/src/server/auth/middleware/bearerAuth.ts b/src/server/auth/middleware/bearerAuth.ts index 14109e17..cd1b314a 100644 --- a/src/server/auth/middleware/bearerAuth.ts +++ b/src/server/auth/middleware/bearerAuth.ts @@ -55,6 +55,11 @@ export function requireBearerAuth({ provider, requiredScopes = [] }: BearerAuthM } } + // Check if the token is expired + if (!!authInfo.expiresAt && authInfo.expiresAt < Date.now() / 1000) { + throw new InvalidTokenError("Token has expired"); + } + req.auth = authInfo; next(); } catch (error) { diff --git a/src/shared/protocol.test.ts b/src/shared/protocol.test.ts index 3073d0af..1d037b98 100644 --- a/src/shared/protocol.test.ts +++ b/src/shared/protocol.test.ts @@ -62,6 +62,162 @@ describe("protocol tests", () => { await transport.close(); expect(oncloseMock).toHaveBeenCalled(); }); + + describe("progress notification timeout behavior", () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { + jest.useRealTimers(); + }); + + test("should reset timeout when progress notification is received", async () => { + await protocol.connect(transport); + const request = { method: "example", params: {} }; + const mockSchema: ZodType<{ result: string }> = z.object({ + result: z.string(), + }); + const onProgressMock = jest.fn(); + const requestPromise = protocol.request(request, mockSchema, { + timeout: 1000, + resetTimeoutOnProgress: true, + onprogress: onProgressMock, + }); + jest.advanceTimersByTime(800); + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: "2.0", + method: "notifications/progress", + params: { + progressToken: 0, + progress: 50, + total: 100, + }, + }); + } + await Promise.resolve(); + expect(onProgressMock).toHaveBeenCalledWith({ + progress: 50, + total: 100, + }); + jest.advanceTimersByTime(800); + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: "2.0", + id: 0, + result: { result: "success" }, + }); + } + await Promise.resolve(); + await expect(requestPromise).resolves.toEqual({ result: "success" }); + }); + + test("should respect maxTotalTimeout", async () => { + await protocol.connect(transport); + const request = { method: "example", params: {} }; + const mockSchema: ZodType<{ result: string }> = z.object({ + result: z.string(), + }); + const onProgressMock = jest.fn(); + const requestPromise = protocol.request(request, mockSchema, { + timeout: 1000, + maxTotalTimeout: 150, + resetTimeoutOnProgress: true, + onprogress: onProgressMock, + }); + + // First progress notification should work + jest.advanceTimersByTime(80); + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: "2.0", + method: "notifications/progress", + params: { + progressToken: 0, + progress: 50, + total: 100, + }, + }); + } + await Promise.resolve(); + expect(onProgressMock).toHaveBeenCalledWith({ + progress: 50, + total: 100, + }); + jest.advanceTimersByTime(80); + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: "2.0", + method: "notifications/progress", + params: { + progressToken: 0, + progress: 75, + total: 100, + }, + }); + } + await expect(requestPromise).rejects.toThrow("Maximum total timeout exceeded"); + expect(onProgressMock).toHaveBeenCalledTimes(1); + }); + + test("should timeout if no progress received within timeout period", async () => { + await protocol.connect(transport); + const request = { method: "example", params: {} }; + const mockSchema: ZodType<{ result: string }> = z.object({ + result: z.string(), + }); + const requestPromise = protocol.request(request, mockSchema, { + timeout: 100, + resetTimeoutOnProgress: true, + }); + jest.advanceTimersByTime(101); + await expect(requestPromise).rejects.toThrow("Request timed out"); + }); + + test("should handle multiple progress notifications correctly", async () => { + await protocol.connect(transport); + const request = { method: "example", params: {} }; + const mockSchema: ZodType<{ result: string }> = z.object({ + result: z.string(), + }); + const onProgressMock = jest.fn(); + const requestPromise = protocol.request(request, mockSchema, { + timeout: 1000, + resetTimeoutOnProgress: true, + onprogress: onProgressMock, + }); + + // Simulate multiple progress updates + for (let i = 1; i <= 3; i++) { + jest.advanceTimersByTime(800); + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: "2.0", + method: "notifications/progress", + params: { + progressToken: 0, + progress: i * 25, + total: 100, + }, + }); + } + await Promise.resolve(); + expect(onProgressMock).toHaveBeenNthCalledWith(i, { + progress: i * 25, + total: 100, + }); + } + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: "2.0", + id: 0, + result: { result: "success" }, + }); + } + await Promise.resolve(); + await expect(requestPromise).resolves.toEqual({ result: "success" }); + }); + }); }); describe("mergeCapabilities", () => { diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index ef303a39..a5b6ad51 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -64,6 +64,20 @@ export type RequestOptions = { * If not specified, `DEFAULT_REQUEST_TIMEOUT_MSEC` will be used as the timeout. */ timeout?: number; + + /** + * If true, receiving a progress notification will reset the request timeout. + * This is useful for long-running operations that send periodic progress updates. + * Default: false + */ + resetTimeoutOnProgress?: boolean; + + /** + * Maximum total time (in milliseconds) to wait for a response. + * If exceeded, an McpError with code `RequestTimeout` will be raised, regardless of progress notifications. + * If not specified, there is no maximum total timeout. + */ + maxTotalTimeout?: number; }; /** @@ -81,6 +95,17 @@ export type RequestHandlerExtra = { sessionId?: string; }; +/** + * Information about a request's timeout state + */ +type TimeoutInfo = { + timeoutId: ReturnType; + startTime: number; + timeout: number; + maxTotalTimeout?: number; + onTimeout: () => void; +}; + /** * Implements MCP protocol framing on top of a pluggable transport, including * features like request/response linking, notifications, and progress. @@ -110,6 +135,7 @@ export abstract class Protocol< (response: JSONRPCResponse | Error) => void > = new Map(); private _progressHandlers: Map = new Map(); + private _timeoutInfo: Map = new Map(); /** * Callback for when the connection is closed for any reason. @@ -154,6 +180,48 @@ export abstract class Protocol< ); } + private _setupTimeout( + messageId: number, + timeout: number, + maxTotalTimeout: number | undefined, + onTimeout: () => void + ) { + this._timeoutInfo.set(messageId, { + timeoutId: setTimeout(onTimeout, timeout), + startTime: Date.now(), + timeout, + maxTotalTimeout, + onTimeout + }); + } + + private _resetTimeout(messageId: number): boolean { + const info = this._timeoutInfo.get(messageId); + if (!info) return false; + + const totalElapsed = Date.now() - info.startTime; + if (info.maxTotalTimeout && totalElapsed >= info.maxTotalTimeout) { + this._timeoutInfo.delete(messageId); + throw new McpError( + ErrorCode.RequestTimeout, + "Maximum total timeout exceeded", + { maxTotalTimeout: info.maxTotalTimeout, totalElapsed } + ); + } + + clearTimeout(info.timeoutId); + info.timeoutId = setTimeout(info.onTimeout, info.timeout); + return true; + } + + private _cleanupTimeout(messageId: number) { + const info = this._timeoutInfo.get(messageId); + if (info) { + clearTimeout(info.timeoutId); + this._timeoutInfo.delete(messageId); + } + } + /** * Attaches to the given transport, starts it, and starts listening for messages. * @@ -292,22 +360,30 @@ export abstract class Protocol< private _onprogress(notification: ProgressNotification): void { const { progressToken, ...params } = notification.params; - const handler = this._progressHandlers.get(Number(progressToken)); - if (handler === undefined) { - this._onerror( - new Error( - `Received a progress notification for an unknown token: ${JSON.stringify(notification)}`, - ), - ); + const messageId = Number(progressToken); + + const handler = this._progressHandlers.get(messageId); + if (!handler) { + this._onerror(new Error(`Received a progress notification for an unknown token: ${JSON.stringify(notification)}`)); return; } + const responseHandler = this._responseHandlers.get(messageId); + if (this._timeoutInfo.has(messageId) && responseHandler) { + try { + this._resetTimeout(messageId); + } catch (error) { + responseHandler(error as Error); + return; + } + } + handler(params); } private _onresponse(response: JSONRPCResponse | JSONRPCError): void { - const messageId = response.id; - const handler = this._responseHandlers.get(Number(messageId)); + const messageId = Number(response.id); + const handler = this._responseHandlers.get(messageId); if (handler === undefined) { this._onerror( new Error( @@ -317,8 +393,10 @@ export abstract class Protocol< return; } - this._responseHandlers.delete(Number(messageId)); - this._progressHandlers.delete(Number(messageId)); + this._responseHandlers.delete(messageId); + this._progressHandlers.delete(messageId); + this._cleanupTimeout(messageId); + if ("result" in response) { handler(response); } else { @@ -404,32 +482,10 @@ export abstract class Protocol< }; } - let timeoutId: ReturnType | undefined = undefined; - - this._responseHandlers.set(messageId, (response) => { - if (timeoutId !== undefined) { - clearTimeout(timeoutId); - } - - if (options?.signal?.aborted) { - return; - } - - if (response instanceof Error) { - return reject(response); - } - - try { - const result = resultSchema.parse(response.result); - resolve(result); - } catch (error) { - reject(error); - } - }); - const cancel = (reason: unknown) => { this._responseHandlers.delete(messageId); this._progressHandlers.delete(messageId); + this._cleanupTimeout(messageId); this._transport ?.send({ @@ -447,30 +503,38 @@ export abstract class Protocol< reject(reason); }; - options?.signal?.addEventListener("abort", () => { - if (timeoutId !== undefined) { - clearTimeout(timeoutId); + this._responseHandlers.set(messageId, (response) => { + if (options?.signal?.aborted) { + return; } + if (response instanceof Error) { + return reject(response); + } + + try { + const result = resultSchema.parse(response.result); + resolve(result); + } catch (error) { + reject(error); + } + }); + + options?.signal?.addEventListener("abort", () => { cancel(options?.signal?.reason); }); const timeout = options?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MSEC; - timeoutId = setTimeout( - () => - cancel( - new McpError(ErrorCode.RequestTimeout, "Request timed out", { - timeout, - }), - ), - timeout, - ); + const timeoutHandler = () => cancel(new McpError( + ErrorCode.RequestTimeout, + "Request timed out", + { timeout } + )); + + this._setupTimeout(messageId, timeout, options?.maxTotalTimeout, timeoutHandler); this._transport.send(jsonrpcRequest).catch((error) => { - if (timeoutId !== undefined) { - clearTimeout(timeoutId); - } - + this._cleanupTimeout(messageId); reject(error); }); });