diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index caddf043adf..0bace688d1d 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -2031,6 +2031,8 @@ export { GENERIC_MCP_AUTH_MODES, genericMcpAuthModeSchema, genericMcpOAuthClientSchema, + type GenericMcpAuthMode, + type GenericMcpOAuthClient, reconnectToolAppSchema, createToolApplicationSchema, finishToolAppSchema, diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 5eae4166fd2..ad1dca720e7 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -868,6 +868,8 @@ export { GENERIC_MCP_AUTH_MODES, genericMcpAuthModeSchema, genericMcpOAuthClientSchema, + type GenericMcpAuthMode, + type GenericMcpOAuthClient, reconnectToolAppSchema, finishToolAppSchema, updateToolApplicationSchema, diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index dec18e332f8..5c984034d00 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { CONNECTABLE_APP_DEFINITIONS } from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; import { queryKeys } from "@/lib/queryKeys"; import { AppsConnect } from "./AppsConnect"; @@ -191,7 +192,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await render(); await gotoLinkFrame(container, "https://www.example.com/actions"); - expect(container.textContent).toContain("Connect with a link"); + expect(container.textContent).toContain("Connect your own MCP server"); expect(container.textContent).toContain("https://www.example.com/actions"); expect(container.textContent).toContain("Does it need a key?"); expect(buttonByText("No")).toBeTruthy(); @@ -651,7 +652,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { container.querySelectorAll("input"), ).filter((i) => i.type === "password"); expect(passwordInputs).toHaveLength(1); - expect(container.textContent).toContain("Your key is stored securely."); + expect(container.textContent).toContain("Stored securely."); await act(async () => setInputValue(passwordInputs[0], "secret-key")); await flushReact(); @@ -683,7 +684,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await flushReact(); - expect(container.textContent).toContain("Connect with a link"); + expect(container.textContent).toContain("Connect your own MCP server"); expect(container.textContent).toContain(zapierUrl); expect(nameInputFrom(container)?.value).toBe("Zapier"); expect(container.querySelector('input[type="password"]')).toBeNull(); @@ -865,7 +866,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { "link=https%3A%2F%2Fwww.example.com%2Factions&name=Bla&applicationId=app-77"; await render(); - expect(container.textContent).toContain("Connect with a link"); + expect(container.textContent).toContain("Connect your own MCP server"); expect(container.textContent).toContain("https://www.example.com/actions"); const nameInput = Array.from(container.querySelectorAll("input")).find( (i) => i.getAttribute("placeholder") === "My app", @@ -1076,3 +1077,304 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); }); }); + +/** + * PAP-17087 — the BYO URL card is now the guided universal flow. What matters is + * that the simple path stayed simple, that each failure mode names the thing the + * operator has to change, and that a pasted endpoint that needs sign-in actually + * gets there instead of a "coming soon" toast. + */ +describe("AppsConnect — guided generic MCP flow (PAP-17087)", () => { + let container: HTMLDivElement; + + beforeEach(() => { + mockSearch.value = ""; + mockParams.appKey = undefined; + container = document.createElement("div"); + document.body.appendChild(container); + listGalleryMock.mockResolvedValue({ apps: [ZAPIER] }); + listApplicationsMock.mockResolvedValue({ applications: [] }); + listConnectionsMock.mockResolvedValue({ connections: [] }); + listAgentsMock.mockResolvedValue([]); + finishAppMock.mockResolvedValue({}); + putConnectionInstallsMock.mockResolvedValue({ connectionId: "conn-1", installs: [] }); + startOAuthMock.mockResolvedValue({ + connectionId: "conn-1", + provider: "mcp_example_test", + authorizationUrl: "https://auth.example.test/authorize?state=abc", + expiresAt: "2099-01-01T00:00:00.000Z", + }); + connectAppMock.mockResolvedValue({ + connectionId: "conn-1", + application: { id: "app-1", name: "mcp.example.test" }, + actions: { readOnly: [], canMakeChanges: [] }, + catalog: [], + suggestedDefaults: {}, + }); + }); + + afterEach(() => { + document.body.removeChild(container); + document.body.innerHTML = ""; + vi.clearAllMocks(); + }); + + async function render() { + const root = createRoot(container); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + await flushReact(); + return root; + } + + async function openAdvanced() { + await act(async () => { + buttonContaining("Advanced authentication")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + } + + // Production branches on `error instanceof ApiError`, so the double has to be a + // real one — a look-alike would silently fall through to the generic message and + // make these tests pass for the wrong reason. + function apiError(status: number, code: string, message: string) { + return new ApiError(message, status, { error: message, details: { code } }); + } + + it("keeps the endpoint host and Unverified server visible from setup through review", async () => { + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + + expect(container.textContent).toContain("Unverified server"); + expect(container.textContent).toContain("mcp.example.test"); + + connectAppMock.mockResolvedValue({ + connectionId: "conn-1", + application: { id: "app-1", name: "mcp.example.test" }, + actions: { + readOnly: [{ + catalogEntryId: "cat-read", + toolName: "list_things", + title: "List things", + description: null, + riskLevel: "read", + isReadOnly: true, + isWrite: false, + isDestructive: false, + status: "active", + }], + canMakeChanges: [], + }, + catalog: [], + suggestedDefaults: { askFirstRiskLevels: ["write", "destructive"] }, + }); + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + // Now on the review step — the label has to survive the step change. + expect(container.textContent).toContain("List things"); + expect(container.textContent).toContain("Unverified server"); + expect(container.textContent).toContain("mcp.example.test"); + }); + + it("offers a curated setup as a convenience without leaving the generic flow", async () => { + await render(); + await gotoLinkFrame(container, "https://mcp.zapier.com/api/v1/connect?token=t"); + + // Both routes are present: the branded shortcut and the generic form itself. + expect(container.textContent).toContain("Paperclip has a guided setup for Zapier."); + expect(container.textContent).toContain("Connect your own MCP server"); + expect(buttonByText("Check link")).toBeTruthy(); + }); + + it("explains a private-network address instead of blaming the key", async () => { + connectAppMock.mockRejectedValue( + apiError(400, "remote_http_private_endpoint", "Remote MCP connection URL cannot target private or reserved network addresses"), + ); + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(container.textContent).toContain("That address is inside a private network"); + // Still on the setup screen with the address in hand, not bounced back. + expect(buttonByText("Check link")).toBeTruthy(); + }); + + it("explains an unreachable host", async () => { + connectAppMock.mockRejectedValue(apiError(400, "remote_http_dns_failed", "hostname could not be resolved")); + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(container.textContent).toContain("We couldn't find that host"); + }); + + it("opens advanced authentication when the server wants a credential we can't discover", async () => { + connectAppMock.mockRejectedValue(apiError(502, "oauth_challenge", "This app needs you to sign in.")); + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(container.textContent).toContain("This server wants a credential"); + // The advanced section is now open, so the fields to fix it are on screen. + expect(container.textContent).toContain("Custom headers"); + expect(container.textContent).not.toContain("coming soon"); + }); + + it("sends the operator to sign-in when the endpoint needs browser authorization", async () => { + connectAppMock.mockResolvedValue({ + connectionId: "conn-1", + application: { id: "app-1", name: "mcp.example.test" }, + actions: { readOnly: [], canMakeChanges: [] }, + catalog: [], + suggestedDefaults: {}, + auth: { kind: "oauth", startUrl: "https://auth.example.test/authorize?state=abc" }, + }); + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(navigateTopLevelMock).toHaveBeenCalledWith("https://auth.example.test/authorize?state=abc"); + }); + + it("asks for a preregistered client rather than losing the draft", async () => { + connectAppMock.mockResolvedValue({ + connectionId: "conn-1", + application: { id: "app-1", name: "mcp.example.test" }, + actions: { readOnly: [], canMakeChanges: [] }, + catalog: [], + suggestedDefaults: {}, + auth: { kind: "oauth", startUrl: null, manualClientRequired: true }, + }); + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(container.textContent).toContain("This server needs sign-in details you create yourself"); + expect(container.textContent).toContain("Client ID"); + expect(container.textContent).toContain("Client secret"); + // No redirect happened: there is nothing to redirect to yet. + expect(navigateTopLevelMock).not.toHaveBeenCalled(); + }); + + it("submits custom headers as secret-backed credential values", async () => { + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await openAdvanced(); + await act(async () => { + buttonByText("Custom headers")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const nameInput = Array.from(container.querySelectorAll("input")) + .find((input) => input.getAttribute("aria-label") === "Header name")!; + await act(async () => setInputValue(nameInput, "X-Api-Key")); + await flushReact(); + const valueInput = Array.from(container.querySelectorAll("input")) + .find((input) => input.type === "password")!; + await act(async () => setInputValue(valueInput, "phx_secret")); + await flushReact(); + + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(connectAppMock).toHaveBeenCalledTimes(1); + const [, input] = connectAppMock.mock.calls[0]; + expect(input).toMatchObject({ + link: "https://mcp.example.test/mcp", + authMode: "custom_headers", + credentialValues: { "headers.X-Api-Key": "phx_secret" }, + }); + }); + + it("blocks a header Paperclip refuses to send before making a request", async () => { + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await openAdvanced(); + await act(async () => { + buttonByText("Custom headers")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const nameInput = Array.from(container.querySelectorAll("input")) + .find((input) => input.getAttribute("aria-label") === "Header name")!; + await act(async () => setInputValue(nameInput, "Host")); + await flushReact(); + const valueInput = Array.from(container.querySelectorAll("input")) + .find((input) => input.type === "password")!; + await act(async () => setInputValue(valueInput, "evil.example")); + await flushReact(); + + expect(container.textContent).toContain('Paperclip manages the "Host" header'); + expect(buttonByText("Check link")?.disabled).toBe(true); + expect(connectAppMock).not.toHaveBeenCalled(); + }); + + it("sends preregistered client credentials when the operator supplies them", async () => { + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await openAdvanced(); + await act(async () => { + buttonByText("Browser sign-in")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const clientIdInput = container.querySelector("#generic-mcp-client-id")!; + await act(async () => setInputValue(clientIdInput, "operator-client")); + await flushReact(); + const clientSecretInput = container.querySelector("#generic-mcp-client-secret")!; + await act(async () => setInputValue(clientSecretInput, "operator-secret")); + await flushReact(); + + await act(async () => { + buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const [, input] = connectAppMock.mock.calls[0]; + expect(input).toMatchObject({ + authMode: "oauth", + oauthClient: { clientId: "operator-client", clientSecret: "operator-secret" }, + }); + }); + + it("keeps protocol jargon off the consumer path", async () => { + await render(); + await gotoLinkFrame(container, "https://mcp.example.test/mcp"); + await openAdvanced(); + + for (const jargon of ["DCR", "Dynamic Client Registration", "CIMD", "Client ID Metadata", "RFC", "PKCE"]) { + expect(container.textContent, jargon).not.toContain(jargon); + } + }); +}); diff --git a/ui/src/pages/apps/AppsConnect.tsx b/ui/src/pages/apps/AppsConnect.tsx index c638f943c16..c6894899f55 100644 --- a/ui/src/pages/apps/AppsConnect.tsx +++ b/ui/src/pages/apps/AppsConnect.tsx @@ -11,6 +11,7 @@ import { Loader2, Lock, Search, + ShieldAlert, TerminalSquare, } from "lucide-react"; import type { LucideIcon } from "lucide-react"; @@ -50,6 +51,18 @@ import { navigateTopLevel } from "@/lib/browserNavigation"; import { AppLogo } from "./AppLogo"; import { appSourceConnectHref, isMcpDirectOAuthConnectSlug } from "./app-connect-policy"; import { parseGoogleSheetIds } from "./google-sheets"; +import { + canSubmitGenericConnect, + customHeaderError, + endpointHost, + genericConnectGuidance, + genericConnectPayload, + newCustomHeaderRow, + type CustomHeaderRow, + type GenericConnectDraft, + type GenericConnectGuidance, + type GenericMcpAuthMode, +} from "./generic-mcp-connect"; import { autoExtendNotice, INSTALL_ALL_WARNING, installInfoNotice, installPayload } from "@/lib/tool-installs"; type Step = "gallery" | "key" | "actions" | "who" | "install" | "success"; @@ -70,7 +83,6 @@ function appConnectHref(appKey: string, step: Step): string { } type AppAccessSelection = "all_agents" | { agentIds: string[] }; type InstallMode = "none" | "specific" | "all"; -const LINK_CREDENTIAL_CONFIG_PATH = "credentials.authorization"; const STEP_LABELS = ["Pick app", "Add your key", "Choose actions", "Choose access", "Install tools"]; const STEP_INDEX: Record, number> = { @@ -177,6 +189,15 @@ export function AppsConnect() { const [linkName, setLinkName] = useState(prefill.name || (zapierSource ? "Zapier" : "")); const [linkNeedsKey, setLinkNeedsKey] = useState(false); const [linkKey, setLinkKey] = useState(""); + // Generic ("connect your own MCP server") flow state. `authMode: auto` is the + // simple path: Paperclip probes the endpoint and branches on what it finds. + const [linkAuthMode, setLinkAuthMode] = useState("auto"); + const [linkHeaders, setLinkHeaders] = useState(() => [newCustomHeaderRow()]); + const [linkOAuthClientId, setLinkOAuthClientId] = useState(""); + const [linkOAuthClientSecret, setLinkOAuthClientSecret] = useState(""); + const [linkAdvancedOpen, setLinkAdvancedOpen] = useState(false); + const [linkGuidance, setLinkGuidance] = useState(null); + const [genericOAuthPending, setGenericOAuthPending] = useState(false); const [credentials, setCredentials] = useState>({}); const [connectionMethodKey, setConnectionMethodKey] = useState(""); const [configValues, setConfigValues] = useState>({}); @@ -193,6 +214,50 @@ export function AppsConnect() { const directOAuthStartedRef = useRef(false); const directOAuthRetryingRef = useRef(false); + const resetGenericAuthState = () => { + setLinkAuthMode("auto"); + setLinkHeaders([newCustomHeaderRow()]); + setLinkOAuthClientId(""); + setLinkOAuthClientSecret(""); + setLinkAdvancedOpen(false); + setLinkGuidance(null); + setGenericOAuthPending(false); + }; + + /** + * Switch to a curated app's branded setup. Reached from the gallery grid and, as + * a convenience, from the guided generic flow when the pasted endpoint matches a + * definition — the generic path stays available either way. + */ + const useMatchedGalleryEntry = (picked: AppDefinition) => { + if ( + getAvailableConnectionMethod(picked)?.auth === "oauth" && + isMcpDirectOAuthConnectSlug(picked.slug) + ) { + navigate(appSourceConnectHref(picked.slug)); + return; + } + setEntry(picked); + setGalleryName(picked.name); + setLinkUrl(""); + setLinkName(""); + setLinkNeedsKey(false); + setLinkKey(""); + resetGenericAuthState(); + setCredentials({}); + const methods = getAvailableConnectionMethods(picked); + const initialMethod = methods.length === 1 ? methods[0]! : null; + setConnectionMethodKey(initialMethod?.key ?? ""); + setConfigValues(defaultMethodConfig(initialMethod)); + setGoogleSheetsLinks(""); + setGoogleSheetsError(null); + setConnectResult(null); + setInstallMode("none"); + setInstallAgentIds(new Set()); + setStep("key"); + navigate(appConnectHref(picked.slug, "key")); + }; + const openGallery = () => { setEntry(null); setGalleryName(""); @@ -200,6 +265,7 @@ export function AppsConnect() { setLinkName(""); setLinkNeedsKey(false); setLinkKey(""); + resetGenericAuthState(); setCredentials({}); setConnectionMethodKey(""); setConfigValues({}); @@ -250,6 +316,13 @@ export function AppsConnect() { [applicationsQuery.data, connectionsQuery.data, createNewConnection, directOAuthSource, prefill.applicationId], ); + // A curated definition covering the pasted endpoint is offered as a branded + // convenience only; the generic flow remains the default and stays complete. + const linkMatchedEntry = useMemo( + () => (linkUrl && !entry ? getAppDefinitionForUrl(linkUrl, galleryQuery.data?.apps ?? []) : null), + [entry, galleryQuery.data, linkUrl], + ); + const directOAuthEntry = entry && getAvailableConnectionMethod(entry)?.auth === "oauth" && isMcpDirectOAuthConnectSlug(entry.slug) @@ -302,28 +375,46 @@ export function AppsConnect() { applicationId: prefill.applicationId, }); } - const trimmedKey = linkNeedsKey ? linkKey.trim() : ""; - const trimmedName = linkName.trim(); return toolsApi.connectApp(selectedCompanyId!, { - link: linkUrl, - name: trimmedName || undefined, - credentialValues: trimmedKey ? { [LINK_CREDENTIAL_CONFIG_PATH]: trimmedKey } : undefined, + ...genericConnectPayload({ + link: linkUrl, + name: linkName, + authMode: linkAuthMode, + needsKey: linkNeedsKey, + keyValue: linkKey, + headers: linkHeaders, + oauthClientId: linkOAuthClientId, + oauthClientSecret: linkOAuthClientSecret, + }), applicationId: prefill.applicationId, }); }, onSuccess: (result) => { if (result.auth?.kind === "oauth") { setConnectResult(result); + // Discovery worked but this authorization server insists on a client the + // operator registers themselves. Keep the draft and ask for it in place + // rather than sending them back to the start. + if (result.auth.manualClientRequired) { + setLinkGuidance(genericConnectGuidance("oauth_manual_client_required", null)); + setLinkAuthMode("oauth"); + setLinkAdvancedOpen(true); + setGenericOAuthPending(false); + return; + } const startUrl = result.auth.startUrl?.trim(); if (!startUrl) { setOAuthPhase("starting"); + setGenericOAuthPending(true); startOAuth(result.connectionId); return; } setOAuthPhase("redirecting"); + setGenericOAuthPending(true); navigateTopLevel(startUrl); return; } + setLinkGuidance(null); setConnectResult(result); const defaults: Record = {}; for (const a of result.actions.readOnly) defaults[a.catalogEntryId] = true; @@ -348,14 +439,20 @@ export function AppsConnect() { ); return; } - const oauthRequired = details?.code === "oauth_challenge"; + // The generic URL path explains the specific corrective action inline, + // beside the fields the operator has to change. A toast can't do that, and + // for a pasted address "check your key" is usually the wrong advice. + if (!entry && linkUrl) { + const code = typeof details?.code === "string" ? details.code : null; + const guidance = genericConnectGuidance(code, error instanceof Error ? error.message : null); + setLinkGuidance(guidance); + setGenericOAuthPending(false); + if (guidance.focus === "credentials") setLinkAdvancedOpen(true); + return; + } pushToast({ - title: oauthRequired ? "Sign-in required" : "Couldn’t connect", - body: oauthRequired - ? "This app needs you to sign in - coming soon." - : error instanceof Error - ? error.message - : "Please check your key and try again.", + title: "Couldn’t connect", + body: error instanceof Error ? error.message : "Please check your key and try again.", tone: "error", }); }, @@ -525,6 +622,39 @@ export function AppsConnect() { ); } + // A pasted endpoint that needs browser sign-in gets the same waiting/retry + // screen a curated OAuth app does, minus the branding it doesn't have. + if (genericOAuthPending && !entry && linkUrl && step === "key") { + return ( + { + setOAuthError(null); + const connectionId = connectResult?.connectionId; + if (connectionId) { + setOAuthPhase("starting"); + startOAuth(connectionId); + return; + } + // No draft to resume, so fall back to the setup screen rather than + // creating a second connection for the same endpoint. + setGenericOAuthPending(false); + setOAuthPhase("entry"); + }} + onCancel={() => { + setGenericOAuthPending(false); + setOAuthPhase("entry"); + setOAuthError(null); + }} + /> + ); + } + const appName = connectResult?.application.name ?? entry?.name ?? @@ -562,6 +692,7 @@ export function AppsConnect() { ? { name: "Zapier", logoUrl: zapierEntry?.branding.logoUrl ?? null } : undefined } + unverifiedHost={!entry && !zapierSource && step !== "gallery" ? endpointHost(linkUrl) : null} onCancel={() => navigate("/apps")} /> )} @@ -572,33 +703,7 @@ export function AppsConnect() { apps={galleryQuery.data?.apps ?? []} byo={searchParams.get("byo") === "1"} source={searchParams.get("source")} - onPick={(picked) => { - if ( - getAvailableConnectionMethod(picked)?.auth === "oauth" && - isMcpDirectOAuthConnectSlug(picked.slug) - ) { - navigate(appSourceConnectHref(picked.slug)); - return; - } - setEntry(picked); - setGalleryName(picked.name); - setLinkUrl(""); - setLinkName(""); - setLinkNeedsKey(false); - setLinkKey(""); - setCredentials({}); - const methods = getAvailableConnectionMethods(picked); - const initialMethod = methods.length === 1 ? methods[0]! : null; - setConnectionMethodKey(initialMethod?.key ?? ""); - setConfigValues(defaultMethodConfig(initialMethod)); - setGoogleSheetsLinks(""); - setGoogleSheetsError(null); - setConnectResult(null); - setInstallMode("none"); - setInstallAgentIds(new Set()); - setStep("key"); - navigate(appConnectHref(picked.slug, "key")); - }} + onPick={useMatchedGalleryEntry} onUseLink={(url) => { const matchedEntry = getAppDefinitionForUrl(url, galleryQuery.data?.apps ?? []); setEntry(null); @@ -671,9 +776,35 @@ export function AppsConnect() { }} keyValue={linkKey} onKeyChange={setLinkKey} - submitting={connectMutation.isPending} + authMode={linkAuthMode} + onAuthModeChange={(next) => { + setLinkAuthMode(next); + setLinkGuidance(null); + // Leaving the simple path means the explicit choice governs; drop the + // "does it need a key?" answer so the two can't disagree. + if (next !== "auto") setLinkNeedsKey(false); + if (next !== "bearer" && next !== "auto") setLinkKey(""); + }} + headers={linkHeaders} + onHeadersChange={(next) => { + setLinkHeaders(next); + setLinkGuidance(null); + }} + oauthClientId={linkOAuthClientId} + onOAuthClientIdChange={setLinkOAuthClientId} + oauthClientSecret={linkOAuthClientSecret} + onOAuthClientSecretChange={setLinkOAuthClientSecret} + advancedOpen={linkAdvancedOpen} + onAdvancedOpenChange={setLinkAdvancedOpen} + guidance={linkGuidance} + matchedEntry={linkMatchedEntry} + onUseMatchedEntry={linkMatchedEntry ? () => useMatchedGalleryEntry(linkMatchedEntry) : undefined} + submitting={connectMutation.isPending || genericOAuthPending} onBack={() => setStep("gallery")} - onConnect={() => connectMutation.mutate(undefined)} + onConnect={() => { + setLinkGuidance(null); + connectMutation.mutate(undefined); + }} /> )} @@ -755,6 +886,7 @@ function StepHeader({ activeIndex, labels, appIdentity, + unverifiedHost, onCancel, }: { subtitle: string; @@ -762,6 +894,12 @@ function StepHeader({ activeIndex: number; labels: string[]; appIdentity?: { name: string; logoUrl: string | null }; + /** + * Host of an unknown remote MCP server. Present for the whole generic flow so + * the operator can see whose server they are configuring at every step, not + * just on the screen where they pasted the address. + */ + unverifiedHost?: string | null; onCancel: () => void; }) { return ( @@ -776,6 +914,7 @@ function StepHeader({ {appIdentity ? `Connect ${appIdentity.name}` : "Connect an app"}

{subtitle}

+ {unverifiedHost ? : null} ) : ( )} @@ -1208,6 +1355,19 @@ function defaultLinkName(link: string): string | null { } } +/** + * The guided universal flow for an unknown remote MCP server (PAP-17087, plan 2). + * + * The simple path stays exactly as short as it was — URL, name, "does it need a + * key?" — because that is all most servers need. Everything protocol-shaped lives + * behind "Advanced authentication", and no OAuth/DCR/CIMD jargon appears on the + * consumer path: the operator picks how the server authenticates, not which RFC + * Paperclip will use to satisfy it. + * + * The endpoint host and the "Unverified server" label stay visible the whole way + * through, so the operator can always see whose server they are about to let + * agents call. + */ function LinkConnectStep({ link, name, @@ -1216,6 +1376,19 @@ function LinkConnectStep({ onNeedsKeyChange, keyValue, onKeyChange, + authMode, + onAuthModeChange, + headers, + onHeadersChange, + oauthClientId, + onOAuthClientIdChange, + oauthClientSecret, + onOAuthClientSecretChange, + advancedOpen, + onAdvancedOpenChange, + guidance, + matchedEntry, + onUseMatchedEntry, submitting, onBack, onConnect, @@ -1227,10 +1400,43 @@ function LinkConnectStep({ onNeedsKeyChange: (next: boolean) => void; keyValue: string; onKeyChange: (next: string) => void; + authMode: GenericMcpAuthMode; + onAuthModeChange: (next: GenericMcpAuthMode) => void; + headers: CustomHeaderRow[]; + onHeadersChange: (next: CustomHeaderRow[]) => void; + oauthClientId: string; + onOAuthClientIdChange: (next: string) => void; + oauthClientSecret: string; + onOAuthClientSecretChange: (next: string) => void; + advancedOpen: boolean; + onAdvancedOpenChange: (next: boolean) => void; + guidance: GenericConnectGuidance | null; + /** A curated app whose endpoint matches, offered as a convenience only. */ + matchedEntry?: AppDefinition | null; + onUseMatchedEntry?: () => void; submitting: boolean; onBack: () => void; onConnect: () => void; }) { + const host = endpointHost(link); + const headerError = authMode === "custom_headers" ? customHeaderError(headers) : null; + const draft: GenericConnectDraft = { + link, + name, + authMode, + needsKey, + keyValue, + headers, + oauthClientId, + oauthClientSecret, + }; + const canSubmit = canSubmitGenericConnect(draft); + const showSimpleKeyQuestion = authMode === "auto"; + + const updateHeader = (id: string, patch: Partial) => { + onHeadersChange(headers.map((row) => (row.id === id ? { ...row, ...patch } : row))); + }; + return (
@@ -1238,51 +1444,76 @@ function LinkConnectStep({
-

Connect with a link

-

{link}

+

Connect your own MCP server

+

{link}

+
+ {matchedEntry && onUseMatchedEntry ? ( +
+
+ + Paperclip has a guided setup for {matchedEntry.name}. +
+ +
+ ) : null} + + {guidance ? ( +
+ + {guidance.body} + +
+ ) : null} +
- + onNameChange(e.target.value)} placeholder="My app" className="mt-2 h-11" />

- We filled this in from the link. Change it if you’d like. + We filled this in from the address. Change it if you'd like.

-
- -
- onNeedsKeyChange(false)} - /> - onNeedsKeyChange(true)} - /> + {showSimpleKeyQuestion && ( +
+ +
+ onNeedsKeyChange(false)} + /> + onNeedsKeyChange(true)} + /> +
+

+ {needsKey + ? "Paste the key this app gave you." + : "Most servers just work from the address — pick Yes only if the server gave you a key, or if it asks you to sign in."} +

-

- {needsKey - ? "Paste the key this app gave you." - : "Most apps just work from the link — pick Yes only if the app gave you a key."} -

-
+ )} - {needsKey && ( + {(showSimpleKeyQuestion && needsKey) || authMode === "bearer" ? (
- +
- -
- -
-
Your key is stored securely.
-
- You can replace it anytime from this app’s page. -
-
-
+
- )} + ) : null} + + + + Advanced authentication + + + +

+ Only needed when the server's docs are specific about how to authenticate. +

+
+ {GENERIC_AUTH_MODE_OPTIONS.map((option) => ( + onAuthModeChange(option.mode)} + /> + ))} +
+

+ {GENERIC_AUTH_MODE_OPTIONS.find((option) => option.mode === authMode)?.hint} +

+ + {authMode === "custom_headers" ? ( +
+ {headers.map((row) => ( +
+ updateHeader(row.id, { name: e.target.value })} + placeholder="Header name" + aria-label="Header name" + className="h-10 font-mono" + /> + updateHeader(row.id, { value: e.target.value })} + placeholder="Value" + aria-label={row.name.trim() ? `Value for ${row.name.trim()}` : "Header value"} + className="h-10 font-mono" + /> + +
+ ))} + + {headerError ?

{headerError}

: null} + +
+ ) : null} + + {authMode === "oauth" ? ( +
+

+ Paperclip sets sign-in up on its own whenever the server allows it. Only fill these in when the + server's docs tell you to register Paperclip yourself first. +

+
+ + onOAuthClientIdChange(e.target.value)} + autoComplete="off" + placeholder="Optional" + className="mt-2 h-11 font-mono" + /> +
+
+ + onOAuthClientSecretChange(e.target.value)} + placeholder="Optional" + className="mt-2 h-11 font-mono" + /> +
+ +
+ ) : null} + +
@@ -1311,9 +1640,9 @@ function LinkConnectStep({
- We’ll check the link before turning anything on. + We'll check the server before turning anything on. - @@ -1323,6 +1652,67 @@ function LinkConnectStep({ ); } +/** + * What the operator is choosing is how the *server* authenticates, in its own + * terms. Paperclip decides internally whether that means a preconfigured client, + * a client ID metadata document, dynamic registration, or the credentials pasted + * below — none of which belongs on this screen. + */ +const GENERIC_AUTH_MODE_OPTIONS: Array<{ mode: GenericMcpAuthMode; label: string; hint: string }> = [ + { + mode: "auto", + label: "Let Paperclip check", + hint: "Paperclip asks the server what it needs and walks you through it. Start here.", + }, + { + mode: "none", + label: "No sign-in needed", + hint: "The server is open to anyone with the address.", + }, + { + mode: "bearer", + label: "Key or token", + hint: "Paperclip sends your key as an Authorization header.", + }, + { + mode: "custom_headers", + label: "Custom headers", + hint: "For servers that name their own headers. Values are stored as Paperclip secrets and can\u2019t be read back.", + }, + { + mode: "oauth", + label: "Browser sign-in", + hint: "You\u2019ll sign in at the provider. Add a client ID and secret only if the provider requires you to register Paperclip first.", + }, +]; + +function UnverifiedServerBadge({ host, className }: { host: string | null; className?: string }) { + return ( +
+ + + Unverified server + + {host ? {host} : null} +
+ ); +} + +function StoredSecurelyNote() { + return ( +
+ +
+
Stored securely.
+
+ Paperclip keeps this in its encrypted secret store. You can replace it anytime from this app's page, + but it can't be read back. +
+
+
+ ); +} + function SegmentedOption({ label, selected, diff --git a/ui/src/pages/apps/generic-mcp-connect.test.ts b/ui/src/pages/apps/generic-mcp-connect.test.ts new file mode 100644 index 00000000000..d8cae041b72 --- /dev/null +++ b/ui/src/pages/apps/generic-mcp-connect.test.ts @@ -0,0 +1,191 @@ +import { describe, expect, it } from "vitest"; +import { + canSubmitGenericConnect, + customHeaderError, + endpointHost, + genericConnectGuidance, + genericConnectPayload, + newCustomHeaderRow, + type GenericConnectDraft, +} from "./generic-mcp-connect"; + +function draft(overrides: Partial = {}): GenericConnectDraft { + return { + link: "https://mcp.example.test/mcp", + name: "Example", + authMode: "auto", + needsKey: false, + keyValue: "", + headers: [newCustomHeaderRow()], + oauthClientId: "", + oauthClientSecret: "", + ...overrides, + }; +} + +describe("endpointHost", () => { + it("returns the host so the operator can see whose server this is", () => { + expect(endpointHost("https://mcp.example.test/mcp?project_id=1")).toBe("mcp.example.test"); + expect(endpointHost("http://127.0.0.1:8848/mcp")).toBe("127.0.0.1:8848"); + }); + + it("returns null for anything that isn't a URL", () => { + expect(endpointHost("not a url")).toBeNull(); + expect(endpointHost("")).toBeNull(); + }); +}); + +describe("genericConnectGuidance", () => { + it("names the URL as the thing to fix for invalid, unsafe and unreachable endpoints", () => { + for (const code of [ + "mcp_remote_url_invalid", + "remote_http_private_endpoint", + "remote_http_dns_failed", + "runtime_error", + ]) { + expect(genericConnectGuidance(code, null).focus, code).toBe("url"); + } + }); + + it("names the credentials for a challenge with no discoverable sign-in", () => { + const guidance = genericConnectGuidance("oauth_challenge", null); + expect(guidance.focus).toBe("credentials"); + expect(guidance.body).toContain("Advanced authentication"); + }); + + it("points at the deployment when Paperclip itself has no public HTTPS address", () => { + expect(genericConnectGuidance("oauth_redirect_origin_unsupported", null).focus).toBe("deployment"); + }); + + it("passes a rejected header's own message through", () => { + const guidance = genericConnectGuidance( + "mcp_header_rejected", + 'Paperclip manages the "Host" header and cannot send a custom value for it.', + ); + expect(guidance.body).toContain('"Host"'); + expect(guidance.focus).toBe("credentials"); + }); + + it("falls back to the server's message for an unrecognised code", () => { + expect(genericConnectGuidance("something_new", "Upstream said no.").body).toBe("Upstream said no."); + expect(genericConnectGuidance(null, null).body).toContain("Check it and try again"); + }); +}); + +describe("customHeaderError", () => { + it("accepts a filled row and ignores a blank one", () => { + expect(customHeaderError([ + { id: "a", name: "X-Api-Key", value: "abc" }, + { id: "b", name: "", value: "" }, + ])).toBeNull(); + }); + + it("rejects headers Paperclip refuses to send", () => { + expect(customHeaderError([{ id: "a", name: "Host", value: "evil.example" }])) + .toContain('Paperclip manages the "Host" header'); + }); + + it("rejects a value that would split the request", () => { + expect(customHeaderError([{ id: "a", name: "X-Api-Key", value: "abc\r\nX-Injected: 1" }])) + .toContain("line breaks"); + }); + + it("asks for a value on a half-filled row and flags duplicates", () => { + expect(customHeaderError([{ id: "a", name: "X-Api-Key", value: "" }])) + .toContain('Add a value for "X-Api-Key"'); + expect(customHeaderError([ + { id: "a", name: "X-Api-Key", value: "one" }, + { id: "b", name: "x-api-key", value: "two" }, + ])).toContain("listed twice"); + }); +}); + +describe("genericConnectPayload", () => { + it("sends nothing but the link on the simplest path", () => { + expect(genericConnectPayload(draft())).toEqual({ + link: "https://mcp.example.test/mcp", + name: "Example", + }); + }); + + it("omits authMode on the simple path so the server probes and decides", () => { + const payload = genericConnectPayload(draft({ needsKey: true, keyValue: "abc" })); + expect(payload.authMode).toBeUndefined(); + expect(payload.credentialValues).toEqual({ "credentials.authorization": "abc" }); + }); + + it("sends the explicit mode chosen under Advanced authentication", () => { + expect(genericConnectPayload(draft({ authMode: "none" }))).toMatchObject({ authMode: "none" }); + expect(genericConnectPayload(draft({ authMode: "bearer", keyValue: " abc " }))) + .toMatchObject({ authMode: "bearer", credentialValues: { "credentials.authorization": "abc" } }); + }); + + it("maps custom headers to headers.* credential paths", () => { + const payload = genericConnectPayload(draft({ + authMode: "custom_headers", + headers: [ + { id: "a", name: " X-Api-Key ", value: "phx_secret" }, + { id: "b", name: "X-Project", value: "12345" }, + { id: "c", name: "", value: "" }, + ], + })); + expect(payload.credentialValues).toEqual({ + "headers.X-Api-Key": "phx_secret", + "headers.X-Project": "12345", + }); + }); + + it("only sends a preregistered client when the operator supplied one", () => { + expect(genericConnectPayload(draft({ authMode: "oauth" })).oauthClient).toBeUndefined(); + expect(genericConnectPayload(draft({ authMode: "oauth", oauthClientId: "cid" })).oauthClient) + .toEqual({ clientId: "cid" }); + expect(genericConnectPayload(draft({ + authMode: "oauth", + oauthClientId: "cid", + oauthClientSecret: "shh", + })).oauthClient).toEqual({ clientId: "cid", clientSecret: "shh" }); + }); + + it("does not carry a bearer key into a custom-header or no-auth submission", () => { + // Switching modes must not leak a value the operator typed under a different + // one — the wizard clears it, and the payload builder does not resurrect it. + expect(genericConnectPayload(draft({ authMode: "custom_headers", keyValue: "stale" })).credentialValues) + .toBeUndefined(); + expect(genericConnectPayload(draft({ authMode: "none", keyValue: "stale" })).credentialValues) + .toBeUndefined(); + }); +}); + +describe("canSubmitGenericConnect", () => { + it("needs a link", () => { + expect(canSubmitGenericConnect(draft({ link: "" }))).toBe(false); + }); + + it("lets the simple no-key path through and holds the yes-key path until a key is typed", () => { + expect(canSubmitGenericConnect(draft())).toBe(true); + expect(canSubmitGenericConnect(draft({ needsKey: true }))).toBe(false); + expect(canSubmitGenericConnect(draft({ needsKey: true, keyValue: "abc" }))).toBe(true); + }); + + it("requires a key for the explicit bearer mode", () => { + expect(canSubmitGenericConnect(draft({ authMode: "bearer" }))).toBe(false); + expect(canSubmitGenericConnect(draft({ authMode: "bearer", keyValue: "abc" }))).toBe(true); + }); + + it("requires at least one valid header for custom-header mode", () => { + expect(canSubmitGenericConnect(draft({ authMode: "custom_headers" }))).toBe(false); + expect(canSubmitGenericConnect(draft({ + authMode: "custom_headers", + headers: [{ id: "a", name: "X-Api-Key", value: "abc" }], + }))).toBe(true); + expect(canSubmitGenericConnect(draft({ + authMode: "custom_headers", + headers: [{ id: "a", name: "Host", value: "abc" }], + }))).toBe(false); + }); + + it("allows no-auth and browser sign-in without any value", () => { + expect(canSubmitGenericConnect(draft({ authMode: "none" }))).toBe(true); + expect(canSubmitGenericConnect(draft({ authMode: "oauth" }))).toBe(true); + }); +}); diff --git a/ui/src/pages/apps/generic-mcp-connect.ts b/ui/src/pages/apps/generic-mcp-connect.ts new file mode 100644 index 00000000000..e68c560e52e --- /dev/null +++ b/ui/src/pages/apps/generic-mcp-connect.ts @@ -0,0 +1,212 @@ +import { checkMcpRemoteHeaderName, checkMcpRemoteHeaderValue, mcpRemoteHeaderRejectionMessage } from "@paperclipai/shared"; +import type { GenericMcpAuthMode } from "@paperclipai/shared"; + +/** + * Logic behind the guided "Connect your own MCP server" flow (PAP-17087). + * + * Kept out of the component so the branch an operator lands on — and the + * corrective advice they get — is directly testable, and so the wizard never has + * to reason about protocol error codes inline. + */ + +export type { GenericMcpAuthMode }; + +export interface CustomHeaderRow { + /** Stable key so React can track rows across add/remove without reordering values. */ + id: string; + name: string; + value: string; +} + +let customHeaderRowSeq = 0; + +/** A blank custom-header row. The id is local only — it never reaches the API. */ +export function newCustomHeaderRow(): CustomHeaderRow { + customHeaderRowSeq += 1; + return { id: `header-${customHeaderRowSeq}`, name: "", value: "" }; +} + +/** The endpoint host, shown prominently so the operator always sees who they are trusting. */ +export function endpointHost(url: string): string | null { + try { + return new URL(url).host || null; + } catch { + return null; + } +} + +/** + * Where the operator has to look to fix a failed connect attempt. The wizard uses + * this to decide whether to reopen the URL field or the credential section, rather + * than leaving them to guess which of the two was wrong. + */ +export type GenericConnectFocus = "url" | "credentials" | "deployment" | "none"; + +export interface GenericConnectGuidance { + title: string; + body: string; + focus: GenericConnectFocus; +} + +/** + * Turn an API failure into the specific corrective action the plan asks for: + * invalid, unsafe, unreachable, or "needs a credential we could not discover". + * + * Anything unrecognised falls through to the server's own message, which is + * already UI-safe (upstream bodies and secrets are stripped server-side). + */ +export function genericConnectGuidance( + code: string | null | undefined, + message: string | null | undefined, +): GenericConnectGuidance { + const fallback = message?.trim() || "Paperclip couldn't connect to that address. Check it and try again."; + switch (code) { + case "mcp_remote_url_missing": + case "mcp_remote_url_invalid": + return { + title: "That doesn't look like a server address", + body: "Paste the full address, starting with https:// — for example https://mcp.example.com/mcp.", + focus: "url", + }; + case "remote_http_private_endpoint": + return { + title: "That address is inside a private network", + body: "This Paperclip is reachable from the internet, so it won't call addresses on your local network. Use the server's public address instead.", + focus: "url", + }; + case "remote_http_dns_failed": + return { + title: "We couldn't find that host", + body: "The address didn't resolve. Check the spelling, or confirm the server is published on the internet.", + focus: "url", + }; + case "mcp_header_rejected": + return { + title: "Paperclip can't send that header", + body: fallback, + focus: "credentials", + }; + case "oauth_challenge": + return { + title: "This server wants a credential", + body: "It asked us to authenticate but didn't offer a sign-in Paperclip can complete on its own. Add the key or headers its docs list under Advanced authentication.", + focus: "credentials", + }; + case "oauth_manual_client_required": + case "oauth_manual_client_rebinding_required": + return { + title: "This server needs sign-in details you create yourself", + body: "Register Paperclip in the provider's settings, then add the client ID and secret it gives you under Advanced authentication.", + focus: "credentials", + }; + case "oauth_redirect_origin_unsupported": + case "oauth_redirect_uri_invalid": + return { + title: "This Paperclip needs a public HTTPS address first", + body: "Sign-in sends the operator back to Paperclip, so this instance has to be reachable over HTTPS. Ask your Paperclip admin to configure it.", + focus: "deployment", + }; + case "runtime_error": + return { + title: "We couldn't reach that server", + body: "Nothing answered at that address. Confirm the server is running and the address is right, then try again.", + focus: "url", + }; + default: + return { + title: "Paperclip couldn't connect", + body: fallback, + focus: "none", + }; + } +} + +/** + * First client-side complaint about the custom headers, or null when they're fine. + * The API validates these too; checking here means the operator finds out before a + * round trip rather than after one. + */ +export function customHeaderError(rows: CustomHeaderRow[]): string | null { + const seen = new Set(); + for (const row of rows) { + const name = row.name.trim(); + if (!name && !row.value.trim()) continue; + const nameCheck = checkMcpRemoteHeaderName(name); + if (!nameCheck.ok) return mcpRemoteHeaderRejectionMessage(name, nameCheck.reason!); + const valueCheck = checkMcpRemoteHeaderValue(row.value); + if (!valueCheck.ok) return mcpRemoteHeaderRejectionMessage(name, valueCheck.reason!); + if (!row.value.trim()) return `Add a value for "${name}", or remove the row.`; + const lower = name.toLowerCase(); + if (seen.has(lower)) return `"${name}" is listed twice.`; + seen.add(lower); + } + return null; +} + +export interface GenericConnectDraft { + link: string; + name: string; + authMode: GenericMcpAuthMode; + /** Simple path: the operator answered "Yes" to "does it need a key?". */ + needsKey: boolean; + keyValue: string; + headers: CustomHeaderRow[]; + oauthClientId: string; + oauthClientSecret: string; +} + +export interface GenericConnectPayload { + link: string; + name?: string; + authMode?: GenericMcpAuthMode; + credentialValues?: Record; + oauthClient?: { clientId: string; clientSecret?: string }; +} + +/** + * Build the connect request from wizard state. + * + * `authMode` is only sent when the operator made an explicit choice under Advanced + * authentication; the simple path leaves it off so the server probes and decides. + * Header values become `headers.` credential paths, which the server turns + * into Paperclip secrets — nothing here ever puts a value in the config. + */ +export function genericConnectPayload(draft: GenericConnectDraft): GenericConnectPayload { + const credentialValues: Record = {}; + const bearerKey = draft.keyValue.trim(); + const useBearer = draft.authMode === "bearer" || (draft.authMode === "auto" && draft.needsKey); + if (useBearer && bearerKey) credentialValues["credentials.authorization"] = bearerKey; + if (draft.authMode === "custom_headers") { + for (const row of draft.headers) { + const name = row.name.trim(); + if (!name || !row.value.trim()) continue; + credentialValues[`headers.${name}`] = row.value; + } + } + const trimmedName = draft.name.trim(); + const clientId = draft.oauthClientId.trim(); + const clientSecret = draft.oauthClientSecret.trim(); + return { + link: draft.link, + ...(trimmedName ? { name: trimmedName } : {}), + ...(draft.authMode === "auto" ? {} : { authMode: draft.authMode }), + ...(Object.keys(credentialValues).length > 0 ? { credentialValues } : {}), + ...(draft.authMode === "oauth" && clientId + ? { oauthClient: { clientId, ...(clientSecret ? { clientSecret } : {}) } } + : {}), + }; +} + +/** Can "Check link" be pressed? */ +export function canSubmitGenericConnect(draft: GenericConnectDraft): boolean { + if (!draft.link.trim()) return false; + if (draft.authMode === "auto") return !draft.needsKey || draft.keyValue.trim().length > 0; + if (draft.authMode === "bearer") return draft.keyValue.trim().length > 0; + if (draft.authMode === "custom_headers") { + const filled = draft.headers.filter((row) => row.name.trim() && row.value.trim()); + return filled.length > 0 && customHeaderError(draft.headers) === null; + } + // "oauth" here means the operator is supplying a preregistered client, and + // "none" means they are asserting the server is public — neither needs a value. + return true; +}