From 3f9500f9548cb1c9032dfc6ccf0145f311deaefa Mon Sep 17 00:00:00 2001 From: Maxwell Gerber Date: Mon, 7 Apr 2025 14:53:16 -0700 Subject: [PATCH 1/5] feat: QoL improvements for OAuth Callback --- client/src/App.tsx | 44 +++++-------- client/src/components/OAuthCallback.tsx | 72 +++++++++++++++------- client/src/utils/__tests__/oauthUtils.ts | 78 ++++++++++++++++++++++++ client/src/utils/oauthUtils.ts | 65 ++++++++++++++++++++ 4 files changed, 207 insertions(+), 52 deletions(-) create mode 100644 client/src/utils/__tests__/oauthUtils.ts create mode 100644 client/src/utils/oauthUtils.ts diff --git a/client/src/App.tsx b/client/src/App.tsx index 7564544..3e83e5d 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -17,7 +17,13 @@ import { Tool, LoggingLevel, } from "@modelcontextprotocol/sdk/types.js"; -import React, { Suspense, useEffect, useRef, useState } from "react"; +import React, { + Suspense, + useCallback, + useEffect, + useRef, + useState, +} from "react"; import { useConnection } from "./lib/hooks/useConnection"; import { useDraggablePane } from "./lib/hooks/useDraggablePane"; import { StdErrNotification } from "./lib/notificationTypes"; @@ -49,14 +55,10 @@ import { getMCPProxyAddress, getMCPServerRequestTimeout, } from "./utils/configUtils"; -import { useToast } from "@/hooks/use-toast"; -const params = new URLSearchParams(window.location.search); const CONFIG_LOCAL_STORAGE_KEY = "inspectorConfig_v1"; const App = () => { - const { toast } = useToast(); - // Handle OAuth callback route const [resources, setResources] = useState([]); const [resourceTemplates, setResourceTemplates] = useState< ResourceTemplate[] @@ -205,31 +207,15 @@ const App = () => { localStorage.setItem(CONFIG_LOCAL_STORAGE_KEY, JSON.stringify(config)); }, [config]); - const hasProcessedRef = useRef(false); - // Auto-connect if serverUrl is provided in URL params (e.g. after OAuth callback) - useEffect(() => { - if (hasProcessedRef.current) { - // Only try to connect once - return; - } - const serverUrl = params.get("serverUrl"); - if (serverUrl) { + // Auto-connect to previously saved serverURL after OAuth callback + const onOAuthConnect = useCallback( + (serverUrl: string) => { setSseUrl(serverUrl); setTransportType("sse"); - // Remove serverUrl from URL without reloading the page - const newUrl = new URL(window.location.href); - newUrl.searchParams.delete("serverUrl"); - window.history.replaceState({}, "", newUrl.toString()); - // Show success toast for OAuth - toast({ - title: "Success", - description: "Successfully authenticated with OAuth", - }); - hasProcessedRef.current = true; - // Connect to the server - connectMcpServer(); - } - }, [connectMcpServer, toast]); + void connectMcpServer(); + }, + [connectMcpServer], + ); useEffect(() => { fetch(`${getMCPProxyAddress(config)}/config`) @@ -453,7 +439,7 @@ const App = () => { ); return ( Loading...}> - + ); } diff --git a/client/src/components/OAuthCallback.tsx b/client/src/components/OAuthCallback.tsx index cba38c3..7f2b503 100644 --- a/client/src/components/OAuthCallback.tsx +++ b/client/src/components/OAuthCallback.tsx @@ -2,8 +2,18 @@ import { useEffect, useRef } from "react"; import { authProvider } from "../lib/auth"; import { SESSION_KEYS } from "../lib/constants"; import { auth } from "@modelcontextprotocol/sdk/client/auth.js"; +import { useToast } from "@/hooks/use-toast.ts"; +import { + generateOAuthErrorDescription, + parseOAuthCallbackParams, +} from "@/utils/oauthUtils.ts"; -const OAuthCallback = () => { +interface OAuthCallbackProps { + onConnect: (serverUrl: string) => void; +} + +const OAuthCallback = ({ onConnect }: OAuthCallbackProps) => { + const { toast } = useToast(); const hasProcessedRef = useRef(false); useEffect(() => { @@ -14,37 +24,53 @@ const OAuthCallback = () => { } hasProcessedRef.current = true; - const params = new URLSearchParams(window.location.search); - const code = params.get("code"); - const serverUrl = sessionStorage.getItem(SESSION_KEYS.SERVER_URL); + const notifyError = (description: string) => + void toast({ + title: "OAuth Authorization Error", + description, + variant: "destructive", + }); - if (!code || !serverUrl) { - console.error("Missing code or server URL"); - window.location.href = "/"; - return; + const params = parseOAuthCallbackParams(window.location.search); + if (!params.successful) { + return notifyError(generateOAuthErrorDescription(params)); } - try { - const result = await auth(authProvider, { - serverUrl, - authorizationCode: code, - }); - if (result !== "AUTHORIZED") { - throw new Error( - `Expected to be authorized after providing auth code, got: ${result}`, - ); - } + const serverUrl = sessionStorage.getItem(SESSION_KEYS.SERVER_URL); + if (!serverUrl) { + return notifyError("Missing Server URL"); + } - // Redirect back to the main app with server URL to trigger auto-connect - window.location.href = `/?serverUrl=${encodeURIComponent(serverUrl)}`; + let result; + try { + result = await auth(authProvider, { + serverUrl, + authorizationCode: params.code, + }); } catch (error) { console.error("OAuth callback error:", error); - window.location.href = "/"; + return notifyError(`Unexpected error occurred: ${error}`); } + + if (result !== "AUTHORIZED") { + return notifyError( + `Expected to be authorized after providing auth code, got: ${result}`, + ); + } + + // Finally, trigger auto-connect + toast({ + title: "Success", + description: "Successfully authenticated with OAuth", + variant: "default", + }); + onConnect(serverUrl); }; - void handleCallback(); - }, []); + handleCallback().finally(() => { + window.history.replaceState({}, document.title, "/"); + }); + }, [toast, onConnect]); return (
diff --git a/client/src/utils/__tests__/oauthUtils.ts b/client/src/utils/__tests__/oauthUtils.ts new file mode 100644 index 0000000..cc9674c --- /dev/null +++ b/client/src/utils/__tests__/oauthUtils.ts @@ -0,0 +1,78 @@ +import { + generateOAuthErrorDescription, + parseOAuthCallbackParams, +} from "@/utils/oauthUtils.ts"; + +describe("parseOAuthCallbackParams", () => { + it("Returns successful: true and code when present", () => { + expect(parseOAuthCallbackParams("?code=fake-code")).toEqual({ + successful: true, + code: "fake-code", + }); + }); + it("Returns successful: false and error when error is present", () => { + expect(parseOAuthCallbackParams("?error=access_denied")).toEqual({ + successful: false, + error: "access_denied", + error_description: null, + error_uri: null, + }); + }); + it("Returns optional error metadata fields when present", () => { + const search = + "?error=access_denied&" + + "error_description=User%20Denied%20Request&" + + "error_uri=https%3A%2F%2Fexample.com%2Ferror-docs"; + expect(parseOAuthCallbackParams(search)).toEqual({ + successful: false, + error: "access_denied", + error_description: "User Denied Request", + error_uri: "https://example.com/error-docs", + }); + }); + it("Returns error when nothing present", () => { + expect(parseOAuthCallbackParams("?")).toEqual({ + successful: false, + error: "invalid_request", + error_description: "Missing code or error in response", + error_uri: null, + }); + }); +}); + +describe("generateOAuthErrorDescription", () => { + it("When only error is present", () => { + expect( + generateOAuthErrorDescription({ + successful: false, + error: "invalid_request", + error_description: null, + error_uri: null, + }), + ).toBe("Error: invalid_request."); + }); + it("When error description is present", () => { + expect( + generateOAuthErrorDescription({ + successful: false, + error: "invalid_request", + error_description: "The request could not be completed as dialed", + error_uri: null, + }), + ).toEqual( + "Error: invalid_request.\nDetails: The request could not be completed as dialed.", + ); + }); + it("When all fields present", () => { + expect( + generateOAuthErrorDescription({ + successful: false, + error: "invalid_request", + error_description: "The request could not be completed as dialed", + error_uri: "https://example.com/error-docs", + }), + ).toEqual( + "Error: invalid_request.\nDetails: The request could not be completed as dialed.\nMore info: https://example.com/error-docs.", + ); + }); +}); diff --git a/client/src/utils/oauthUtils.ts b/client/src/utils/oauthUtils.ts new file mode 100644 index 0000000..c971271 --- /dev/null +++ b/client/src/utils/oauthUtils.ts @@ -0,0 +1,65 @@ +// The parsed query parameters returned by the Authorization Server +// representing either a valid authorization_code or an error +// ref: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-12#section-4.1.2 +type CallbackParams = + | { + successful: true; + // The authorization code is generated by the authorization server. + code: string; + } + | { + successful: false; + // The OAuth 2.1 Error Code. + // Usually one of: + // ``` + // invalid_request, unauthorized_client, access_denied, unsupported_response_type, + // invalid_scope, server_error, temporarily_unavailable + // ``` + error: string; + // Human-readable ASCII text providing additional information, used to assist the + // developer in understanding the error that occurred. + error_description: string | null; + // A URI identifying a human-readable web page with information about the error, + // used to provide the client developer with additional information about the error. + error_uri: string | null; + }; + +export const parseOAuthCallbackParams = (location: string): CallbackParams => { + const params = new URLSearchParams(location); + + const code = params.get("code"); + if (code) { + return { successful: true, code }; + } + + const error = params.get("error"); + const error_description = params.get("error_description"); + const error_uri = params.get("error_uri"); + + if (error) { + return { successful: false, error, error_description, error_uri }; + } + + return { + successful: false, + error: "invalid_request", + error_description: "Missing code or error in response", + error_uri: null, + }; +}; + +export const generateOAuthErrorDescription = ( + params: Extract, +): string => { + const error = params.error; + const errorDescription = params.error_description; + const errorUri = params.error_uri; + + return [ + `Error: ${error}.`, + errorDescription ? `Details: ${errorDescription}.` : "", + errorUri ? `More info: ${errorUri}.` : "", + ] + .filter(Boolean) + .join("\n"); +}; From 6a16e7cd24676f24b616b88dc9274cdca0f06224 Mon Sep 17 00:00:00 2001 From: Maxwell Gerber Date: Mon, 7 Apr 2025 15:15:28 -0700 Subject: [PATCH 2/5] fix: Disconnecting should clear oauth state --- client/src/lib/auth.ts | 6 ++++++ client/src/lib/hooks/useConnection.ts | 1 + 2 files changed, 7 insertions(+) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index ba610bc..72d194e 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -68,6 +68,12 @@ class InspectorOAuthClientProvider implements OAuthClientProvider { return verifier; } + + clear() { + sessionStorage.removeItem(SESSION_KEYS.CLIENT_INFORMATION); + sessionStorage.removeItem(SESSION_KEYS.TOKENS); + sessionStorage.removeItem(SESSION_KEYS.CODE_VERIFIER); + } } export const authProvider = new InspectorOAuthClientProvider(); diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index bff01ce..667da77 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -361,6 +361,7 @@ export function useConnection({ const disconnect = async () => { await mcpClient?.close(); + authProvider.clear(); setMcpClient(null); setConnectionStatus("disconnected"); setCompletionsSupported(false); From fa7eba23b73472de04835eb7f0ee63d38441cb64 Mon Sep 17 00:00:00 2001 From: Hoa Lam Date: Wed, 16 Apr 2025 08:48:04 +0700 Subject: [PATCH 3/5] Update README requirement node js version --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2552a01..53afa5b 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,10 @@ The MCP inspector is a developer tool for testing and debugging MCP servers. ## Running the Inspector +### Requirements + +- Node.js: ^22.7.5 + ### From an MCP server repository To inspect an MCP server implementation, there's no need to clone this repo. Instead, use `npx`. For example, if your server is built at `build/index.js`: @@ -69,7 +73,7 @@ Development mode: npm run dev ``` -> **Note for Windows users:** +> **Note for Windows users:** > On Windows, use the following command instead: > > ```bash From 15960f5aa484f0bb656abe17d877fba9ad03d21d Mon Sep 17 00:00:00 2001 From: Maxwell Gerber Date: Wed, 16 Apr 2025 16:24:49 -0700 Subject: [PATCH 4/5] refactor: Use new serverspecifickey API --- client/src/lib/auth.ts | 6 +++--- client/src/lib/hooks/useConnection.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 39b489d..65650fd 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -90,8 +90,8 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { } clear() { - sessionStorage.removeItem(SESSION_KEYS.CLIENT_INFORMATION); - sessionStorage.removeItem(SESSION_KEYS.TOKENS); - sessionStorage.removeItem(SESSION_KEYS.CODE_VERIFIER); + sessionStorage.removeItem(getServerSpecificKey(SESSION_KEYS.CLIENT_INFORMATION, this.serverUrl)); + sessionStorage.removeItem(getServerSpecificKey(SESSION_KEYS.TOKENS, this.serverUrl)); + sessionStorage.removeItem(getServerSpecificKey(SESSION_KEYS.CODE_VERIFIER, this.serverUrl)); } } diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 8c5c009..73f2e1c 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -396,6 +396,7 @@ export function useConnection({ const disconnect = async () => { await mcpClient?.close(); + const authProvider = new InspectorOAuthClientProvider(sseUrl); authProvider.clear(); setMcpClient(null); setConnectionStatus("disconnected"); From 1345a50011976f155de2c1cd84d5da5b73742451 Mon Sep 17 00:00:00 2001 From: Maxwell Gerber Date: Wed, 16 Apr 2025 16:25:51 -0700 Subject: [PATCH 5/5] lint --- client/src/lib/auth.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 65650fd..7ef3182 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -90,8 +90,14 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { } clear() { - sessionStorage.removeItem(getServerSpecificKey(SESSION_KEYS.CLIENT_INFORMATION, this.serverUrl)); - sessionStorage.removeItem(getServerSpecificKey(SESSION_KEYS.TOKENS, this.serverUrl)); - sessionStorage.removeItem(getServerSpecificKey(SESSION_KEYS.CODE_VERIFIER, this.serverUrl)); + sessionStorage.removeItem( + getServerSpecificKey(SESSION_KEYS.CLIENT_INFORMATION, this.serverUrl), + ); + sessionStorage.removeItem( + getServerSpecificKey(SESSION_KEYS.TOKENS, this.serverUrl), + ); + sessionStorage.removeItem( + getServerSpecificKey(SESSION_KEYS.CODE_VERIFIER, this.serverUrl), + ); } }