Merge branch 'main' into halter73/restore-resource

client/src/components/__tests__/AuthDebugger.test.tsx

@@ -25,6 +25,7 @@ const mockOAuthMetadata = {
   token_endpoint: "https://oauth.example.com/token",
   response_types_supported: ["code"],
   grant_types_supported: ["authorization_code"],
+  scopes_supported: ["read", "write"],
 };
 
 const mockOAuthClientInfo = {
@@ -56,6 +57,57 @@ import {
 import { OAuthMetadata } from "@modelcontextprotocol/sdk/shared/auth.js";
 import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types";
 
+// Mock local auth module
+jest.mock("@/lib/auth", () => ({
+  DebugInspectorOAuthClientProvider: jest.fn().mockImplementation(() => ({
+    tokens: jest.fn().mockImplementation(() => Promise.resolve(undefined)),
+    clear: jest.fn().mockImplementation(() => {
+      // Mock the real clear() behavior which removes items from sessionStorage
+      sessionStorage.removeItem("[https://example.com/mcp] mcp_tokens");
+      sessionStorage.removeItem("[https://example.com/mcp] mcp_client_info");
+      sessionStorage.removeItem(
+        "[https://example.com/mcp] mcp_server_metadata",
+      );
+    }),
+    redirectUrl: "http://localhost:3000/oauth/callback/debug",
+    clientMetadata: {
+      redirect_uris: ["http://localhost:3000/oauth/callback/debug"],
+      token_endpoint_auth_method: "none",
+      grant_types: ["authorization_code", "refresh_token"],
+      response_types: ["code"],
+      client_name: "MCP Inspector",
+    },
+    clientInformation: jest.fn().mockImplementation(async () => {
+      const serverUrl = "https://example.com/mcp";
+      const preregisteredKey = `[${serverUrl}] ${SESSION_KEYS.PREREGISTERED_CLIENT_INFORMATION}`;
+      const preregisteredData = sessionStorage.getItem(preregisteredKey);
+      if (preregisteredData) {
+        return JSON.parse(preregisteredData);
+      }
+      const dynamicKey = `[${serverUrl}] ${SESSION_KEYS.CLIENT_INFORMATION}`;
+      const dynamicData = sessionStorage.getItem(dynamicKey);
+      if (dynamicData) {
+        return JSON.parse(dynamicData);
+      }
+      return undefined;
+    }),
+    saveClientInformation: jest.fn().mockImplementation((clientInfo) => {
+      const serverUrl = "https://example.com/mcp";
+      const key = `[${serverUrl}] ${SESSION_KEYS.CLIENT_INFORMATION}`;
+      sessionStorage.setItem(key, JSON.stringify(clientInfo));
+    }),
+    saveTokens: jest.fn(),
+    redirectToAuthorization: jest.fn(),
+    saveCodeVerifier: jest.fn(),
+    codeVerifier: jest.fn(),
+    saveServerMetadata: jest.fn(),
+    getServerMetadata: jest.fn(),
+  })),
+  discoverScopes: jest.fn().mockResolvedValue("read write" as never),
+}));
+
+import { discoverScopes } from "@/lib/auth";
+
 // Type the mocked functions properly
 const mockDiscoverAuthorizationServerMetadata =
   discoverAuthorizationServerMetadata as jest.MockedFunction<
@@ -75,6 +127,9 @@ const mockDiscoverOAuthProtectedResourceMetadata =
   discoverOAuthProtectedResourceMetadata as jest.MockedFunction<
     typeof discoverOAuthProtectedResourceMetadata
   >;
+const mockDiscoverScopes = discoverScopes as jest.MockedFunction<
+  typeof discoverScopes
+>;
 
 const sessionStorageMock = {
   getItem: jest.fn(),
@@ -103,9 +158,15 @@ describe("AuthDebugger", () => {
     // Suppress console errors in tests to avoid JSDOM navigation noise
     jest.spyOn(console, "error").mockImplementation(() => {});
 
-    mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
+    // Set default mock behaviors with complete OAuth metadata
-      mockOAuthMetadata,
+    mockDiscoverAuthorizationServerMetadata.mockResolvedValue({
-    );
+      issuer: "https://oauth.example.com",
+      authorization_endpoint: "https://oauth.example.com/authorize",
+      token_endpoint: "https://oauth.example.com/token",
+      response_types_supported: ["code"],
+      grant_types_supported: ["authorization_code"],
+      scopes_supported: ["read", "write"],
+    });
     mockRegisterClient.mockResolvedValue(mockOAuthClientInfo);
     mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(
       new Error("No protected resource metadata found"),
@@ -427,7 +488,24 @@ describe("AuthDebugger", () => {
       });
     });
 
-    it("should not include scope in authorization URL when scopes_supported is not present", async () => {
+    it("should include scope in authorization URL when scopes_supported is not present", async () => {
+      const updateAuthState =
+        await setupAuthorizationUrlTest(mockOAuthMetadata);
+
+      // Wait for the updateAuthState to be called
+      await waitFor(() => {
+        expect(updateAuthState).toHaveBeenCalledWith(
+          expect.objectContaining({
+            authorizationUrl: expect.stringContaining("scope="),
+          }),
+        );
+      });
+    });
+
+    it("should omit scope from authorization URL when discoverScopes returns undefined", async () => {
+      // Mock discoverScopes to return undefined (no scopes available)
+      mockDiscoverScopes.mockResolvedValueOnce(undefined);
+
       const updateAuthState =
         await setupAuthorizationUrlTest(mockOAuthMetadata);
 

client/src/lib/__tests__/auth.test.ts

@@ -0,0 +1,155 @@
+import { discoverScopes } from "../auth";
+import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";
+
+jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
+  discoverAuthorizationServerMetadata: jest.fn(),
+}));
+
+const mockDiscoverAuth =
+  discoverAuthorizationServerMetadata as jest.MockedFunction<
+    typeof discoverAuthorizationServerMetadata
+  >;
+
+const baseMetadata = {
+  issuer: "https://test.com",
+  authorization_endpoint: "https://test.com/authorize",
+  token_endpoint: "https://test.com/token",
+  response_types_supported: ["code"],
+  grant_types_supported: ["authorization_code"],
+  scopes_supported: ["read", "write"],
+};
+
+describe("discoverScopes", () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  const testCases = [
+    {
+      name: "returns joined scopes from OAuth metadata",
+      mockResolves: baseMetadata,
+      serverUrl: "https://example.com",
+      expected: "read write",
+      expectedCallUrl: "https://example.com/",
+    },
+    {
+      name: "prefers resource metadata over OAuth metadata",
+      mockResolves: baseMetadata,
+      serverUrl: "https://example.com",
+      resourceMetadata: {
+        resource: "https://example.com",
+        scopes_supported: ["admin", "full"],
+      },
+      expected: "admin full",
+    },
+    {
+      name: "falls back to OAuth when resource has empty scopes",
+      mockResolves: baseMetadata,
+      serverUrl: "https://example.com",
+      resourceMetadata: {
+        resource: "https://example.com",
+        scopes_supported: [],
+      },
+      expected: "read write",
+    },
+    {
+      name: "normalizes URL with port and path",
+      mockResolves: baseMetadata,
+      serverUrl: "https://example.com:8080/some/path",
+      expected: "read write",
+      expectedCallUrl: "https://example.com:8080/",
+    },
+    {
+      name: "normalizes URL with trailing slash",
+      mockResolves: baseMetadata,
+      serverUrl: "https://example.com/",
+      expected: "read write",
+      expectedCallUrl: "https://example.com/",
+    },
+    {
+      name: "handles single scope",
+      mockResolves: { ...baseMetadata, scopes_supported: ["admin"] },
+      serverUrl: "https://example.com",
+      expected: "admin",
+    },
+    {
+      name: "prefers resource metadata even with fewer scopes",
+      mockResolves: {
+        ...baseMetadata,
+        scopes_supported: ["read", "write", "admin", "full"],
+      },
+      serverUrl: "https://example.com",
+      resourceMetadata: {
+        resource: "https://example.com",
+        scopes_supported: ["read"],
+      },
+      expected: "read",
+    },
+  ];
+
+  const undefinedCases = [
+    {
+      name: "returns undefined when OAuth discovery fails",
+      mockRejects: new Error("Discovery failed"),
+      serverUrl: "https://example.com",
+    },
+    {
+      name: "returns undefined when OAuth has no scopes",
+      mockResolves: { ...baseMetadata, scopes_supported: [] },
+      serverUrl: "https://example.com",
+    },
+    {
+      name: "returns undefined when scopes_supported missing",
+      mockResolves: (() => {
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        const { scopes_supported, ...rest } = baseMetadata;
+        return rest;
+      })(),
+      serverUrl: "https://example.com",
+    },
+    {
+      name: "returns undefined with resource metadata but OAuth fails",
+      mockRejects: new Error("No OAuth metadata"),
+      serverUrl: "https://example.com",
+      resourceMetadata: {
+        resource: "https://example.com",
+        scopes_supported: ["read", "write"],
+      },
+    },
+  ];
+
+  test.each(testCases)(
+    "$name",
+    async ({
+      mockResolves,
+      serverUrl,
+      resourceMetadata,
+      expected,
+      expectedCallUrl,
+    }) => {
+      mockDiscoverAuth.mockResolvedValue(mockResolves);
+
+      const result = await discoverScopes(serverUrl, resourceMetadata);
+
+      expect(result).toBe(expected);
+      if (expectedCallUrl) {
+        expect(mockDiscoverAuth).toHaveBeenCalledWith(new URL(expectedCallUrl));
+      }
+    },
+  );
+
+  test.each(undefinedCases)(
+    "$name",
+    async ({ mockResolves, mockRejects, serverUrl, resourceMetadata }) => {
+      if (mockRejects) {
+        mockDiscoverAuth.mockRejectedValue(mockRejects);
+      } else {
+        mockDiscoverAuth.mockResolvedValue(mockResolves);
+      }
+
+      const result = await discoverScopes(serverUrl, resourceMetadata);
+
+      expect(result).toBeUndefined();
+    },
+  );
+});

client/src/lib/auth.ts

@@ -6,10 +6,45 @@ import {
   OAuthTokensSchema,
   OAuthClientMetadata,
   OAuthMetadata,
+  OAuthProtectedResourceMetadata,
 } from "@modelcontextprotocol/sdk/shared/auth.js";
+import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";
 import { SESSION_KEYS, getServerSpecificKey } from "./constants";
 import { generateOAuthState } from "@/utils/oauthUtils";
 
+/**
+ * Discovers OAuth scopes from server metadata, with preference for resource metadata scopes
+ * @param serverUrl - The MCP server URL
+ * @param resourceMetadata - Optional resource metadata containing preferred scopes
+ * @returns Promise resolving to space-separated scope string or undefined
+ */
+export const discoverScopes = async (
+  serverUrl: string,
+  resourceMetadata?: OAuthProtectedResourceMetadata,
+): Promise<string | undefined> => {
+  try {
+    const metadata = await discoverAuthorizationServerMetadata(
+      new URL("/", serverUrl),
+    );
+
+    // Prefer resource metadata scopes, but fall back to OAuth metadata if empty
+    const resourceScopes = resourceMetadata?.scopes_supported;
+    const oauthScopes = metadata?.scopes_supported;
+
+    const scopesSupported =
+      resourceScopes && resourceScopes.length > 0
+        ? resourceScopes
+        : oauthScopes;
+
+    return scopesSupported && scopesSupported.length > 0
+      ? scopesSupported.join(" ")
+      : undefined;
+  } catch (error) {
+    console.debug("OAuth scope discovery failed:", error);
+    return undefined;
+  }
+};
+
 export const getClientInformationFromSessionStorage = async ({
   serverUrl,
   isPreregistered,
@@ -110,10 +145,17 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
   }
 
   saveClientInformation(clientInformation: OAuthClientInformation) {
+    // Remove client_secret before storing (not needed after initial OAuth flow)
+    const safeInfo = Object.fromEntries(
+      Object.entries(clientInformation).filter(
+        ([key]) => key !== "client_secret",
+      ),
+    ) as OAuthClientInformation;
+
     // Save the dynamically registered client information to session storage
     saveClientInformationToSessionStorage({
       serverUrl: this.serverUrl,
-      clientInformation,
+      clientInformation: safeInfo,
       isPreregistered: false,
     });
   }

client/src/lib/hooks/__tests__/useConnection.test.tsx

@@ -3,11 +3,16 @@ import { useConnection } from "../useConnection";
 import { z } from "zod";
 import { ClientRequest } from "@modelcontextprotocol/sdk/types.js";
 import { DEFAULT_INSPECTOR_CONFIG } from "../../constants";
-import { SSEClientTransportOptions } from "@modelcontextprotocol/sdk/client/sse.js";
+import {
+  SSEClientTransportOptions,
+  SseError,
+} from "@modelcontextprotocol/sdk/client/sse.js";
 import {
   ElicitResult,
   ElicitRequest,
 } from "@modelcontextprotocol/sdk/types.js";
+import { auth } from "@modelcontextprotocol/sdk/client/auth.js";
+import { discoverScopes } from "../../auth";
 
 // Mock fetch
 global.fetch = jest.fn().mockResolvedValue({
@@ -53,14 +58,27 @@ jest.mock("@modelcontextprotocol/sdk/client/index.js", () => ({
   Client: jest.fn().mockImplementation(() => mockClient),
 }));
 
-jest.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({
+jest.mock("@modelcontextprotocol/sdk/client/sse.js", () => {
-  SSEClientTransport: jest.fn((url, options) => {
+  // Minimal mock class that supports instanceof checks
-    mockSSETransport.url = url;
+  class SseError extends Error {
-    mockSSETransport.options = options;
+    code: number;
-    return mockSSETransport;
+    event: ErrorEvent;
-  }),
+    constructor(code: number, message: string, event: ErrorEvent) {
-  SseError: jest.fn(),
+      super(message);
-}));
+      this.code = code;
+      this.event = event;
+    }
+  }
+
+  return {
+    SSEClientTransport: jest.fn((url, options) => {
+      mockSSETransport.url = url;
+      mockSSETransport.options = options;
+      return mockSSETransport;
+    }),
+    SseError,
+  };
+});
 
 jest.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
   StreamableHTTPClientTransport: jest.fn((url, options) => {
@@ -85,11 +103,18 @@ jest.mock("@/lib/hooks/useToast", () => ({
 jest.mock("../../auth", () => ({
   InspectorOAuthClientProvider: jest.fn().mockImplementation(() => ({
     tokens: jest.fn().mockResolvedValue({ access_token: "mock-token" }),
+    redirectUrl: "http://localhost:3000/oauth/callback",
   })),
   clearClientInformationFromSessionStorage: jest.fn(),
   saveClientInformationToSessionStorage: jest.fn(),
+  discoverScopes: jest.fn(),
 }));
 
+const mockAuth = auth as jest.MockedFunction<typeof auth>;
+const mockDiscoverScopes = discoverScopes as jest.MockedFunction<
+  typeof discoverScopes
+>;
+
 describe("useConnection", () => {
   const defaultProps = {
     transportType: "sse" as const,
@@ -714,4 +739,142 @@ describe("useConnection", () => {
       ).toHaveProperty("X-MCP-Proxy-Auth", "Bearer test-proxy-token");
     });
   });
+
+  describe("OAuth Error Handling with Scope Discovery", () => {
+    beforeEach(() => {
+      jest.clearAllMocks();
+      mockAuth.mockResolvedValue("AUTHORIZED");
+      mockDiscoverScopes.mockResolvedValue(undefined);
+    });
+
+    const setup401Error = () => {
+      const mockErrorEvent = new ErrorEvent("error", {
+        message: "Mock error event",
+      });
+      mockClient.connect.mockRejectedValueOnce(
+        new SseError(401, "Unauthorized", mockErrorEvent),
+      );
+    };
+
+    const attemptConnection = async (props = defaultProps) => {
+      const { result } = renderHook(() => useConnection(props));
+      await act(async () => {
+        try {
+          await result.current.connect();
+        } catch {
+          // Expected error from auth handling
+        }
+      });
+    };
+
+    const testCases = [
+      [
+        "discovers and includes scopes in auth call",
+        {
+          discoveredScope: "read write admin",
+          oauthScope: undefined,
+          expectScopeCall: true,
+          expectedAuthScope: "read write admin",
+          authResult: "AUTHORIZED",
+        },
+      ],
+      [
+        "handles scope discovery failure gracefully",
+        {
+          discoveredScope: undefined,
+          oauthScope: undefined,
+          expectScopeCall: true,
+          expectedAuthScope: undefined,
+          authResult: "AUTHORIZED",
+        },
+      ],
+      [
+        "uses manual oauthScope override instead of discovered scopes",
+        {
+          discoveredScope: "discovered:scope",
+          oauthScope: "manual:scope",
+          expectScopeCall: false,
+          expectedAuthScope: "manual:scope",
+          authResult: "AUTHORIZED",
+        },
+      ],
+      [
+        "triggers scope discovery when oauthScope is whitespace",
+        {
+          discoveredScope: "discovered:scope",
+          oauthScope: "   ",
+          expectScopeCall: true,
+          expectedAuthScope: "discovered:scope",
+          authResult: "AUTHORIZED",
+        },
+      ],
+      [
+        "handles auth failure after scope discovery",
+        {
+          discoveredScope: "read write",
+          oauthScope: undefined,
+          expectScopeCall: true,
+          expectedAuthScope: "read write",
+          authResult: "UNAUTHORIZED",
+        },
+      ],
+    ] as const;
+
+    test.each(testCases)(
+      "should %s",
+      async (
+        _,
+        {
+          discoveredScope,
+          oauthScope,
+          expectScopeCall,
+          expectedAuthScope,
+          authResult = "AUTHORIZED",
+        },
+      ) => {
+        mockDiscoverScopes.mockResolvedValue(discoveredScope);
+        mockAuth.mockResolvedValue(authResult as never);
+        setup401Error();
+
+        const props =
+          oauthScope !== undefined
+            ? { ...defaultProps, oauthScope }
+            : defaultProps;
+        await attemptConnection(props);
+
+        if (expectScopeCall) {
+          expect(mockDiscoverScopes).toHaveBeenCalledWith(
+            defaultProps.sseUrl,
+            undefined,
+          );
+        } else {
+          expect(mockDiscoverScopes).not.toHaveBeenCalled();
+        }
+
+        expect(mockAuth).toHaveBeenCalledWith(expect.any(Object), {
+          serverUrl: defaultProps.sseUrl,
+          scope: expectedAuthScope,
+        });
+      },
+    );
+
+    it("should handle slow scope discovery gracefully", async () => {
+      mockDiscoverScopes.mockImplementation(
+        () =>
+          new Promise((resolve) => setTimeout(() => resolve(undefined), 100)),
+      );
+
+      setup401Error();
+      await attemptConnection();
+
+      expect(mockDiscoverScopes).toHaveBeenCalledWith(
+        defaultProps.sseUrl,
+        undefined,
+      );
+      expect(mockAuth).toHaveBeenCalledWith(expect.any(Object), {
+        serverUrl: defaultProps.sseUrl,
+        scope: undefined,
+      });
+    });
+  });
 });

client/src/lib/hooks/useConnection.ts

@@ -37,11 +37,15 @@ import { useToast } from "@/lib/hooks/useToast";
 import { z } from "zod";
 import { ConnectionStatus } from "../constants";
 import { Notification, StdErrNotificationSchema } from "../notificationTypes";
-import { auth } from "@modelcontextprotocol/sdk/client/auth.js";
+import {
+  auth,
+  discoverOAuthProtectedResourceMetadata,
+} from "@modelcontextprotocol/sdk/client/auth.js";
 import {
   clearClientInformationFromSessionStorage,
   InspectorOAuthClientProvider,
   saveClientInformationToSessionStorage,
+  discoverScopes,
 } from "../auth";
 import packageJson from "../../../package.json";
 import {
@@ -317,9 +321,23 @@ export function useConnection({
     if (is401Error(error)) {
       const serverAuthProvider = new InspectorOAuthClientProvider(sseUrl);
 
+      let scope = oauthScope?.trim();
+      if (!scope) {
+        // Only discover resource metadata when we need to discover scopes
+        let resourceMetadata;
+        try {
+          resourceMetadata = await discoverOAuthProtectedResourceMetadata(
+            new URL("/", sseUrl),
+          );
+        } catch {
+          // Resource metadata is optional, continue without it
+        }
+        scope = await discoverScopes(sseUrl, resourceMetadata);
+      }
+
       const result = await auth(serverAuthProvider, {
         serverUrl: sseUrl,
-        scope: oauthScope,
+        scope,
       });
       return result === "AUTHORIZED";
     }

client/src/lib/oauth-state-machine.ts

@@ -1,5 +1,5 @@
 import { OAuthStep, AuthDebuggerState } from "./auth-types";
-import { DebugInspectorOAuthClientProvider } from "./auth";
+import { DebugInspectorOAuthClientProvider, discoverScopes } from "./auth";
 import {
   discoverAuthorizationServerMetadata,
   registerClient,
@@ -113,10 +113,10 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
       const metadata = context.state.oauthMetadata!;
       const clientInformation = context.state.oauthClientInfo!;
 
-      let scope: string | undefined = undefined;
+      const scope = await discoverScopes(
-      if (metadata.scopes_supported) {
+        context.serverUrl,
-        scope = metadata.scopes_supported.join(" ");
+        context.state.resourceMetadata ?? undefined,
-      }
+      );
 
       const { authorizationUrl, codeVerifier } = await startAuthorization(
         context.serverUrl,