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 a4141a5..c8e2b7a 100644 --- a/src/components/mcp-servers/MCPServerForm.tsx +++ b/src/components/mcp-servers/MCPServerForm.tsx @@ -1,4 +1,4 @@ -import { useState, type ReactNode } from "react"; +import { useCallback, useState, type ReactNode } from "react"; import { useIntl } from "react-intl"; import { ChevronDown, CircleAlert } from "lucide-react"; import { Button } from "@/components/ui/button"; @@ -96,6 +96,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(); @@ -347,7 +354,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 cc83de6..9674a51 100644 --- a/src/components/mcp-servers/OAuth2Auth.test.tsx +++ b/src/components/mcp-servers/OAuth2Auth.test.tsx @@ -1,5 +1,5 @@ -import { describe, it, expect, vi } from "vitest"; -import { screen, fireEvent } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { screen, fireEvent, act } from "@testing-library/react"; import { renderWithProviders as render } from "@/test/test-utils"; import { OAuth2Auth } from "./OAuth2Auth"; @@ -145,23 +145,24 @@ 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( , ); - 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" }, @@ -169,6 +170,46 @@ describe("OAuth2Auth", () => { expect(onAuthorizationUrlChange).toHaveBeenCalledWith("https://auth.com/authorize"); }); + it("displays a stored redirect URI verbatim without overwriting it", () => { + const onRedirectUriChange = vi.fn(); + + render( + , + ); + + 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(); + + render( + , + ); + + expect(onRedirectUriChange).not.toHaveBeenCalled(); + }); + + it("only offers the password grant option when already selected (legacy)", () => { + const { rerender } = render(); + 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(); @@ -187,4 +228,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 () => { + render(); + + 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(); + + render(); + + 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' + render(); + + 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", () => { + render( + , + ); + + 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", () => { + render( + , + ); + + 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 60e83a6..6e2f591 100644 --- a/src/components/mcp-servers/OAuth2Auth.tsx +++ b/src/components/mcp-servers/OAuth2Auth.tsx @@ -2,6 +2,9 @@ import { useIntl } from "react-intl"; 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, @@ -66,7 +69,26 @@ export function OAuth2Auth({ 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 (
@@ -94,13 +116,21 @@ export function OAuth2Auth({ {intl.formatMessage({ id: "mcpServer.auth.oauth.grantType.clientCredentials" })} - - {intl.formatMessage({ id: "mcpServer.auth.oauth.grantType.password" })} - + {grantType === "password" && ( + + {intl.formatMessage({ id: "mcpServer.auth.oauth.grantType.password" })} + + )}
+ {grantType === "password" && ( +

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

+ )} +
)} diff --git a/src/i18n/locales/en-US/mcpServer.json b/src/i18n/locales/en-US/mcpServer.json index 6dc4323..05269aa 100644 --- a/src/i18n/locales/en-US/mcpServer.json +++ b/src/i18n/locales/en-US/mcpServer.json @@ -144,13 +144,15 @@ "mcpServer.auth.oauth.grantTypePlaceholder": "Select grant type", "mcpServer.auth.oauth.grantType.authorizationCode": "Authorization code (user login)", "mcpServer.auth.oauth.grantType.clientCredentials": "Client credentials (machine to machine)", - "mcpServer.auth.oauth.grantType.password": "Resource owner password (legacy)", + "mcpServer.auth.oauth.grantType.password": "Resource owner password (deprecated)", + "mcpServer.auth.oauth.passwordDeprecated": "Password grant is deprecated by OAuth 2.1. Migrate this MCP server to authorization_code or client_credentials.", "mcpServer.auth.oauth.issuerUrlLabel": "Issuer URL", "mcpServer.auth.oauth.issuerUrlPlaceholder": "e.g. https://auth.example.com", "mcpServer.auth.oauth.issuerUrlHelp": "Authorization server's base URL for endpoint discovery and Dynamic Client Registration (DCR)", "mcpServer.auth.oauth.redirectUriLabel": "Redirect URI", - "mcpServer.auth.oauth.redirectUriPlaceholder": "e.g. https://gateway.example.com/oauth/callback", - "mcpServer.auth.oauth.redirectUriHelp": "Copy URI into the OAuth application's allowed redirect URI", + "mcpServer.auth.oauth.redirectUriCopy": "Copy to clipboard", + "mcpServer.auth.oauth.redirectUriHelp": "Configure your OAuth app to use this redirect URI.", + "mcpServer.auth.oauth.redirectUriLocalWarning": "The server's public URL is not configured. Redirect URIs derived from localhost will not work for external OAuth providers.", "mcpServer.auth.oauth.usernameLabel": "Username", "mcpServer.auth.oauth.usernamePlaceholder": "e.g. service-account", "mcpServer.auth.oauth.passwordLabel": "Password", diff --git a/src/i18n/locales/es-ES/mcpServer.json b/src/i18n/locales/es-ES/mcpServer.json index 5512dd8..a3d4cc9 100644 --- a/src/i18n/locales/es-ES/mcpServer.json +++ b/src/i18n/locales/es-ES/mcpServer.json @@ -144,13 +144,15 @@ "mcpServer.auth.oauth.grantTypePlaceholder": "Selecciona el tipo de concesión", "mcpServer.auth.oauth.grantType.authorizationCode": "Código de autorización (inicio de sesión de usuario)", "mcpServer.auth.oauth.grantType.clientCredentials": "Credenciales de cliente (máquina a máquina)", - "mcpServer.auth.oauth.grantType.password": "Contraseña del propietario del recurso (heredado)", + "mcpServer.auth.oauth.grantType.password": "Contraseña del propietario del recurso (obsoleto)", + "mcpServer.auth.oauth.passwordDeprecated": "La concesión por contraseña está obsoleta en OAuth 2.1. Migre este servidor MCP a authorization_code o client_credentials.", "mcpServer.auth.oauth.issuerUrlLabel": "URL del emisor", "mcpServer.auth.oauth.issuerUrlPlaceholder": "p. ej. https://auth.example.com", "mcpServer.auth.oauth.issuerUrlHelp": "URL base del servidor de autorización para el descubrimiento de endpoints y el registro dinámico de clientes (DCR)", "mcpServer.auth.oauth.redirectUriLabel": "URI de redirección", - "mcpServer.auth.oauth.redirectUriPlaceholder": "p. ej. https://gateway.example.com/oauth/callback", - "mcpServer.auth.oauth.redirectUriHelp": "Copia la URI en las URI de redirección permitidas de la aplicación OAuth", + "mcpServer.auth.oauth.redirectUriCopy": "Copiar al portapapeles", + "mcpServer.auth.oauth.redirectUriHelp": "Configure su aplicación OAuth para usar esta URI de redirección.", + "mcpServer.auth.oauth.redirectUriLocalWarning": "La URL pública del servidor no está configurada. Las URI de redirección derivadas de localhost no funcionarán con proveedores OAuth externos.", "mcpServer.auth.oauth.usernameLabel": "Nombre de usuario", "mcpServer.auth.oauth.usernamePlaceholder": "p. ej. service-account", "mcpServer.auth.oauth.passwordLabel": "Contraseña", diff --git a/src/i18n/locales/pt-BR/mcpServer.json b/src/i18n/locales/pt-BR/mcpServer.json index 8efabfc..5e60464 100644 --- a/src/i18n/locales/pt-BR/mcpServer.json +++ b/src/i18n/locales/pt-BR/mcpServer.json @@ -144,13 +144,15 @@ "mcpServer.auth.oauth.grantTypePlaceholder": "Selecione o tipo de concessão", "mcpServer.auth.oauth.grantType.authorizationCode": "Código de autorização (login do usuário)", "mcpServer.auth.oauth.grantType.clientCredentials": "Credenciais do cliente (máquina a máquina)", - "mcpServer.auth.oauth.grantType.password": "Senha do proprietário do recurso (legado)", + "mcpServer.auth.oauth.grantType.password": "Senha do proprietário do recurso (obsoleto)", + "mcpServer.auth.oauth.passwordDeprecated": "A concessão por senha está obsoleta no OAuth 2.1. Migre este servidor MCP para authorization_code ou client_credentials.", "mcpServer.auth.oauth.issuerUrlLabel": "URL do emissor", "mcpServer.auth.oauth.issuerUrlPlaceholder": "ex.: https://auth.example.com", "mcpServer.auth.oauth.issuerUrlHelp": "URL base do servidor de autorização para descoberta de endpoints e registro dinâmico de clientes (DCR)", "mcpServer.auth.oauth.redirectUriLabel": "URI de redirecionamento", - "mcpServer.auth.oauth.redirectUriPlaceholder": "ex.: https://gateway.example.com/oauth/callback", - "mcpServer.auth.oauth.redirectUriHelp": "Copie a URI para as URIs de redirecionamento permitidas do aplicativo OAuth", + "mcpServer.auth.oauth.redirectUriCopy": "Copiar para a área de transferência", + "mcpServer.auth.oauth.redirectUriHelp": "Configure seu aplicativo OAuth para usar esta URI de redirecionamento.", + "mcpServer.auth.oauth.redirectUriLocalWarning": "A URL pública do servidor não está configurada. URIs de redirecionamento derivadas de localhost não funcionarão com provedores OAuth externos.", "mcpServer.auth.oauth.usernameLabel": "Nome de usuário", "mcpServer.auth.oauth.usernamePlaceholder": "ex.: service-account", "mcpServer.auth.oauth.passwordLabel": "Senha",