Merge branch 'main' into update-readme

cliffhall · web-flow · commit d45d8c912538 · 2025-04-17T10:53:47.000-04:00
diff --git 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";
@@ -46,14 +52,10 @@ import ToolsTab from "./components/ToolsTab";
 import { DEFAULT_INSPECTOR_CONFIG } from "./lib/constants";
 import { InspectorConfig } from "./lib/configurationTypes";
 import { getMCPProxyAddress } 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<Resource[]>([]);
   const [resourceTemplates, setResourceTemplates] = useState<
     ResourceTemplate[]
@@ -221,31 +223,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`)
@@ -486,7 +472,7 @@ const App = () => {
     );
     return (
       <Suspense fallback={<div>Loading...</div>}>
-        <OAuthCallback />
+        <OAuthCallback onConnect={onOAuthConnect} />
       </Suspense>
     );
   }
diff --git a/client/src/components/OAuthCallback.tsx b/client/src/components/OAuthCallback.tsx
@@ -2,8 +2,18 @@ import { useEffect, useRef } from "react";
 import { InspectorOAuthClientProvider } 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,40 +24,56 @@ 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));
+      }
+
+      const serverUrl = sessionStorage.getItem(SESSION_KEYS.SERVER_URL);
+      if (!serverUrl) {
+        return notifyError("Missing Server URL");
       }
 
+      let result;
       try {
         // Create an auth provider with the current server URL
         const serverAuthProvider = new InspectorOAuthClientProvider(serverUrl);
 
-        const result = await auth(serverAuthProvider, {
+        result = await auth(serverAuthProvider, {
           serverUrl,
-          authorizationCode: code,
+          authorizationCode: params.code,
         });
-        if (result !== "AUTHORIZED") {
-          throw new Error(
-            `Expected to be authorized after providing auth code, got: ${result}`,
-          );
-        }
-
-        // Redirect back to the main app with server URL to trigger auto-connect
-        window.location.href = `/?serverUrl=${encodeURIComponent(serverUrl)}`;
       } 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 (
     <div className="flex items-center justify-center h-screen">
diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts
@@ -88,4 +88,16 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
 
     return verifier;
   }
+
+  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),
+    );
+  }
 }
diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts
@@ -396,6 +396,8 @@ export function useConnection({
 
   const disconnect = async () => {
     await mcpClient?.close();
+    const authProvider = new InspectorOAuthClientProvider(sseUrl);
+    authProvider.clear();
     setMcpClient(null);
     setConnectionStatus("disconnected");
     setCompletionsSupported(false);
diff --git a/client/src/utils/__tests__/oauthUtils.ts 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
@@ -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<CallbackParams, { successful: false }>,
+): 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");
+};
