Merge branch 'main' into fix-reset-timeout-on-progress

This commit is contained in:
Cliff Hall 2025-04-02 18:52:22 -04:00 • committed by GitHub
commit deb2737d0f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 465 additions and 30 deletions

View file

@ -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,
@ -192,6 +250,7 @@ describe("OAuth Authorization", () => {
clientInformation: validClientInfo,
authorizationCode: "code123",
codeVerifier: "verifier123",
redirectUri: "http://localhost:3000/callback",
});
expect(tokens).toEqual(validTokens);
@ -213,6 +272,7 @@ describe("OAuth Authorization", () => {
expect(body.get("code_verifier")).toBe("verifier123");
expect(body.get("client_id")).toBe("client123");
expect(body.get("client_secret")).toBe("secret123");
expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback");
});
it("validates token response schema", async () => {
@ -230,6 +290,7 @@ describe("OAuth Authorization", () => {
clientInformation: validClientInfo,
authorizationCode: "code123",
codeVerifier: "verifier123",
redirectUri: "http://localhost:3000/callback",
})
).rejects.toThrow();
});
@ -245,6 +306,7 @@ describe("OAuth Authorization", () => {
clientInformation: validClientInfo,
authorizationCode: "code123",
codeVerifier: "verifier123",
redirectUri: "http://localhost:3000/callback",
})
).rejects.toThrow("Token exchange failed");
});

View file

@ -115,6 +115,7 @@ export async function auth(
clientInformation,
authorizationCode,
codeVerifier,
redirectUri: provider.redirectUrl,
});
await provider.saveTokens(tokens);
@ -163,11 +164,21 @@ export async function discoverOAuthMetadata(
opts?: { protocolVersion?: string },
): Promise<OAuthMetadata | undefined> {
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;
@ -249,11 +260,13 @@ export async function exchangeAuthorization(
clientInformation,
authorizationCode,
codeVerifier,
redirectUri,
}: {
metadata?: OAuthMetadata;
clientInformation: OAuthClientInformation;
authorizationCode: string;
codeVerifier: string;
redirectUri: string | URL;
},
): Promise<OAuthTokens> {
const grantType = "authorization_code";
@ -280,6 +293,7 @@ export async function exchangeAuthorization(
client_id: clientInformation.client_id,
code: authorizationCode,
code_verifier: codeVerifier,
redirect_uri: String(redirectUri),
});
if (clientInformation.client_secret) {

View file

@ -0,0 +1,131 @@
import { StdioClientTransport } from "./stdio.js";
import spawn from "cross-spawn";
import { JSONRPCMessage } from "../types.js";
import { ChildProcess } from "node:child_process";
// mock cross-spawn
jest.mock("cross-spawn");
const mockSpawn = spawn as jest.MockedFunction<typeof spawn>;
describe("StdioClientTransport using cross-spawn", () => {
beforeEach(() => {
// mock cross-spawn's return value
mockSpawn.mockImplementation(() => {
const mockProcess: {
on: jest.Mock;
stdin?: { on: jest.Mock; write: jest.Mock };
stdout?: { on: jest.Mock };
stderr?: null;
} = {
on: jest.fn((event: string, callback: () => void) => {
if (event === "spawn") {
callback();
}
return mockProcess;
}),
stdin: {
on: jest.fn(),
write: jest.fn().mockReturnValue(true)
},
stdout: {
on: jest.fn()
},
stderr: null
};
return mockProcess as unknown as ChildProcess;
});
});
afterEach(() => {
jest.clearAllMocks();
});
test("should call cross-spawn correctly", async () => {
const transport = new StdioClientTransport({
command: "test-command",
args: ["arg1", "arg2"]
});
await transport.start();
// verify spawn is called correctly
expect(mockSpawn).toHaveBeenCalledWith(
"test-command",
["arg1", "arg2"],
expect.objectContaining({
shell: false
})
);
});
test("should pass environment variables correctly", async () => {
const customEnv = { TEST_VAR: "test-value" };
const transport = new StdioClientTransport({
command: "test-command",
env: customEnv
});
await transport.start();
// verify environment variables are passed correctly
expect(mockSpawn).toHaveBeenCalledWith(
"test-command",
[],
expect.objectContaining({
env: customEnv
})
);
});
test("should send messages correctly", async () => {
const transport = new StdioClientTransport({
command: "test-command"
});
// get the mock process object
const mockProcess: {
on: jest.Mock;
stdin: {
on: jest.Mock;
write: jest.Mock;
once: jest.Mock;
};
stdout: {
on: jest.Mock;
};
stderr: null;
} = {
on: jest.fn((event: string, callback: () => void) => {
if (event === "spawn") {
callback();
}
return mockProcess;
}),
stdin: {
on: jest.fn(),
write: jest.fn().mockReturnValue(true),
once: jest.fn()
},
stdout: {
on: jest.fn()
},
stderr: null
};
mockSpawn.mockReturnValue(mockProcess as unknown as ChildProcess);
await transport.start();
// 关键修复:确保 jsonrpc 是字面量 "2.0"
const message: JSONRPCMessage = {
jsonrpc: "2.0",
id: "test-id",
method: "test-method"
};
await transport.send(message);
// verify message is sent correctly
expect(mockProcess.stdin.write).toHaveBeenCalled();
});
});

View file

@ -1,4 +1,5 @@
import { ChildProcess, IOType, spawn } from "node:child_process";
import { ChildProcess, IOType } from "node:child_process";
import spawn from "cross-spawn";
import process from "node:process";
import { Stream } from "node:stream";
import { ReadBuffer, serializeMessage } from "../shared/stdio.js";

View file

@ -11,6 +11,7 @@ export class InMemoryTransport implements Transport {
onclose?: () => void;
onerror?: (error: Error) => void;
onmessage?: (message: JSONRPCMessage) => void;
sessionId?: string;
/**
* Creates a pair of linked in-memory transports that can communicate with each other. One should be passed to a Client and one to a Server.

View file

@ -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 () => {

View file

@ -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);

View file

@ -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 = {

View file

@ -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) {

View file

@ -323,6 +323,59 @@ describe("tool()", () => {
mcpServer.tool("tool2", () => ({ content: [] }));
});
test("should pass sessionId to tool callback via RequestHandlerExtra", async () => {
const mcpServer = new McpServer({
name: "test server",
version: "1.0",
});
const client = new Client(
{
name: "test client",
version: "1.0",
},
{
capabilities: {
tools: {},
},
},
);
let receivedSessionId: string | undefined;
mcpServer.tool("test-tool", async (extra) => {
receivedSessionId = extra.sessionId;
return {
content: [
{
type: "text",
text: "Test response",
},
],
};
});
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
// Set a test sessionId on the server transport
serverTransport.sessionId = "test-session-123";
await Promise.all([
client.connect(clientTransport),
mcpServer.server.connect(serverTransport),
]);
await client.request(
{
method: "tools/call",
params: {
name: "test-tool",
},
},
CallToolResultSchema,
);
expect(receivedSessionId).toBe("test-session-123");
});
test("should allow client to call server tools", async () => {
const mcpServer = new McpServer({
name: "test server",

View file

@ -88,6 +88,11 @@ export type RequestHandlerExtra = {
* An abort signal used to communicate if the request was cancelled from the sender's side.
*/
signal: AbortSignal;
/**
* The session ID from the transport, if available.
*/
sessionId?: string;
};
/**
@ -310,9 +315,15 @@ export abstract class Protocol<
const abortController = new AbortController();
this._requestHandlerAbortControllers.set(request.id, abortController);
// Create extra object with both abort signal and sessionId from transport
const extra: RequestHandlerExtra = {
signal: abortController.signal,
sessionId: this._transport?.sessionId,
};
// Starting with Promise.resolve() puts any synchronous errors into the monad as well.
Promise.resolve()
.then(() => handler(request, { signal: abortController.signal }))
.then(() => handler(request, extra))
.then(
(result) => {
if (abortController.signal.aborted) {

View file

@ -41,4 +41,9 @@ export interface Transport {
* Callback for when a message (request or response) is received over the connection.
*/
onmessage?: (message: JSONRPCMessage) => void;
/**
* The session ID generated for this connection.
*/
sessionId?: string;
}

View file

@ -666,6 +666,23 @@ export const ImageContentSchema = z
})
.passthrough();
/**
* An Audio provided to or from an LLM.
*/
export const AudioContentSchema = z
.object({
type: z.literal("audio"),
/**
* The base64-encoded audio data.
*/
data: z.string().base64(),
/**
* The MIME type of the audio. Different providers may support different audio types.
*/
mimeType: z.string(),
})
.passthrough();
/**
* The contents of a resource, embedded into a prompt or tool call result.
*/
@ -685,6 +702,7 @@ export const PromptMessageSchema = z
content: z.union([
TextContentSchema,
ImageContentSchema,
AudioContentSchema,
EmbeddedResourceSchema,
]),
})
@ -753,7 +771,7 @@ export const ListToolsResultSchema = PaginatedResultSchema.extend({
*/
export const CallToolResultSchema = ResultSchema.extend({
content: z.array(
z.union([TextContentSchema, ImageContentSchema, EmbeddedResourceSchema]),
z.union([TextContentSchema, ImageContentSchema, AudioContentSchema, EmbeddedResourceSchema]),
),
isError: z.boolean().default(false).optional(),
});
@ -877,7 +895,7 @@ export const ModelPreferencesSchema = z
export const SamplingMessageSchema = z
.object({
role: z.enum(["user", "assistant"]),
content: z.union([TextContentSchema, ImageContentSchema]),
content: z.union([TextContentSchema, ImageContentSchema, AudioContentSchema]),
})
.passthrough();
@ -931,6 +949,7 @@ export const CreateMessageResultSchema = ResultSchema.extend({
content: z.discriminatedUnion("type", [
TextContentSchema,
ImageContentSchema,
AudioContentSchema
]),
});
@ -1195,6 +1214,7 @@ export type ListPromptsResult = Infer<typeof ListPromptsResultSchema>;
export type GetPromptRequest = Infer<typeof GetPromptRequestSchema>;
export type TextContent = Infer<typeof TextContentSchema>;
export type ImageContent = Infer<typeof ImageContentSchema>;
export type AudioContent = Infer<typeof AudioContentSchema>;
export type EmbeddedResource = Infer<typeof EmbeddedResourceSchema>;
export type PromptMessage = Infer<typeof PromptMessageSchema>;
export type GetPromptResult = Infer<typeof GetPromptResultSchema>;