diff --git a/src/components/mcp-servers/MCPServerForm.test.tsx b/src/components/mcp-servers/MCPServerForm.test.tsx index fd80af2..9db9ed0 100644 --- a/src/components/mcp-servers/MCPServerForm.test.tsx +++ b/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(); - 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(); + 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 () => { diff --git a/src/components/mcp-servers/MCPServerForm.tsx b/src/components/mcp-servers/MCPServerForm.tsx index 9d63516..ad24e46 100644 --- a/src/components/mcp-servers/MCPServerForm.tsx +++ b/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} diff --git a/src/components/mcp-servers/OAuth2Auth.test.tsx b/src/components/mcp-servers/OAuth2Auth.test.tsx index 16e6727..b99fde4 100644 --- a/src/components/mcp-servers/OAuth2Auth.test.tsx +++ b/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({ui}); + describe("OAuth2Auth", () => { const defaultProps = { grantType: "client_credentials", @@ -31,7 +34,7 @@ describe("OAuth2Auth", () => { }; it("should render client_credentials fields by default", () => { - render(); + renderWithI18n(); 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(); + renderWithI18n(); 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(); + renderWithI18n(); 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( { const onTokenUrlChange = vi.fn(); const onScopesChange = vi.fn(); - render( + renderWithI18n( { const onUsernameChange = vi.fn(); const onPasswordChange = vi.fn(); - render( + renderWithI18n( { 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( , ); - 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" }, @@ -168,11 +172,57 @@ describe("OAuth2Auth", () => { expect(onAuthorizationUrlChange).toHaveBeenCalledWith("https://auth.com/authorize"); }); + it("displays a stored redirect URI verbatim without overwriting it", () => { + const onRedirectUriChange = vi.fn(); + + renderWithI18n( + , + ); + + 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( + , + ); + + expect(onRedirectUriChange).not.toHaveBeenCalled(); + }); + + it("only offers the password grant option when already selected (legacy)", () => { + const { rerender } = renderWithI18n( + , + ); + expect(screen.queryByText(/Password grant is deprecated/i)).not.toBeInTheDocument(); + + rerender( + + + , + ); + 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( { 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(); + + 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(); + + 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(); + + 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( + , + ); + + 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( + , + ); + + expect( + screen.getByText(/Redirect URIs derived from localhost will not work/i), + ).toBeInTheDocument(); + }); + }); }); diff --git a/src/components/mcp-servers/OAuth2Auth.tsx b/src/components/mcp-servers/OAuth2Auth.tsx index bd0fc8a..3c5c920 100644 --- a/src/components/mcp-servers/OAuth2Auth.tsx +++ b/src/components/mcp-servers/OAuth2Auth.tsx @@ -1,6 +1,9 @@ import { Input } from "@/components/ui/input"; import { Textarea } from "@/components/ui/textarea"; import { Checkbox } from "@/components/ui/checkbox"; +import { useEffect, useState } from "react"; +import { Check, Copy } from "lucide-react"; +import { Button } from "@/components/ui/button"; import { Select, SelectContent, @@ -8,6 +11,7 @@ import { SelectTrigger, SelectValue, } from "@/components/ui/select"; +import { useIntl } from "react-intl"; interface OAuth2AuthProps { grantType: string; @@ -64,6 +68,27 @@ export function OAuth2Auth({ onPasswordChange, errors, }: OAuth2AuthProps) { + const intl = useIntl(); + const derivedRedirectUri = `${window.location.origin}/oauth/callback`; + const displayRedirectUri = redirectUri || derivedRedirectUri; + const isLocalRedirect = /^https?:\/\/(localhost|127\.0\.0\.1|\[::1\])(:|\/|$)/i.test( + displayRedirectUri, + ); + const [copied, setCopied] = useState(false); + + // The displayed URI is what the OAuth app is registered with, so it has to be the value we + // store and send to the IdP — a display-only derivation submits no redirect_uri at all. + useEffect(() => { + if (grantType === "authorization_code" && !redirectUri) { + onRedirectUriChange(derivedRedirectUri); + } + }, [grantType, redirectUri, derivedRedirectUri, onRedirectUriChange]); + + const handleCopyRedirect = () => { + void navigator.clipboard?.writeText(displayRedirectUri); + setCopied(true); + window.setTimeout(() => setCopied(false), 2000); + }; return (
@@ -71,46 +96,60 @@ export function OAuth2Auth({ htmlFor="oauth-grant-type" className="inline-flex items-center gap-0.5 text-sm font-medium text-neutral-900 dark:text-neutral-100" > - Grant type* - (required) + {intl.formatMessage({ id: "mcpServer.oauth2.grantType.label" })} + * + {intl.formatMessage({ id: "common.required" })}
+ {grantType === "password" && ( +

+ {intl.formatMessage({ id: "mcpServer.oauth2.passwordDeprecated" })} +

+ )} +
onIssuerUrlChange(e.target.value)} - placeholder="e.g. https://auth.example.com" + placeholder={intl.formatMessage({ id: "mcpServer.oauth2.issuerUrl.placeholder" })} className="rounded-md border-neutral-300 px-4 text-sm text-neutral-900 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 placeholder:text-neutral-400 dark:border-neutral-700 dark:text-neutral-100 dark:placeholder:text-neutral-500" />

- { - "Authorization server's base URL for endpoint discovery and Dynamic Client Registration (DCR)" - } + {intl.formatMessage({ id: "mcpServer.oauth2.issuerUrl.hint" })}

@@ -118,22 +157,36 @@ export function OAuth2Auth({
- onRedirectUriChange(e.target.value)} - placeholder="e.g. https://gateway.example.com/oauth/callback" - className="rounded-md border-neutral-300 px-4 text-sm text-neutral-900 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 placeholder:text-neutral-400 dark:border-neutral-700 dark:text-neutral-100 dark:placeholder:text-neutral-500" - /> +
+ + +

- {"Copy URI into the OAuth application's allowed redirect URI"} + {intl.formatMessage({ id: "mcpServer.oauth2.redirectUri.hint" })}

+ {isLocalRedirect && ( +

+ {intl.formatMessage({ id: "mcpServer.oauth2.redirectUri.localWarning" })} +

+ )}
)} @@ -144,15 +197,16 @@ export function OAuth2Auth({ htmlFor="oauth-username" className="inline-flex items-center gap-0.5 text-sm font-medium text-neutral-900 dark:text-neutral-100" > - Username* - (required) + {intl.formatMessage({ id: "mcpServer.oauth2.username.label" })} + * + {intl.formatMessage({ id: "common.required" })} onUsernameChange(e.target.value)} - placeholder="e.g. service-account" + placeholder={intl.formatMessage({ id: "mcpServer.oauth2.username.placeholder" })} aria-invalid={!!errors?.username} aria-describedby={errors?.username ? "oauth-username-error" : undefined} className="rounded-md border-neutral-300 px-4 text-sm text-neutral-900 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 placeholder:text-neutral-400 dark:border-neutral-700 dark:text-neutral-100 dark:placeholder:text-neutral-500" @@ -168,8 +222,9 @@ export function OAuth2Auth({ htmlFor="oauth-password" className="inline-flex items-center gap-0.5 text-sm font-medium text-neutral-900 dark:text-neutral-100" > - Password* - (required) + {intl.formatMessage({ id: "mcpServer.oauth2.password.label" })} + * + {intl.formatMessage({ id: "common.required" })} - Client ID + {intl.formatMessage({ id: "mcpServer.oauth2.clientId.label" })} onClientIdChange(e.target.value)} - placeholder="e.g. 8f3a2c1d-4b5e-4f6a-9c8d-1e2f3a4b5c6" + placeholder={intl.formatMessage({ id: "mcpServer.oauth2.clientId.placeholder" })} className="rounded-md border-neutral-300 px-4 text-sm text-neutral-900 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 placeholder:text-neutral-400 dark:border-neutral-700 dark:text-neutral-100 dark:placeholder:text-neutral-500" />

- Not required for servers that support Dynamic Client Registration (DCR) + {intl.formatMessage({ id: "mcpServer.oauth2.clientId.hint" })}

@@ -215,18 +270,18 @@ export function OAuth2Auth({ htmlFor="oauth-client-secret" className="text-sm font-medium text-neutral-900 dark:text-neutral-100" > - Client Secret + {intl.formatMessage({ id: "mcpServer.oauth2.clientSecret.label" })} onClientSecretChange(e.target.value)} - placeholder="e.g. a1b2c3d4e5f6" + placeholder={intl.formatMessage({ id: "mcpServer.oauth2.clientSecret.placeholder" })} className="rounded-md border-neutral-300 px-4 text-sm text-neutral-900 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 placeholder:text-neutral-400 dark:border-neutral-700 dark:text-neutral-100 dark:placeholder:text-neutral-500" />

- Not required for servers that support Dynamic Client Registration (DCR) + {intl.formatMessage({ id: "mcpServer.oauth2.clientSecret.hint" })}

@@ -235,19 +290,20 @@ export function OAuth2Auth({ htmlFor="oauth-token-url" className="inline-flex items-center gap-0.5 text-sm font-medium text-neutral-900 dark:text-neutral-100" > - Token URL* - (required) + {intl.formatMessage({ id: "mcpServer.oauth2.tokenUrl.label" })} + * + {intl.formatMessage({ id: "common.required" })} onTokenUrlChange(e.target.value)} - placeholder="e.g. https://oauth.example.com/token" + placeholder={intl.formatMessage({ id: "mcpServer.oauth2.tokenUrl.placeholder" })} className="rounded-md border-neutral-300 px-4 text-sm text-neutral-900 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 placeholder:text-neutral-400 dark:border-neutral-700 dark:text-neutral-100 dark:placeholder:text-neutral-500" />

- Exchanges authorization codes or credentials for access tokens + {intl.formatMessage({ id: "mcpServer.oauth2.tokenUrl.hint" })}

@@ -257,19 +313,22 @@ export function OAuth2Auth({ htmlFor="oauth-authorization-url" className="inline-flex items-center gap-0.5 text-sm font-medium text-neutral-900 dark:text-neutral-100" > - Authorization URL* - (required) + {intl.formatMessage({ id: "mcpServer.oauth2.authorizationUrl.label" })} + * + {intl.formatMessage({ id: "common.required" })} onAuthorizationUrlChange(e.target.value)} - placeholder="e.g. https://oauth.example.com/authorize" + placeholder={intl.formatMessage({ + id: "mcpServer.oauth2.authorizationUrl.placeholder", + })} className="rounded-md border-neutral-300 px-4 text-sm text-neutral-900 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 placeholder:text-neutral-400 dark:border-neutral-700 dark:text-neutral-100 dark:placeholder:text-neutral-500" />

- Where users are redirected to log in and grant access + {intl.formatMessage({ id: "mcpServer.oauth2.authorizationUrl.hint" })}

)} @@ -279,23 +338,23 @@ export function OAuth2Auth({ htmlFor="oauth-scopes" className="text-sm font-medium text-neutral-900 dark:text-neutral-100" > - Scopes + {intl.formatMessage({ id: "mcpServer.oauth2.scopes.label" })}

- Space-separated list of OAuth scopes + {intl.formatMessage({ id: "mcpServer.oauth2.scopes.hint" })}