From dab9e295e26aafebe59a599c564a9b1443b1f180 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 18 Sep 2026 11:13:10 -0700 Subject: [PATCH 1/4] Reuse saved OAuth apps for MCP connections --- .changeset/connection-saved-mcp-app.md | 5 + e2e/scenarios/connection-setup-ux.test.ts | 105 +++++++++++++++++- packages/core/api/src/oauth/api.ts | 5 +- .../src/components/add-account-modal.test.ts | 64 +++++++++++ .../src/components/add-account-modal.tsx | 37 +++++- 5 files changed, 208 insertions(+), 8 deletions(-) create mode 100644 .changeset/connection-saved-mcp-app.md diff --git a/.changeset/connection-saved-mcp-app.md b/.changeset/connection-saved-mcp-app.md new file mode 100644 index 0000000000..941fd81038 --- /dev/null +++ b/.changeset/connection-saved-mcp-app.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Reuse saved OAuth apps for MCP connections without requiring another registration or click. diff --git a/e2e/scenarios/connection-setup-ux.test.ts b/e2e/scenarios/connection-setup-ux.test.ts index fa87ea93d6..6ba54e361e 100644 --- a/e2e/scenarios/connection-setup-ux.test.ts +++ b/e2e/scenarios/connection-setup-ux.test.ts @@ -1,8 +1,9 @@ import { randomBytes } from "node:crypto"; import { expect } from "@effect/vitest"; -import { Effect } from "effect"; +import { Effect, Schema } from "effect"; import { composePluginApi } from "@executor-js/api/server"; import { connectEmulator } from "@executor-js/emulate"; +import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; import { IntegrationSlug, OAuthClientSlug } from "@executor-js/sdk/shared"; import { variable } from "@executor-js/sdk/http-auth"; @@ -12,6 +13,10 @@ import { Api, Browser, Target } from "../src/services"; import { hydrated, visit } from "../src/surfaces/browser"; const api = composePluginApi([openApiHttpPlugin()] as const); +const decodeRegisteredClient = Schema.decodeUnknownSync( + Schema.Struct({ client_id: Schema.String, client_secret: Schema.String }), +); + // Each journey has its own real provider state, OAuth app, user and integration. const connectionFixture = (registerClient: boolean) => Effect.gen(function* () { @@ -288,3 +293,101 @@ scenario( }), ), ); +for (const origin of ["integration", "workspace"] as const) { + scenario( + origin === "integration" + ? "Connection setup · a saved MCP app opens sign-in on the first click" + : "Connection setup · discovery reuses a workspace OAuth app without another click", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(composePluginApi([mcpHttpPlugin()] as const), identity); + const base = yield* createEmulatorInstance("mcp", "saved-app"); + const emulator = yield* Effect.promise(() => connectEmulator({ baseUrl: base })); + const slug = IntegrationSlug.make(`saved-app-${randomBytes(4).toString("hex")}`); + const app = OAuthClientSlug.make(`${slug}-client`); + const registered = yield* Effect.promise(async () => { + const response = await fetch(`${base}/register`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + client_name: "Saved app", + redirect_uris: [new URL("/api/oauth/callback", target.baseUrl).toString()], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "client_secret_post", + }), + }); + expect(response.status).toBe(201); + return decodeRegisteredClient(await response.json()); + }); + yield* Effect.addFinalizer(() => + Effect.gen(function* () { + yield* client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore); + yield* client.oauth + .removeClient({ params: { slug: app }, payload: { owner: "org" } }) + .pipe(Effect.ignore); + }), + ); + yield* client.mcp.addServer({ + payload: { + transport: "remote", + slug, + name: "Team MCP", + endpoint: `${base}/mcp`, + authenticationTemplate: [{ kind: "oauth2" }], + }, + }); + yield* client.oauth.createClient({ + payload: { + owner: "org", + slug: app, + grant: "authorization_code", + clientId: registered.client_id, + clientSecret: registered.client_secret, + authorizationUrl: `${base}/authorize`, + tokenUrl: `${base}/token`, + resource: base, + ...(origin === "integration" ? { originIntegration: slug } : {}), + }, + }); + const savedClients = yield* client.oauth.listClients({}); + expect(savedClients.find((saved) => saved.slug === app)?.origin).toEqual( + origin === "integration" + ? { kind: "manual", integration: slug } + : { kind: "manual", integration: null }, + ); + yield* Effect.promise(() => emulator.ledger.clear()); + yield* browser.session(identity, async ({ page, step }) => { + await step("Open an integration that already has a saved OAuth app", async () => { + await visit(page, `/integrations/${slug}?addAccount=1`); + await page.getByRole("tab", { name: "OAuth", exact: true }).waitFor(); + const opened = page.waitForEvent("popup"); + await page.getByRole("button", { name: /^Connect(?: with OAuth)?$/ }).click(); + const popup = await opened; + await popup.waitForURL(/\/authorize/); + expect( + new URL(popup.url()).searchParams.get("client_id"), + "the existing app is reused without another registration step", + ).toBe(registered.client_id); + await popup.close(); + await page + .getByRole("dialog") + .getByRole("button", { name: "Close", exact: true }) + .last() + .click(); + }); + }); + const ledger = yield* Effect.promise(() => emulator.ledger.list()); + expect( + ledger.filter((entry) => entry.method === "POST" && entry.path === "/register"), + "connecting must reuse the saved app", + ).toHaveLength(0); + }), + ), + ); +} diff --git a/packages/core/api/src/oauth/api.ts b/packages/core/api/src/oauth/api.ts index 5d0eee3d41..1389683d98 100644 --- a/packages/core/api/src/oauth/api.ts +++ b/packages/core/api/src/oauth/api.ts @@ -121,7 +121,10 @@ const OAuthClientSummaryResponse = Schema.Struct({ clientId: Schema.String, tokenEndpointAuthMethod: Schema.optional(TokenEndpointAuthMethodSchema), origin: Schema.Union([ - Schema.Struct({ kind: Schema.Literal("manual") }), + Schema.Struct({ + kind: Schema.Literal("manual"), + integration: Schema.optional(Schema.NullOr(IntegrationSlug)), + }), Schema.Struct({ kind: Schema.Literal("dynamic_client_registration"), integration: Schema.optional(Schema.NullOr(IntegrationSlug)), diff --git a/packages/react/src/components/add-account-modal.test.ts b/packages/react/src/components/add-account-modal.test.ts index 90558590e6..385532cf75 100644 --- a/packages/react/src/components/add-account-modal.test.ts +++ b/packages/react/src/components/add-account-modal.test.ts @@ -1505,3 +1505,67 @@ describe("preferredMethodId", () => { expect(preferredMethodId([], [], integration)).toBe(""); }); }); + +describe("discovered saved OAuth apps", () => { + const cases = [ + { + variation: "shared", + outcome: { kind: "started", flow: "byo" }, + calls: [{ client: "saved", owner: "org", reservation: RESERVED }], + }, + { variation: "other-owner", outcome: { kind: "fallback" }, calls: [] }, + { variation: "other-endpoint", outcome: { kind: "fallback" }, calls: [] }, + { variation: "other-resource", outcome: { kind: "fallback" }, calls: [] }, + ] as const; + for (const { variation, outcome: expectedOutcome, calls } of cases) { + it(`handles a ${variation} app without changing the connection owner`, async () => { + const started: StartArgs[] = []; + const outcome = await runAutomaticOAuthConnect( + { + ...popupSpy(), + isActive: () => true, + probe: async () => ({ + authorizationUrl: "https://auth.example/authorize", + tokenUrl: "https://auth.example/token", + resource: "https://api.example/mcp", + }), + createCimdClient: async () => null, + register: async () => null, + start: (args) => { + started.push(args); + }, + }, + { + owner: variation === "other-owner" ? "org" : "user", + integration: TEST_INTEGRATION, + discoveryUrl: "https://api.example/mcp", + registeredClients: [ + { + owner: variation === "other-owner" ? "user" : "org", + slug: OAuthClientSlug.make("saved"), + grant: "authorization_code", + clientId: "client", + origin: { kind: "manual", integration: null }, + authorizationUrl: "https://auth.example/authorize", + tokenUrl: + variation === "other-endpoint" + ? "https://other.example/token" + : "https://auth.example/token", + resource: + variation === "other-resource" + ? "https://other.example/mcp" + : "https://api.example/mcp", + }, + ], + cimd: { + integrationName: "MCP", + clientIdMetadataDocumentUrl: "https://app.example/client.json", + existingClients: [], + }, + }, + ); + expect(outcome).toMatchObject(expectedOutcome); + expect(started).toEqual(calls); + }); + } +}); diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index 222dd0f682..e5228cbd68 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -882,7 +882,7 @@ type DcrStartArgs = { * carries no probe result; the other two reasons always carry the probe that * seeds the picker. */ type AutomaticOAuthOutcome = - | { readonly kind: "started"; readonly flow: "cimd" | "dcr" } + | { readonly kind: "started"; readonly flow: "cimd" | "dcr" | "byo" } | { readonly kind: "popup-blocked" } /** The owning surface went away mid-flight (`isActive` turned false): the * sequence stopped before its next side effect and released the window. @@ -927,6 +927,8 @@ type RunAutomaticOAuthConnectDeps = { }; type RunAutomaticOAuthConnectInput = { + /** Saved apps available for a fresh connection; omitted for reconnects. */ + readonly registeredClients?: readonly OAuthClientSummary[]; readonly discoveryUrl: string; /** The integration's genuine protected-resource URL (the MCP discovery URL), * used as the RFC 8707 resource indicator when the server's PRM names no @@ -1022,6 +1024,22 @@ export async function runAutomaticOAuthConnect( ? null : (probe.resource ?? input.storedResource) : (probe.resource ?? input.resourceFallback ?? null); + // A workspace app may have been registered outside this dialog. Once + // discovery establishes the exact endpoints, reuse it without a second + // click. Never select a near match or another owner's personal app. + const savedClient = input.registeredClients?.find( + (client) => + client.origin.kind === "manual" && + client.grant === "authorization_code" && + (client.owner === input.owner || client.owner === "org") && + client.authorizationUrl === probe.authorizationUrl && + client.tokenUrl === probe.tokenUrl && + (client.resource == null || client.resource === resource), + ); + if (savedClient) { + deps.start({ client: savedClient.slug, owner: savedClient.owner, reservation }); + return { kind: "started", flow: "byo" }; + } if (probe.clientIdMetadataDocumentSupported === true) { const resolved = await resolveCimdClient( { createClient: deps.createCimdClient }, @@ -1788,7 +1806,15 @@ function AddAccountModalView(props: AddAccountModalProps) { // DCR-capable (see `hasDcr`). When DCR-capable and not yet fallen back, we // skip the app picker entirely (Option A). const isDcr = !cimdActive && hasDcr(method); - const dcrActive = isDcr && !dcrFailed; + // Reuse an app explicitly registered for this integration. Discovery must + // not send its users through automatic registration again on every connect. + const hasSavedOAuthApp = clientSummaries.some( + (client) => + client.grant === "authorization_code" && + client.origin.kind === "manual" && + client.origin.integration === integration, + ); + const dcrActive = isDcr && !dcrFailed && !hasSavedOAuthApp; const automaticOAuthActive = cimdActive || dcrActive; // OAuth apps usable for this integration (user-owned first). Hooks run @@ -2493,8 +2519,8 @@ function AddAccountModalView(props: AddAccountModalProps) { reservation: args.reservation, payload: { client: args.client, - // DCR/CIMD mints the client under the connection owner, so the - // app and connection share one owner. + // Discovery may reuse a shared app for a Personal connection. + // Keep the app owner separate from the requested connection owner. clientOwner: args.owner, owner: dcrOwner, name: request.connectionName, @@ -2542,11 +2568,10 @@ function AddAccountModalView(props: AddAccountModalProps) { // not, so pass the un-collapsed method value here. resourceFallback: requestMethod.oauth?.discoveryUrl, owner: dcrOwner, - // DCR slugs are server-keyed (Part A): the connect path no longer depends - // on the picker's app list, so it need not be threaded here. declaredScopes: requestMethod.oauth?.scopes, redirectUri: oauthCallbackUrl(), integration, + ...(reconnect ? {} : { registeredClients: clientSummaries }), cimd: { integrationName, clientIdMetadataDocumentUrl: oauthClientIdMetadataDocumentUrl(), From 41a4d2d857e7f1bc04cf427e07df6983a62ba32a Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:32:01 -0700 Subject: [PATCH 2/4] Verify saved MCP apps through provider consent --- e2e/scenarios/connection-setup-ux.test.ts | 40 ++++++++++++++++------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/e2e/scenarios/connection-setup-ux.test.ts b/e2e/scenarios/connection-setup-ux.test.ts index 6ba54e361e..1939a074be 100644 --- a/e2e/scenarios/connection-setup-ux.test.ts +++ b/e2e/scenarios/connection-setup-ux.test.ts @@ -327,6 +327,14 @@ for (const origin of ["integration", "workspace"] as const) { }); yield* Effect.addFinalizer(() => Effect.gen(function* () { + const connections = yield* client.connections.list({ query: { integration: slug } }); + for (const connection of connections) { + yield* client.connections + .remove({ + params: { owner: connection.owner, integration: slug, name: connection.name }, + }) + .pipe(Effect.ignore); + } yield* client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore); yield* client.oauth .removeClient({ params: { slug: app }, payload: { owner: "org" } }) @@ -355,17 +363,13 @@ for (const origin of ["integration", "workspace"] as const) { ...(origin === "integration" ? { originIntegration: slug } : {}), }, }); - const savedClients = yield* client.oauth.listClients({}); - expect(savedClients.find((saved) => saved.slug === app)?.origin).toEqual( - origin === "integration" - ? { kind: "manual", integration: slug } - : { kind: "manual", integration: null }, - ); yield* Effect.promise(() => emulator.ledger.clear()); yield* browser.session(identity, async ({ page, step }) => { await step("Open an integration that already has a saved OAuth app", async () => { await visit(page, `/integrations/${slug}?addAccount=1`); await page.getByRole("tab", { name: "OAuth", exact: true }).waitFor(); + }); + await step("Connect once using the saved app", async () => { const opened = page.waitForEvent("popup"); await page.getByRole("button", { name: /^Connect(?: with OAuth)?$/ }).click(); const popup = await opened; @@ -374,14 +378,28 @@ for (const origin of ["integration", "workspace"] as const) { new URL(popup.url()).searchParams.get("client_id"), "the existing app is reused without another registration step", ).toBe(registered.client_id); - await popup.close(); + }); + await step("Approve provider sign-in and save the connection", async () => { + const popup = page + .context() + .pages() + .find((candidate) => candidate !== page); + if (!popup) throw new Error("Provider sign-in window was not open"); + await popup.getByRole("button", { name: /admin/ }).click(); await page - .getByRole("dialog") - .getByRole("button", { name: "Close", exact: true }) - .last() - .click(); + .getByRole("heading", { name: /Add connection/ }) + .waitFor({ state: "hidden", timeout: 30_000 }); }); }); + const connections = yield* client.connections.list({ query: { integration: slug } }); + expect(connections, "provider consent saves the connection").toHaveLength(1); + expect(connections[0]?.owner, "a shared app keeps the connection personal").toBe("user"); + const savedClients = yield* client.oauth.listClients({}); + expect(savedClients.find((saved) => saved.slug === app)?.origin).toEqual( + origin === "integration" + ? { kind: "manual", integration: slug } + : { kind: "manual", integration: null }, + ); const ledger = yield* Effect.promise(() => emulator.ledger.list()); expect( ledger.filter((entry) => entry.method === "POST" && entry.path === "/register"), From 5c18fde6437e46bcfb0a26df4490339456fd23df Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:36:55 -0700 Subject: [PATCH 3/4] Correct MCP consent fixture and async test readiness --- e2e/scenarios/connection-setup-ux.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/e2e/scenarios/connection-setup-ux.test.ts b/e2e/scenarios/connection-setup-ux.test.ts index 1939a074be..d2bcb566b1 100644 --- a/e2e/scenarios/connection-setup-ux.test.ts +++ b/e2e/scenarios/connection-setup-ux.test.ts @@ -267,6 +267,7 @@ scenario( await step("Add a connection with both a token and a registered sign-in app", async () => { await visit(page, `/integrations/${slug}?addAccount=1`); await page.getByRole("tab", { name: "OAuth2", exact: true }).waitFor(); + await page.getByRole("tab", { name: "OAuth2", exact: true, selected: true }).waitFor(); expect( await page .getByRole("tab", { name: "OAuth2", exact: true }) @@ -339,7 +340,7 @@ for (const origin of ["integration", "workspace"] as const) { yield* client.oauth .removeClient({ params: { slug: app }, payload: { owner: "org" } }) .pipe(Effect.ignore); - }), + }).pipe(Effect.ignore), ); yield* client.mcp.addServer({ payload: { @@ -385,6 +386,13 @@ for (const origin of ["integration", "workspace"] as const) { .pages() .find((candidate) => candidate !== page); if (!popup) throw new Error("Provider sign-in window was not open"); + // The published MCP consent form omits its selected user's login. + // Keep the real provider exchange; forward the identity clicked below. + await popup.route(`${base}/authorize/approve`, (route) => { + const body = new URLSearchParams(route.request().postData() ?? ""); + body.set("login", "admin"); + return route.continue({ postData: body.toString() }); + }); await popup.getByRole("button", { name: /admin/ }).click(); await page .getByRole("heading", { name: /Add connection/ }) From 262c76c71ad5996dd00e1233a3e29c128b08a145 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:44:48 -0700 Subject: [PATCH 4/4] Open saved app directly in resource indicator regression test --- e2e/selfhost/oauth-resource-indicator-clear.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/e2e/selfhost/oauth-resource-indicator-clear.test.ts b/e2e/selfhost/oauth-resource-indicator-clear.test.ts index e494436773..38876be4a0 100644 --- a/e2e/selfhost/oauth-resource-indicator-clear.test.ts +++ b/e2e/selfhost/oauth-resource-indicator-clear.test.ts @@ -212,15 +212,13 @@ scenario( expect(saved, "the browser-registered app is in the catalog").toBeDefined(); expect(saved?.resource ?? null, "a cleared resource persists as absent").toBeNull(); - // Reopening the form: the cleared field STAYS empty. A DCR-capable method - // only shows the app picker after automatic setup falls back, so take the - // same path a returning user would. + // Reopening the form: the cleared field STAYS empty. The saved app is + // available immediately, without repeating automatic registration. yield* browser.session(identity, async ({ page, step }) => { - await step("Reach the app picker again through the failed automatic setup", async () => { + await step("Reopen the saved app without repeating automatic registration", async () => { await visit(page, `/integrations/${String(slug)}`); await page.getByRole("button", { name: "Add connection" }).first().click(); await page.getByRole("heading", { name: /Add connection/ }).waitFor(); - await page.getByRole("button", { name: "Connect", exact: true }).click(); await page .getByRole("button", { name: `Actions for ${appName}` }) .waitFor({ timeout: 30_000 });