From 929b93061865afed3bed38ad19ee08abf4e3003b Mon Sep 17 00:00:00 2001 From: Konstantin Konstantinov Date: Mon, 11 Aug 2025 21:24:26 +0300 Subject: [PATCH 1/6] reusable generateOAuthState helper, introduce in InspectorOAuthClientProvider and in oauth-state-machine, add test --- client/src/lib/auth.ts | 5 +++++ client/src/lib/oauth-state-machine.ts | 10 ++-------- .../{oauthUtils.ts => oauthUtils.test.ts} | 8 ++++++++ client/src/utils/oauthUtils.ts | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 8 deletions(-) rename client/src/utils/__tests__/{oauthUtils.ts => oauthUtils.test.ts} (92%) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 9989e944..73217a60 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -8,6 +8,7 @@ import { OAuthMetadata, } from "@modelcontextprotocol/sdk/shared/auth.js"; import { SESSION_KEYS, getServerSpecificKey } from "./constants"; +import { generateOAuthState } from "@/utils/oauthUtils"; export const getClientInformationFromSessionStorage = async ({ serverUrl, @@ -75,6 +76,10 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { return window.location.origin + "/oauth/callback"; } + state(): string | Promise { + return generateOAuthState(); + } + get clientMetadata(): OAuthClientMetadata { return { redirect_uris: [this.redirectUrl], diff --git a/client/src/lib/oauth-state-machine.ts b/client/src/lib/oauth-state-machine.ts index 5078cfe4..eec7ce8d 100644 --- a/client/src/lib/oauth-state-machine.ts +++ b/client/src/lib/oauth-state-machine.ts @@ -12,6 +12,7 @@ import { OAuthMetadataSchema, OAuthProtectedResourceMetadata, } from "@modelcontextprotocol/sdk/shared/auth.js"; +import { generateOAuthState } from "@/utils/oauthUtils"; export interface StateMachineContext { state: AuthDebuggerState; @@ -113,13 +114,6 @@ export const oauthTransitions: Record = { scope = metadata.scopes_supported.join(" "); } - // Generate a random state - const array = new Uint8Array(32); - crypto.getRandomValues(array); - const state = Array.from(array, (byte) => - byte.toString(16).padStart(2, "0"), - ).join(""); - const { authorizationUrl, codeVerifier } = await startAuthorization( context.serverUrl, { @@ -127,7 +121,7 @@ export const oauthTransitions: Record = { clientInformation, redirectUrl: context.provider.redirectUrl, scope, - state: state, + state: generateOAuthState(), resource: context.state.resource ?? undefined, }, ); diff --git a/client/src/utils/__tests__/oauthUtils.ts b/client/src/utils/__tests__/oauthUtils.test.ts similarity index 92% rename from client/src/utils/__tests__/oauthUtils.ts rename to client/src/utils/__tests__/oauthUtils.test.ts index cc9674cb..c42f014e 100644 --- a/client/src/utils/__tests__/oauthUtils.ts +++ b/client/src/utils/__tests__/oauthUtils.test.ts @@ -1,6 +1,7 @@ import { generateOAuthErrorDescription, parseOAuthCallbackParams, + generateOAuthState, } from "@/utils/oauthUtils.ts"; describe("parseOAuthCallbackParams", () => { @@ -75,4 +76,11 @@ describe("generateOAuthErrorDescription", () => { "Error: invalid_request.\nDetails: The request could not be completed as dialed.\nMore info: https://example.com/error-docs.", ); }); + + describe("generateOAuthState", () => { + it("Returns a string", () => { + expect(generateOAuthState()).toBeDefined(); + expect(generateOAuthState()).toHaveLength(64); + }); + }); }); diff --git a/client/src/utils/oauthUtils.ts b/client/src/utils/oauthUtils.ts index c971271e..83de080b 100644 --- a/client/src/utils/oauthUtils.ts +++ b/client/src/utils/oauthUtils.ts @@ -48,6 +48,20 @@ export const parseOAuthCallbackParams = (location: string): CallbackParams => { }; }; +/** + * Generate a random state for the OAuth 2.0 flow. + * + * @returns A random state for the OAuth 2.0 flow. + */ +export const generateOAuthState = () => { + // Generate a random state + const array = new Uint8Array(32); + crypto.getRandomValues(array); + return Array.from(array, (byte) => byte.toString(16).padStart(2, "0")).join( + "", + ); +}; + export const generateOAuthErrorDescription = ( params: Extract, ): string => { From 60049a5b6ce1c8ad9ebc82960fd7e5c8287f1fe5 Mon Sep 17 00:00:00 2001 From: Konstantin Konstantinov Date: Tue, 12 Aug 2025 00:29:53 +0300 Subject: [PATCH 2/6] reorder methods --- client/src/lib/auth.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 73217a60..0e044037 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -76,10 +76,6 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { return window.location.origin + "/oauth/callback"; } - state(): string | Promise { - return generateOAuthState(); - } - get clientMetadata(): OAuthClientMetadata { return { redirect_uris: [this.redirectUrl], @@ -91,6 +87,10 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { }; } + state(): string | Promise { + return generateOAuthState(); + } + async clientInformation() { // Try to get the preregistered client information from session storage first const preregisteredClientInformation = From 9ef2a6c22d98c3f7a633d40bd627377bcb8aafcc Mon Sep 17 00:00:00 2001 From: 2underscores Date: Tue, 12 Aug 2025 14:13:31 +1000 Subject: [PATCH 3/6] Use new auth svr discovery method from latest auth SDK --- .../__tests__/AuthDebugger.test.tsx | 31 ++++++++++++------- client/src/lib/oauth-state-machine.ts | 4 +-- package-lock.json | 8 ++--- package.json | 2 +- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/client/src/components/__tests__/AuthDebugger.test.tsx b/client/src/components/__tests__/AuthDebugger.test.tsx index fa9f2f7d..3d8e2290 100644 --- a/client/src/components/__tests__/AuthDebugger.test.tsx +++ b/client/src/components/__tests__/AuthDebugger.test.tsx @@ -36,7 +36,7 @@ const mockOAuthClientInfo = { // Mock MCP SDK functions - must be before imports jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ auth: jest.fn(), - discoverOAuthMetadata: jest.fn(), + discoverAuthorizationServerMetadata: jest.fn(), registerClient: jest.fn(), startAuthorization: jest.fn(), exchangeAuthorization: jest.fn(), @@ -46,7 +46,7 @@ jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ // Import the functions to get their types import { - discoverOAuthMetadata, + discoverAuthorizationServerMetadata, registerClient, startAuthorization, exchangeAuthorization, @@ -57,9 +57,10 @@ import { OAuthMetadata } from "@modelcontextprotocol/sdk/shared/auth.js"; import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types"; // Type the mocked functions properly -const mockDiscoverOAuthMetadata = discoverOAuthMetadata as jest.MockedFunction< - typeof discoverOAuthMetadata ->; +const mockDiscoverAuthorizationServerMetadata = + discoverAuthorizationServerMetadata as jest.MockedFunction< + typeof discoverAuthorizationServerMetadata + >; const mockRegisterClient = registerClient as jest.MockedFunction< typeof registerClient >; @@ -102,7 +103,9 @@ describe("AuthDebugger", () => { // Suppress console errors in tests to avoid JSDOM navigation noise jest.spyOn(console, "error").mockImplementation(() => {}); - mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata); + mockDiscoverAuthorizationServerMetadata.mockResolvedValue( + mockOAuthMetadata, + ); mockRegisterClient.mockResolvedValue(mockOAuthClientInfo); mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue( new Error("No protected resource metadata found"), @@ -203,7 +206,7 @@ describe("AuthDebugger", () => { }); // Should first discover and save OAuth metadata - expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith( + expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith( new URL("https://example.com/"), ); @@ -216,7 +219,7 @@ describe("AuthDebugger", () => { }); it("should show error when quick OAuth flow fails to discover metadata", async () => { - mockDiscoverOAuthMetadata.mockRejectedValue( + mockDiscoverAuthorizationServerMetadata.mockRejectedValue( new Error("Metadata discovery failed"), ); @@ -362,7 +365,7 @@ describe("AuthDebugger", () => { fireEvent.click(screen.getByText("Continue")); }); - expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith( + expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith( new URL("https://example.com/"), ); }); @@ -509,7 +512,9 @@ describe("AuthDebugger", () => { mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValue( mockResourceMetadata, ); - mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata); + mockDiscoverAuthorizationServerMetadata.mockResolvedValue( + mockOAuthMetadata, + ); await act(async () => { renderAuthDebugger({ @@ -563,7 +568,9 @@ describe("AuthDebugger", () => { // Mock failed metadata discovery mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(mockError); // But OAuth metadata should still work with the original URL - mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata); + mockDiscoverAuthorizationServerMetadata.mockResolvedValue( + mockOAuthMetadata, + ); await act(async () => { renderAuthDebugger({ @@ -603,7 +610,7 @@ describe("AuthDebugger", () => { }); // Verify that regular OAuth metadata discovery was still called - expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith( + expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith( new URL("https://example.com/"), ); }); diff --git a/client/src/lib/oauth-state-machine.ts b/client/src/lib/oauth-state-machine.ts index 5078cfe4..851f5746 100644 --- a/client/src/lib/oauth-state-machine.ts +++ b/client/src/lib/oauth-state-machine.ts @@ -1,7 +1,7 @@ import { OAuthStep, AuthDebuggerState } from "./auth-types"; import { DebugInspectorOAuthClientProvider } from "./auth"; import { - discoverOAuthMetadata, + discoverAuthorizationServerMetadata, registerClient, startAuthorization, exchangeAuthorization, @@ -56,7 +56,7 @@ export const oauthTransitions: Record = { resourceMetadata ?? undefined, ); - const metadata = await discoverOAuthMetadata(authServerUrl); + const metadata = await discoverAuthorizationServerMetadata(authServerUrl); if (!metadata) { throw new Error("Failed to discover OAuth metadata"); } diff --git a/package-lock.json b/package-lock.json index f088d270..9fb2ed00 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,7 @@ "@modelcontextprotocol/inspector-cli": "^0.16.3", "@modelcontextprotocol/inspector-client": "^0.16.3", "@modelcontextprotocol/inspector-server": "^0.16.3", - "@modelcontextprotocol/sdk": "^1.17.0", + "@modelcontextprotocol/sdk": "^1.17.2", "concurrently": "^9.2.0", "open": "^10.2.0", "shell-quote": "^1.8.3", @@ -1999,9 +1999,9 @@ "link": true }, "node_modules/@modelcontextprotocol/sdk": { - "version": "1.17.0", - "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.17.0.tgz", - "integrity": "sha512-qFfbWFA7r1Sd8D697L7GkTd36yqDuTkvz0KfOGkgXR8EUhQn3/EDNIR/qUdQNMT8IjmasBvHWuXeisxtXTQT2g==", + "version": "1.17.2", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.17.2.tgz", + "integrity": "sha512-EFLRNXR/ixpXQWu6/3Cu30ndDFIFNaqUXcTqsGebujeMan9FzhAaFFswLRiFj61rgygDRr8WO1N+UijjgRxX9g==", "license": "MIT", "dependencies": { "ajv": "^6.12.6", diff --git a/package.json b/package.json index 9de5f464..bd494090 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,7 @@ "@modelcontextprotocol/inspector-cli": "^0.16.3", "@modelcontextprotocol/inspector-client": "^0.16.3", "@modelcontextprotocol/inspector-server": "^0.16.3", - "@modelcontextprotocol/sdk": "^1.17.0", + "@modelcontextprotocol/sdk": "^1.17.2", "concurrently": "^9.2.0", "open": "^10.2.0", "shell-quote": "^1.8.3", From d21e51e6ab132bbe0e725da36507d2ed37adabfe Mon Sep 17 00:00:00 2001 From: superboy-zjc <1826599908@qq.com> Date: Wed, 6 Aug 2025 13:00:08 -0700 Subject: [PATCH 4/6] fix(auth): sanitize authorization URL --- client/src/lib/auth.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 9989e944..326164e6 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -129,6 +129,12 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { } redirectToAuthorization(authorizationUrl: URL) { + if ( + authorizationUrl.protocol !== "http:" && + authorizationUrl.protocol !== "https:" + ) { + throw new Error("Authorization URL must be HTTP or HTTPS"); + } window.location.href = authorizationUrl.href; } From 15b58c8142a54302152077553fe515230198bad6 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Tue, 12 Aug 2025 15:25:09 -0400 Subject: [PATCH 5/6] This fixes #610 * In useConnection.ts - During initialization, if there is an error immediately following the client sending 'logging/setLevel', pops up a toast with the message `Server declares logging capability but doesn't implement method "logging/setLevel"` --- client/src/lib/hooks/useConnection.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 78dfa6df..9b03f748 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -351,6 +351,7 @@ export function useConnection({ return; } + let lastRequest = ""; try { // Inject auth manually instead of using SSEClientTransport, because we're // proxying through the inspector server first. @@ -564,7 +565,9 @@ export function useConnection({ } if (capabilities?.logging && defaultLoggingLevel) { + lastRequest = "logging/setLevel"; await client.setLoggingLevel(defaultLoggingLevel); + lastRequest = ""; } if (onElicitationRequest) { @@ -578,6 +581,13 @@ export function useConnection({ setMcpClient(client); setConnectionStatus("connected"); } catch (e) { + if (lastRequest === "logging/setLevel" && e instanceof McpError && e.code === ErrorCode.MethodNotFound) { + toast({ + title: "Error", + description: `Server declares logging capability but doesn't implement method: "${lastRequest}"`, + variant: "destructive", + }); + } console.error(e); setConnectionStatus("error"); } From 0d8e8fce4bbcb4aa815a0dc6929f7e58dfd9839c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Tue, 12 Aug 2025 15:26:15 -0400 Subject: [PATCH 6/6] This fixes #610 * In useConnection.ts - During initialization, if there is an error immediately following the client sending 'logging/setLevel', pops up a toast with the message `Server declares logging capability but doesn't implement method "logging/setLevel"` --- client/src/lib/hooks/useConnection.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 9b03f748..12d8ff3e 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -581,7 +581,11 @@ export function useConnection({ setMcpClient(client); setConnectionStatus("connected"); } catch (e) { - if (lastRequest === "logging/setLevel" && e instanceof McpError && e.code === ErrorCode.MethodNotFound) { + if ( + lastRequest === "logging/setLevel" && + e instanceof McpError && + e.code === ErrorCode.MethodNotFound + ) { toast({ title: "Error", description: `Server declares logging capability but doesn't implement method: "${lastRequest}"`,