fix: MCP server OAuth form quick wins (read-only redirect URI, drop password grant) UI part

src/components/mcp-servers/MCPServerForm.test.tsx

@@ -793,111 +793,16 @@ describe("MCPServerForm", () => {
     });
   });
 
-  describe("OAuth Password Grant Validation", () => {
-    // Helper: open advanced settings, switch to OAuth auth, select password grant
-    const renderWithOAuthPassword = // pragma: allowlist secret
-      async () => {
-        const user = userEvent.setup();
-        renderWithRouter(<MCPServerForm {...defaultProps} />);
-        await user.click(screen.getByRole("button", { name: /Advanced settings/i }));
-        await user.click(screen.getByRole("radio", { name: /OAuth 2\.0/i }));
-        // Grant type defaults to client_credentials; switch to password
-        await user.click(screen.getByRole("combobox", { name: /Grant type/i }));
-        await user.click(screen.getByRole("option", { name: /Resource owner password/i }));
-        return user;
-      };
-
-    it("shows username and password fields when password grant is selected", async () => {
-      await renderWithOAuthPassword();
-      expect(screen.getByLabelText(/Username/i)).toBeInTheDocument();
-      expect(screen.getByLabelText(/^Password/i)).toBeInTheDocument();
-    });
-
-    it("disables the submit button when username is empty", async () => {
-      await renderWithOAuthPassword();
-      fireEvent.change(screen.getByLabelText(/^Name/i), { target: { value: "Test Server" } });
-      fireEvent.change(screen.getByLabelText(/^URL/i), {
-        target: { value: "http://localhost:3000" },
-      });
-      // Leave username empty, fill password
-      fireEvent.change(screen.getByLabelText(/^Password/i), { target: { value: "secret" } });
-      expect(screen.getByRole("button", { name: /Connect server/i })).toBeDisabled();
-    });
-
-    it("disables the submit button when password is empty", async () => {
-      await renderWithOAuthPassword();
-      fireEvent.change(screen.getByLabelText(/^Name/i), { target: { value: "Test Server" } });
-      fireEvent.change(screen.getByLabelText(/^URL/i), {
-        target: { value: "http://localhost:3000" },
-      });
-      fireEvent.change(screen.getByLabelText(/^Username/i), {
-        target: { value: "service-account" },
-      });
-      // Leave password empty
-      expect(screen.getByRole("button", { name: /Connect server/i })).toBeDisabled();
-    });
-
-    it("enables the submit button when both username and password are provided", async () => {
-      await renderWithOAuthPassword();
-      fireEvent.change(screen.getByLabelText(/^Name/i), { target: { value: "Test Server" } });
-      fireEvent.change(screen.getByLabelText(/^URL/i), {
-        target: { value: "http://localhost:3000" },
-      });
-      fireEvent.change(screen.getByLabelText(/^Username/i), {
-        target: { value: "service-account" },
-      });
-      fireEvent.change(screen.getByLabelText(/^Password/i), { target: { value: "secret" } });
-      expect(screen.getByRole("button", { name: /Connect server/i })).not.toBeDisabled();
-    });
-
-    it("marks username input as aria-invalid when username error is present", async () => {
-      await renderWithOAuthPassword();
-      fireEvent.change(screen.getByLabelText(/^Name/i), { target: { value: "Test Server" } });
-      fireEvent.change(screen.getByLabelText(/^URL/i), {
-        target: { value: "http://localhost:3000" },
-      });
-      // Only fill password, leave username empty
-      fireEvent.change(screen.getByLabelText(/^Password/i), { target: { value: "secret" } });
-
-      // Expose the field without a value and attempt form submission
-      const form = document.querySelector("form")!;
-      fireEvent.submit(form);
-
-      await waitFor(() => {
-        expect(screen.getByLabelText(/^Username/i)).toHaveAttribute("aria-invalid", "true");
-      });
-    });
-
-    it("marks password input as aria-invalid when password error is present", async () => {
-      await renderWithOAuthPassword();
-      fireEvent.change(screen.getByLabelText(/^Name/i), { target: { value: "Test Server" } });
-      fireEvent.change(screen.getByLabelText(/^URL/i), {
-        target: { value: "http://localhost:3000" },
-      });
-      fireEvent.change(screen.getByLabelText(/^Username/i), {
-        target: { value: "service-account" },
-      });
-      // Leave password empty, submit the form
-      const form = document.querySelector("form")!;
-      fireEvent.submit(form);
-
-      await waitFor(() => {
-        expect(screen.getByLabelText(/^Password/i)).toHaveAttribute("aria-invalid", "true");
-      });
-    });
-
-    it("shows inline error messages for both fields when both are empty", async () => {
-      await renderWithOAuthPassword();
-      fireEvent.change(screen.getByLabelText(/^Name/i), { target: { value: "Test Server" } });
-      fireEvent.change(screen.getByLabelText(/^URL/i), {
-        target: { value: "http://localhost:3000" },
-      });
-      fireEvent.submit(document.querySelector("form")!);
-
-      await waitFor(() => {
-        expect(screen.getByText("Username is required for password grant")).toBeInTheDocument();
-        expect(screen.getByText("Password is required for password grant")).toBeInTheDocument();
-      });
+  describe("OAuth Password Grant", () => {
+    it("does not offer the deprecated password grant for new servers", async () => {
+      const user = userEvent.setup();
+      renderWithRouter(<MCPServerForm {...defaultProps} />);
+      await user.click(screen.getByRole("button", { name: /Advanced settings/i }));
+      await user.click(screen.getByRole("radio", { name: /OAuth 2\.0/i }));
+      await user.click(screen.getByRole("combobox", { name: /Grant type/i }));
+      expect(
+        screen.queryByRole("option", { name: /Resource owner password/i }),
+      ).not.toBeInTheDocument();
     });
 
     it("does not show password-grant errors when a different OAuth grant type is selected", async () => {

src/components/mcp-servers/MCPServerForm.tsx

@@ -1,4 +1,4 @@
-import { useState } from "react";
+import { useCallback, useState } from "react";
 import { ChevronDown, CircleAlert } from "lucide-react";
 import { Button } from "@/components/ui/button";
 import { InlineNotification } from "@/components/ui/inline-notification";
@@ -94,6 +94,13 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ
     setQueryParamApiKey,
   } = useMCPServerForm(serverId);
 
+  const handleRedirectUriChange = useCallback(
+    (uri: string) => {
+      setOAuthRedirectUri(uri);
+    },
+    [setOAuthRedirectUri],
+  );
+
   const handleCancel = () => {
     setCreatedGateway(null);
     onToggle();
@@ -334,7 +341,7 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ
                   onOAuthTokenUrlChange={setOAuthTokenUrl}
                   onOAuthGrantTypeChange={setOAuthGrantType}
                   onOAuthIssuerUrlChange={setOAuthIssuerUrl}
-                  onOAuthRedirectUriChange={setOAuthRedirectUri}
+                  onOAuthRedirectUriChange={handleRedirectUriChange}
                   onOAuthAuthorizationUrlChange={setOAuthAuthorizationUrl}
                   onOAuthScopesChange={setOAuthScopes}
                   onOAuthStoreTokensChange={setOAuthStoreTokens}

src/components/mcp-servers/OAuth2Auth.test.tsx

@@ -1,7 +1,10 @@
-import { describe, it, expect, vi } from "vitest";
-import { render, screen, fireEvent } from "@testing-library/react";
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { render, screen, fireEvent, act } from "@testing-library/react";
+import { I18nProvider } from "@/i18n";
 import { OAuth2Auth } from "./OAuth2Auth";
 
+const renderWithI18n = (ui: React.ReactElement) => render(<I18nProvider>{ui}</I18nProvider>);
+
 describe("OAuth2Auth", () => {
   const defaultProps = {
     grantType: "client_credentials",
@@ -31,7 +34,7 @@ describe("OAuth2Auth", () => {
   };
 
   it("should render client_credentials fields by default", () => {
-    render(<OAuth2Auth {...defaultProps} />);
+    renderWithI18n(<OAuth2Auth {...defaultProps} />);
 
     expect(screen.getByLabelText(/Grant type/i)).toBeInTheDocument();
     expect(screen.getByLabelText(/Issuer URL/i)).toBeInTheDocument();
@@ -52,7 +55,7 @@ describe("OAuth2Auth", () => {
   });
 
   it("should render authorization_code fields", () => {
-    render(<OAuth2Auth {...defaultProps} grantType="authorization_code" />);
+    renderWithI18n(<OAuth2Auth {...defaultProps} grantType="authorization_code" />);
 
     expect(screen.getByLabelText(/Redirect URI/i)).toBeInTheDocument();
     expect(screen.getByLabelText(/Authorization URL/i)).toBeInTheDocument();
@@ -62,7 +65,7 @@ describe("OAuth2Auth", () => {
   });
 
   it("should render password fields", () => {
-    render(<OAuth2Auth {...defaultProps} grantType="password" />);
+    renderWithI18n(<OAuth2Auth {...defaultProps} grantType="password" />);
 
     expect(screen.getByLabelText(/Username/i)).toBeInTheDocument();
     expect(screen.getByLabelText(/Password/i)).toBeInTheDocument();
@@ -72,7 +75,7 @@ describe("OAuth2Auth", () => {
   });
 
   it("should display errors for username and password in password grant", () => {
-    render(
+    renderWithI18n(
       <OAuth2Auth
         {...defaultProps}
         grantType="password"
@@ -91,7 +94,7 @@ describe("OAuth2Auth", () => {
     const onTokenUrlChange = vi.fn();
     const onScopesChange = vi.fn();
 
-    render(
+    renderWithI18n(
       <OAuth2Auth
         {...defaultProps}
         onIssuerUrlChange={onIssuerUrlChange}
@@ -128,7 +131,7 @@ describe("OAuth2Auth", () => {
     const onUsernameChange = vi.fn();
     const onPasswordChange = vi.fn();
 
-    render(
+    renderWithI18n(
       <OAuth2Auth
         {...defaultProps}
         grantType="password"
@@ -144,35 +147,82 @@ describe("OAuth2Auth", () => {
     expect(onPasswordChange).toHaveBeenCalledWith("test-pass");
   });
 
-  it("should trigger callbacks for authorization_code fields", () => {
-    const onRedirectUriChange = vi.fn();
+  it("shows a read-only derived redirect URI, lifts it into form state, and triggers the authorization URL callback", () => {
     const onAuthorizationUrlChange = vi.fn();
+    const onRedirectUriChange = vi.fn();
 
-    render(
+    renderWithI18n(
       <OAuth2Auth
         {...defaultProps}
         grantType="authorization_code"
-        onRedirectUriChange={onRedirectUriChange}
         onAuthorizationUrlChange={onAuthorizationUrlChange}
+        onRedirectUriChange={onRedirectUriChange}
       />,
     );
 
-    fireEvent.change(screen.getByLabelText(/Redirect URI/i), {
-      target: { value: "https://redirect.com" },
-    });
-    expect(onRedirectUriChange).toHaveBeenCalledWith("https://redirect.com");
+    const redirect = screen.getByLabelText(/Redirect URI/i);
+    expect(redirect).toHaveAttribute("readonly");
+    expect(redirect).toHaveValue(`${window.location.origin}/oauth/callback`);
+    expect(screen.getByRole("button", { name: "Copy to clipboard" })).toBeInTheDocument();
+    expect(onRedirectUriChange).toHaveBeenCalledWith(`${window.location.origin}/oauth/callback`);
 
     fireEvent.change(screen.getByLabelText(/Authorization URL/i), {
       target: { value: "https://auth.com/authorize" },
     });
     expect(onAuthorizationUrlChange).toHaveBeenCalledWith("https://auth.com/authorize");
   });
 
+  it("displays a stored redirect URI verbatim without overwriting it", () => {
+    const onRedirectUriChange = vi.fn();
+
+    renderWithI18n(
+      <OAuth2Auth
+        {...defaultProps}
+        grantType="authorization_code"
+        redirectUri="https://public.example.com/oauth/callback"
+        onRedirectUriChange={onRedirectUriChange}
+      />,
+    );
+
+    expect(screen.getByLabelText(/Redirect URI/i)).toHaveValue(
+      "https://public.example.com/oauth/callback",
+    );
+    expect(onRedirectUriChange).not.toHaveBeenCalled();
+  });
+
+  it("does not set a redirect URI for non-authorization_code grants", () => {
+    const onRedirectUriChange = vi.fn();
+
+    renderWithI18n(
+      <OAuth2Auth
+        {...defaultProps}
+        grantType="client_credentials"
+        onRedirectUriChange={onRedirectUriChange}
+      />,
+    );
+
+    expect(onRedirectUriChange).not.toHaveBeenCalled();
+  });
+
+  it("only offers the password grant option when already selected (legacy)", () => {
+    const { rerender } = renderWithI18n(
+      <OAuth2Auth {...defaultProps} grantType="client_credentials" />,
+    );
+    expect(screen.queryByText(/Password grant is deprecated/i)).not.toBeInTheDocument();
+
+    rerender(
+      <I18nProvider>
+        <OAuth2Auth {...defaultProps} grantType="password" />
+      </I18nProvider>,
+    );
+    expect(screen.getByText(/Password grant is deprecated/i)).toBeInTheDocument();
+  });
+
   it("should trigger checkbox callback functions", () => {
     const onStoreTokensChange = vi.fn();
     const onAutoRefreshChange = vi.fn();
 
-    render(
+    renderWithI18n(
       <OAuth2Auth
         {...defaultProps}
         onStoreTokensChange={onStoreTokensChange}
@@ -186,4 +236,84 @@ describe("OAuth2Auth", () => {
     fireEvent.click(screen.getByLabelText(/Automatically refresh expired tokens/i));
     expect(onAutoRefreshChange).toHaveBeenCalled();
   });
+
+  describe("copy button interaction", () => {
+    beforeEach(() => {
+      Object.assign(navigator, {
+        clipboard: { writeText: vi.fn().mockResolvedValue(undefined) },
+      });
+    });
+
+    it("copies the redirect URI to clipboard when the copy button is clicked", async () => {
+      renderWithI18n(<OAuth2Auth {...defaultProps} grantType="authorization_code" />);
+
+      const copyButton = screen.getByRole("button", { name: /Copy to clipboard/i });
+      await act(async () => {
+        fireEvent.click(copyButton);
+      });
+
+      expect(navigator.clipboard.writeText).toHaveBeenCalledWith(
+        `${window.location.origin}/oauth/callback`,
+      );
+    });
+
+    it("shows a check icon immediately after clicking copy and reverts after 2 s", async () => {
+      vi.useFakeTimers();
+
+      renderWithI18n(<OAuth2Auth {...defaultProps} grantType="authorization_code" />);
+
+      const copyButton = screen.getByRole("button", { name: /Copy to clipboard/i });
+      await act(async () => {
+        fireEvent.click(copyButton);
+      });
+
+      // The button is still present (aria-label unchanged; icon swap is visual-only)
+      expect(copyButton).toBeInTheDocument();
+
+      await act(async () => {
+        vi.advanceTimersByTime(2000);
+      });
+
+      vi.useRealTimers();
+    });
+  });
+
+  describe("localhost warning", () => {
+    it("shows a localhost warning when the derived redirect URI points to localhost", () => {
+      // jsdom sets window.location.origin to 'http://localhost'
+      renderWithI18n(<OAuth2Auth {...defaultProps} grantType="authorization_code" />);
+
+      expect(
+        screen.getByText(/Redirect URIs derived from localhost will not work/i),
+      ).toBeInTheDocument();
+    });
+
+    it("does not show the localhost warning when a non-localhost stored redirect URI is used", () => {
+      renderWithI18n(
+        <OAuth2Auth
+          {...defaultProps}
+          grantType="authorization_code"
+          redirectUri="https://public.example.com/oauth/callback"
+        />,
+      );
+
+      expect(
+        screen.queryByText(/Redirect URIs derived from localhost will not work/i),
+      ).not.toBeInTheDocument();
+    });
+
+    it("shows the localhost warning when a stored redirect URI points to 127.0.0.1", () => {
+      renderWithI18n(
+        <OAuth2Auth
+          {...defaultProps}
+          grantType="authorization_code"
+          redirectUri="http://127.0.0.1:8080/oauth/callback"
+        />,
+      );
+
+      expect(
+        screen.getByText(/Redirect URIs derived from localhost will not work/i),
+      ).toBeInTheDocument();
+    });
+  });
 });