diff --git a/server/src/services/paperclip-cloud-connector.test.ts b/server/src/services/paperclip-cloud-connector.test.ts index 3540a3baa8..a0319b1264 100644 --- a/server/src/services/paperclip-cloud-connector.test.ts +++ b/server/src/services/paperclip-cloud-connector.test.ts @@ -93,6 +93,54 @@ describe("Paperclip Cloud connector", () => { }); }); + it("prefers a validated HTTPS provider URL over the legacy confirmation URL", async () => { + const keys = config(); + const connector = createPaperclipCloudConnector({ + config: keys.config, + request: vi.fn(async () => Response.json({ + confirmationUrl: "https://my.example.test/connections/confirm?session=broker-state", + authorizationUrl: "https://github.com/login/oauth/authorize?client_id=client&state=broker-state", + expiresAt: "2099-08-21T20:00:00.000Z", + })) as typeof fetch, + }); + + await expect(connector.startAuthorization({ + subject, + companyId, + profile: "github.code", + returnUri: "https://paperclip.example.test/api/tools/oauth/cloud-connector/callback", + returnState: "state-direct", + })).resolves.toMatchObject({ + authorizationUrl: "https://github.com/login/oauth/authorize?client_id=client&state=broker-state", + }); + }); + + it.each([ + ["non-string", { href: "https://github.com/login/oauth/authorize" }], + ["plaintext HTTP", "http://github.com/login/oauth/authorize"], + ["embedded credentials", "https://user:password@github.com/login/oauth/authorize"], + ["fragment", "https://github.com/login/oauth/authorize#unexpected"], + ["not a URL", "not-a-url"], + ])("rejects a malformed direct provider URL: %s", async (_label, authorizationUrl) => { + const keys = config(); + const connector = createPaperclipCloudConnector({ + config: keys.config, + request: vi.fn(async () => Response.json({ + confirmationUrl: "https://my.example.test/connections/confirm?session=broker-state", + authorizationUrl, + expiresAt: "2099-08-21T20:00:00.000Z", + })) as typeof fetch, + }); + + await expect(connector.startAuthorization({ + subject, + companyId, + profile: "github.code", + returnUri: "https://paperclip.example.test/api/tools/oauth/cloud-connector/callback", + returnState: "state-malformed-direct", + })).rejects.toMatchObject({ code: "CONNECTOR_BAD_RESPONSE" }); + }); + it("binds active GitHub installations to proof from the current user token", async () => { const keys = config(); const request = vi.fn(async (_url: string | URL | Request, init?: RequestInit) => { diff --git a/server/src/services/paperclip-cloud-connector.ts b/server/src/services/paperclip-cloud-connector.ts index cac5782a08..1932142414 100644 --- a/server/src/services/paperclip-cloud-connector.ts +++ b/server/src/services/paperclip-cloud-connector.ts @@ -93,6 +93,7 @@ type SealedEnvelope = { type ConnectorResponse = { confirmationUrl?: unknown; + authorizationUrl?: unknown; handoff?: unknown; expiresAt?: unknown; scopes?: unknown; @@ -348,14 +349,44 @@ export function createPaperclipCloudConnector(input: { if (typeof response.confirmationUrl !== "string" || typeof response.expiresAt !== "string") { throw new PaperclipCloudConnectorError("Paperclip Cloud connector returned an invalid session", "CONNECTOR_BAD_RESPONSE"); } - const confirmationUrl = new URL(response.confirmationUrl); - const expectedBroker = new URL(config.baseUrl); - if (confirmationUrl.origin !== expectedBroker.origin || confirmationUrl.pathname !== "/connections/confirm") { + let confirmationUrl: URL; + try { + confirmationUrl = new URL(response.confirmationUrl); + } catch { throw new PaperclipCloudConnectorError("Paperclip Cloud connector returned an invalid confirmation URL", "CONNECTOR_BAD_RESPONSE"); } + const expectedBroker = new URL(config.baseUrl); + if ( + confirmationUrl.origin !== expectedBroker.origin + || confirmationUrl.pathname !== "/connections/confirm" + || confirmationUrl.username + || confirmationUrl.password + || confirmationUrl.hash + ) { + throw new PaperclipCloudConnectorError("Paperclip Cloud connector returned an invalid confirmation URL", "CONNECTOR_BAD_RESPONSE"); + } + let authorizationUrl: URL | undefined; + if (response.authorizationUrl !== undefined) { + if (typeof response.authorizationUrl !== "string") { + throw new PaperclipCloudConnectorError("Paperclip Cloud connector returned an invalid provider URL", "CONNECTOR_BAD_RESPONSE"); + } + try { + authorizationUrl = new URL(response.authorizationUrl); + } catch { + throw new PaperclipCloudConnectorError("Paperclip Cloud connector returned an invalid provider URL", "CONNECTOR_BAD_RESPONSE"); + } + if ( + authorizationUrl.protocol !== "https:" + || authorizationUrl.username + || authorizationUrl.password + || authorizationUrl.hash + ) { + throw new PaperclipCloudConnectorError("Paperclip Cloud connector returned an invalid provider URL", "CONNECTOR_BAD_RESPONSE"); + } + } const handoff = parseCloudHandoff(response.handoff); return { - authorizationUrl: confirmationUrl.toString(), + authorizationUrl: authorizationUrl?.toString() ?? confirmationUrl.toString(), expiresAt: response.expiresAt, ...(handoff ? { handoff } : {}), }; diff --git a/ui/src/features/connections/ConnectionSetupFlow.tsx b/ui/src/features/connections/ConnectionSetupFlow.tsx index b769600b66..194ff62588 100644 --- a/ui/src/features/connections/ConnectionSetupFlow.tsx +++ b/ui/src/features/connections/ConnectionSetupFlow.tsx @@ -124,6 +124,19 @@ const ROUTE_STAGE_BY_STEP: Partial> = { success: "complete", }; +export function requestedConnectionInitialStep(input: { + requestedAppKey: string | undefined; + routeStage: string | null; + resumeConnectionId: string | null; + hasPrefilledLink: boolean; + zapierSource: boolean; +}): Step { + if (input.requestedAppKey) { + return input.resumeConnectionId || input.routeStage === "setup" ? "key" : "access"; + } + return input.hasPrefilledLink || input.zapierSource ? "access" : "gallery"; +} + export function requestedConnectionEntry(input: { requestedAppKey: string; galleryApps: readonly AppDefinition[]; @@ -440,6 +453,7 @@ export function ConnectionSetupFlow({ const appKey = routeParams.appKey ?? searchParams.get("appKey") ?? undefined; const sourceSlug = searchParams.get("source")?.trim() || null; const createNewConnection = searchParams.get("new") === "1"; + const routeStage = searchParams.get("stage")?.trim() || null; const resumeConnectionId = searchParams.get("resume")?.trim() || null; const oauthCallbackOutcome = searchParams.get("oauth"); const oauthCallbackCode = searchParams.get("code"); @@ -476,11 +490,13 @@ export function ConnectionSetupFlow({ }; }); - const [step, setStep] = useState( - requestedAppKey - ? resumeConnectionId ? "key" : "access" - : prefill.link || zapierSource ? "access" : "gallery", - ); + const [step, setStep] = useState(() => requestedConnectionInitialStep({ + requestedAppKey, + routeStage, + resumeConnectionId, + hasPrefilledLink: Boolean(prefill.link), + zapierSource, + })); const [entry, setEntry] = useState(null); const [galleryName, setGalleryName] = useState(""); const [linkUrl, setLinkUrl] = useState(prefill.link); @@ -1170,7 +1186,13 @@ export function ConnectionSetupFlow({ // Route/service selection initializes the wizard once. Later renders must // preserve the user's current step in both hosts instead of snapping back // to Access after they continue. - setStep(resumeConnectionId ? "key" : "access"); + setStep(requestedConnectionInitialStep({ + requestedAppKey, + routeStage, + resumeConnectionId, + hasPrefilledLink: Boolean(prefill.link), + zapierSource, + })); } if (automaticOAuth && ( @@ -1202,6 +1224,8 @@ export function ConnectionSetupFlow({ resumeConnectionId, requestedAppKey, requestedAgentId, + routeStage, + zapierSource, ]); // Resume the exact method and non-secret provider configuration that the @@ -1781,43 +1805,48 @@ export function ConnectionSetupFlow({ {step === "key" && entry && showConnectorEnrollmentStep ? (
-
-
- +
+
+
+ +
+
+

+ Connect with Paperclip +

+

+ You must connect this instance to Paperclip to connect to {entry.name} (you only need to do this once). +

+
-
-

- Connect with Paperclip -

+ + {connectorEnrollmentQuery.isError || connectorEnrollmentError ? ( + + {connectorEnrollmentError ?? "Paperclip couldn’t check Cloud registration. Try again."} + + ) : null} + +
+ +
- - {connectorEnrollmentQuery.isError || connectorEnrollmentError ? ( - - {connectorEnrollmentError ?? "Paperclip couldn’t check Cloud registration. Try again."} - - ) : null} - -
- - -
) : step === "key" && entry ? (
@@ -3449,7 +3480,7 @@ export function AccessStep({ queryFn: () => agentsApi.list(companyId), }); const allAgents: Agent[] = (agentsQuery.data ?? []).filter((a) => a.status !== "terminated"); - // "Just agents I pick" means agents this person may actually edit. When the server + // "Only agents I choose" / "Just agents I pick" means agents this person may actually edit. When the server // has not told us, fall back to every live agent rather than an empty list — // an empty picker would read as "you have no agents". const editableAgentIds = capabilities?.editableAgentIds; @@ -3479,13 +3510,22 @@ export function AccessStep({ const lockedAgentName = lockedAgentId ? allAgents.find((agent) => agent.id === lockedAgentId)?.name ?? "the requesting agent" : null; + const identityHeading = githubIdentity ? "Connect GitHub as" : "Which humans can use this credential?"; + const agentAccessHeading = grantKind === "agent" + ? "Which agent owns this GitHub account?" + : githubIdentity && grantKind === "user" + ? "Which agents may use your GitHub when you’re responsible?" + : githubIdentity + ? "Which agents may use the shared GitHub account?" + : "Which agents can use this connection?"; + const agentAccessLabel = githubIdentity ? agentAccessHeading : "Which agents can use this connection?"; return (
-

{githubIdentity ? "Which GitHub identity should this use?" : "Which humans can use this credential?"}

+

{identityHeading}

{identityLoading ? (
@@ -3500,17 +3540,28 @@ export function AccessStep({ ) : (
-

{grantKind === "agent" ? "Which agent owns this GitHub account?" : "Which agents can use this connection?"}

+

{agentAccessHeading}

{preserveAgentAccess ? (