Use saved OAuth apps for MCP connections

.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.

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* () {
@@ -262,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 })
@@ -288,3 +294,126 @@ 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* () {
+            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" } })
+              .pipe(Effect.ignore);
+          }).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 } : {}),
+          },
+        });
+        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;
+            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 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");
+            // 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/ })
+              .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"),
+          "connecting must reuse the saved app",
+        ).toHaveLength(0);
+      }),
+    ),
+  );
+}

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 });

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)),

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);
+    });
+  }
+});

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(),