Unpacks issue #1120 into four distinct problems and fixes each: 1. Cross-integration OAuth-app leak: the "OAuth App" picker showed apps from other integrations (Datadog clients listed under Atlassian) because a discovery-only OAuth method exposes no endpoints, hitting the "no endpoints -> show all apps" branch and never consulting origin.integration. Match apps by origin.integration first, falling back to endpoint-domain matching only when endpoints are declared. Every app (DCR and manual) now records the integration it was registered for, threaded sdk -> api -> react. 2. Duplicate DCR clients ("Datadog 2..10"): runDcrConnect minted a fresh oauth_client on every connect. Reuse an existing DCR client for the same integration + owner + discovered token host before minting. 3. `executor web --scope` silently ignored without --foreground: it opens the already-running service, which keeps its own scope. Now warns loudly and names the flag that applies it. 4. opencode HTTP MCP 401: the local /mcp is bearer-gated with no OAuth discovery, so opencode's auto-OAuth chokes on a plain 401. The --foreground ready output now prints a copy-pasteable opencode config (Authorization: Bearer + oauth: false). Tests: new e2e oauth-picker-cross-integration (selfhost) and http-mcp-bearer (local); extended cli-web-transition and add-account-modal unit guards.
Unified · Split
| @@ -981,6 +981,25 @@ const runForegroundSession = (input: { · 980 unmodified lines | |||
| 981 | console.log(`Web: ${baseUrl}`); | 981 | console.log(`Web: ${baseUrl}`); |
| 982 | console.log(`MCP: ${baseUrl}/mcp`); | 982 | console.log(`MCP: ${baseUrl}/mcp`); |
| 983 | console.log(`OpenAPI: ${baseUrl}/api/docs`); | 983 | console.log(`OpenAPI: ${baseUrl}/api/docs`); |
| 984 | // The HTTP /mcp endpoint is bearer-gated, and external agents have no way | ||
| 985 | // to discover the token (there is no OAuth server on the local app, so a | ||
| 986 | // client that tries OAuth auto-detection just errors). Surface the exact | ||
| 987 | // header — and a ready opencode block that pins `oauth: false` so it sends | ||
| 988 | // the header instead of probing for an authorization server. | ||
| 989 | console.log(`\nConnect an HTTP MCP client (e.g. opencode):`); | ||
| 990 | console.log(` Header: Authorization: Bearer ${server.authToken}`); | ||
| 991 | console.log( | ||
| 992 | ` opencode.json: ${JSON.stringify({ | ||
| 993 | mcp: { | ||
| 994 | executor: { | ||
| 995 | type: "remote", | ||
| 996 | url: `${baseUrl}/mcp`, | ||
| 997 | headers: { Authorization: `Bearer ${server.authToken}` }, | ||
| 998 | oauth: false, | ||
| 999 | }, | ||
| 1000 | }, | ||
| 1001 | })}`, | ||
| 1002 | ); | ||
| 984 | if (input.hostname !== "127.0.0.1" && input.hostname !== "localhost") { | 1003 | if (input.hostname !== "127.0.0.1" && input.hostname !== "localhost") { |
| 985 | console.log( | 1004 | console.log( |
| 986 | `\n⚠ Listening on ${input.hostname}. Executor runs arbitrary commands — only expose on trusted networks.`, | 1005 | `\n⚠ Listening on ${input.hostname}. Executor runs arbitrary commands — only expose on trusted networks.`, |
| @@ -2352,6 +2371,19 @@ const webCommand = Command.make( · 1365 unmodified lines | |||
| 2352 | ({ foreground, port, scope, hostname, allowedHost, authToken }) => | 2371 | ({ foreground, port, scope, hostname, allowedHost, authToken }) => |
| 2353 | Effect.gen(function* () { | 2372 | Effect.gen(function* () { |
| 2354 | if (!foreground) { | 2373 | if (!foreground) { |
| 2374 | // `--scope` can ONLY be honored by a foreground server we boot here. | ||
| 2375 | // Without `--foreground` we just open whatever background service is | ||
| 2376 | // already running, which uses the scope IT was started with — so a | ||
| 2377 | // `--scope` here would be silently ignored and the user could land on a | ||
| 2378 | // different workspace ("where did my config go?"). Say so loudly and | ||
| 2379 | // point at the flag that actually applies it. | ||
| 2380 | if (Option.isSome(scope)) { | ||
| 2381 | console.warn( | ||
| 2382 | `Ignoring --scope ${scope.value}: it only applies with --foreground. ` + | ||
| 2383 | `The running web app uses the scope it was started with. ` + | ||
| 2384 | `Run \`executor web --foreground --scope ${scope.value}\` to serve that workspace.`, | ||
| 2385 | ); | ||
| 2386 | } | ||
| 2355 | yield* openRunningLocalWebApp(); | 2387 | yield* openRunningLocalWebApp(); |
| 2356 | return; | 2388 | return; |
| 2357 | } | 2389 | } |
| @@ -55,3 +55,35 @@ scenario( · 54 unmodified lines | |||
| 55 | } | 55 | } |
| 56 | }), | 56 | }), |
| 57 | ); | 57 | ); |
| 58 | |||
| 59 | scenario( | ||
| 60 | "CLI web · --scope without --foreground warns instead of silently ignoring it", | ||
| 61 | { timeout: 120_000 }, | ||
| 62 | Effect.promise(async () => { | ||
| 63 | // Regression guard for "Had to setup my config again": `executor web --scope X` | ||
| 64 | // without `--foreground` cannot honor the scope (it opens the running service, | ||
| 65 | // which keeps its own scope). That used to be a SILENT no-op — the user lands | ||
| 66 | // on a different workspace and thinks their config vanished. It must warn. | ||
| 67 | const root = mkdtempSync(join(tmpdir(), "executor-web-scope-")); | ||
| 68 | const dataDir = join(root, "data"); | ||
| 69 | const scopeDir = join(root, "workspace"); | ||
| 70 | try { | ||
| 71 | const { stdout, stderr } = await execFileAsync( | ||
| 72 | "bun", | ||
| 73 | ["run", "dev:cli", "web", "--scope", scopeDir], | ||
| 74 | { cwd: repoRoot, env: { ...process.env, EXECUTOR_DATA_DIR: dataDir } }, | ||
| 75 | ); | ||
| 76 | const output = `${stdout}\n${stderr}`; | ||
| 77 | |||
| 78 | expect(output, "the dropped --scope is called out, not silently ignored").toContain( | ||
| 79 | "Ignoring --scope", | ||
| 80 | ); | ||
| 81 | expect(output, "the warning names the scope it dropped").toContain(scopeDir); | ||
| 82 | expect(output, "the warning points at the flag that actually applies scope").toContain( | ||
| 83 | "--foreground", | ||
| 84 | ); | ||
| 85 | } finally { | ||
| 86 | rmSync(root, { recursive: true, force: true }); | ||
| 87 | } | ||
| 88 | }), | ||
| 89 | ); | ||
| @@ -0,0 +1,75 @@ | |||
| 1 | // Local-only — REPRO + guard for "I've been running executor as stdio as get | ||
| 2 | // errors trying http one in opencode". The local app's HTTP `/mcp` endpoint is | ||
| 3 | // bearer-gated (hardened so loopback is not a free pass) and serves NO OAuth | ||
| 4 | // discovery. An external agent like opencode, pointed at the URL, tries MCP | ||
| 5 | // OAuth auto-detection, gets a plain `401 Bearer realm="executor"` with no | ||
| 6 | // resource-metadata to discover an authorization server from, and errors out. | ||
| 7 | // | ||
| 8 | // The HTTP transport itself is fine — it works the moment the bearer is supplied | ||
| 9 | // (opencode's remote MCP supports `headers` + `oauth: false`). This scenario | ||
| 10 | // proves exactly that: tools list over HTTP WITH the bearer, and the gate 401s | ||
| 11 | // WITHOUT it. It also asserts the `--foreground` ready output now prints a | ||
| 12 | // ready-to-paste opencode config (URL + bearer header + `oauth: false`) so a | ||
| 13 | // user does not have to reverse-engineer the gate. | ||
| 14 | import { expect } from "@effect/vitest"; | ||
| 15 | import { Effect } from "effect"; | ||
| 16 | import { Client } from "@modelcontextprotocol/sdk/client/index.js"; | ||
| 17 | import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; | ||
| 18 | |||
| 19 | import { scenario } from "../src/scenario"; | ||
| 20 | import { Cli, RunDir } from "../src/services"; | ||
| 21 | import { withLocalServer } from "./local-server"; | ||
| 22 | |||
| 23 | /** Connect an MCP client to the local `/mcp` over HTTP and list tools. Rejects | ||
| 24 | * if the bearer gate (or transport) refuses the connection. */ | ||
| 25 | const listToolsOverHttp = async ( | ||
| 26 | origin: string, | ||
| 27 | headers?: Record<string, string>, | ||
| 28 | ): Promise<readonly string[]> => { | ||
| 29 | const client = new Client({ name: "e2e-http-mcp", version: "0.0.0" }); | ||
| 30 | const transport = new StreamableHTTPClientTransport(new URL(`${origin}/mcp`), { | ||
| 31 | requestInit: headers ? { headers } : undefined, | ||
| 32 | }); | ||
| 33 | await client.connect(transport); | ||
| 34 | try { | ||
| 35 | const { tools } = await client.listTools(); | ||
| 36 | return tools.map((t) => t.name); | ||
| 37 | } finally { | ||
| 38 | await client.close().catch(() => {}); | ||
| 39 | } | ||
| 40 | }; | ||
| 41 | |||
| 42 | scenario( | ||
| 43 | "Local · HTTP MCP works with the bearer header and 401s without it", | ||
| 44 | { timeout: 180_000 }, | ||
| 45 | Effect.gen(function* () { | ||
| 46 | const cli = yield* Cli; | ||
| 47 | const runDir = yield* RunDir; | ||
| 48 | |||
| 49 | yield* withLocalServer(cli, runDir, (server) => | ||
| 50 | Effect.gen(function* () { | ||
| 51 | // WITH the bearer: the HTTP transport connects and lists tools — the very | ||
| 52 | // thing that "errors in opencode" when the agent omits the token. | ||
| 53 | const tools = yield* Effect.promise(() => | ||
| 54 | listToolsOverHttp(server.origin, { authorization: `Bearer ${server.token}` }), | ||
| 55 | ); | ||
| 56 | expect( | ||
| 57 | tools.length, | ||
| 58 | "HTTP MCP lists tools once the bearer is supplied", | ||
| 59 | ).toBeGreaterThan(0); | ||
| 60 | |||
| 61 | // WITHOUT the bearer: the gate rejects, which is what trips opencode's | ||
| 62 | // default OAuth auto-detection (no resource-metadata to recover from). | ||
| 63 | const unauthorized = yield* Effect.promise(async () => { | ||
| 64 | try { | ||
| 65 | await listToolsOverHttp(server.origin); | ||
| 66 | return "connected"; | ||
| 67 | } catch { | ||
| 68 | return "rejected"; | ||
| 69 | } | ||
| 70 | }); | ||
| 71 | expect(unauthorized, "HTTP MCP rejects a tokenless connection").toBe("rejected"); | ||
| 72 | }), | ||
| 73 | ); | ||
| 74 | }), | ||
| 75 | ); | ||
| @@ -0,0 +1,166 @@ | |||
| 1 | // Selfhost (browser): REPRO for issue #1120 ("Can't configure oauth mcp which | ||
| 2 | // used to work"). The user added an Atlassian remote-MCP source and the OAuth | ||
| 3 | // app picker listed a pile of unrelated *Datadog* apps. | ||
| 4 | // | ||
| 5 | // Why it happens: a remote MCP OAuth method carries ONLY a `discoveryUrl` | ||
| 6 | // (endpoints are discovered live, see describeMcpAuthMethods in the mcp plugin), | ||
| 7 | // never a static token/authorization URL. When transparent DCR falls back to the | ||
| 8 | // bring-your-own app picker, the picker filters candidate apps by endpoint root | ||
| 9 | // domain (`selectClientsForEndpoints`). With no declared endpoints the filter | ||
| 10 | // short-circuits to "every app is usable", so EVERY registered OAuth client the | ||
| 11 | // owner has, including DCR-minted apps from other integrations, leaks into this | ||
| 12 | // integration's picker. The `origin.integration` slug each client carries is | ||
| 13 | // never consulted. | ||
| 14 | // | ||
| 15 | // The fix: the picker matches apps to the integration by `origin.integration` | ||
| 16 | // (recorded on every app, DCR or manual), falling back to endpoint-domain | ||
| 17 | // matching only when endpoints are declared. So an app registered FOR Atlassian | ||
| 18 | // shows, and a Datadog app (different integration, no shared endpoint) does not. | ||
| 19 | // | ||
| 20 | // This scenario registers two apps for the same owner — one Datadog app with NO | ||
| 21 | // integration association, and one app registered FOR the Atlassian MCP source — | ||
| 22 | // then drives that source into the BYO picker (its discovery endpoint advertises | ||
| 23 | // no OAuth metadata, so DCR falls back) and asserts the Atlassian app appears | ||
| 24 | // while the Datadog app does not. | ||
| 25 | import { randomBytes } from "node:crypto"; | ||
| 26 | |||
| 27 | import { expect } from "@effect/vitest"; | ||
| 28 | import { Effect } from "effect"; | ||
| 29 | import { composePluginApi } from "@executor-js/api/server"; | ||
| 30 | import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; | ||
| 31 | import { makeGreetingMcpServer, serveMcpServer } from "@executor-js/plugin-mcp/testing"; | ||
| 32 | import { IntegrationSlug, OAuthClientSlug } from "@executor-js/sdk/shared"; | ||
| 33 | |||
| 34 | import { scenario } from "../src/scenario"; | ||
| 35 | import { Api, Browser, Target } from "../src/services"; | ||
| 36 | |||
| 37 | const api = composePluginApi([mcpHttpPlugin()] as const); | ||
| 38 | |||
| 39 | scenario( | ||
| 40 | "OAuth picker · a different integration's app does not leak into an MCP source's picker", | ||
| 41 | { timeout: 180_000 }, | ||
| 42 | Effect.scoped( | ||
| 43 | Effect.gen(function* () { | ||
| 44 | const target = yield* Target; | ||
| 45 | const browser = yield* Browser; | ||
| 46 | const { client: makeApiClient } = yield* Api; | ||
| 47 | const identity = yield* target.newIdentity(); | ||
| 48 | const client = yield* makeApiClient(api, identity); | ||
| 49 | |||
| 50 | // Selfhost shares one tenant across identities, so make every resource | ||
| 51 | // unique per run — including the Datadog host, which is what the picker | ||
| 52 | // row renders and what we assert against. | ||
| 53 | const id = randomBytes(4).toString("hex"); | ||
| 54 | const datadogSlug = OAuthClientSlug.make(`datadog-${id}`); | ||
| 55 | const datadogHost = `api-${id}.datadoghq.com`; | ||
| 56 | const atlassianSlug = OAuthClientSlug.make(`atlassian-app-${id}`); | ||
| 57 | const atlassianHost = `auth-${id}.atlassian.com`; | ||
| 58 | const mcpSlug = IntegrationSlug.make(`atlassian-mcp-${id}`); | ||
| 59 | |||
| 60 | // An Atlassian-shaped remote MCP source: it speaks MCP but advertises NO | ||
| 61 | // OAuth metadata, so the connect-time DCR probe finds no registration | ||
| 62 | // endpoint and the modal falls back to the bring-your-own app picker. | ||
| 63 | const server = yield* serveMcpServer(() => | ||
| 64 | makeGreetingMcpServer({ name: `atlassian-mcp-${id}` }), | ||
| 65 | ); | ||
| 66 | |||
| 67 | yield* Effect.ensuring( | ||
| 68 | Effect.gen(function* () { | ||
| 69 | // A Datadog OAuth app the owner registered for a DIFFERENT integration. | ||
| 70 | // Endpoints are on datadoghq.com — nothing to do with Atlassian, and no | ||
| 71 | // integration association — so it must never surface in this picker. | ||
| 72 | yield* client.oauth.createClient({ | ||
| 73 | payload: { | ||
| 74 | owner: "user", | ||
| 75 | slug: datadogSlug, | ||
| 76 | authorizationUrl: `https://${datadogHost}/oauth2/authorize`, | ||
| 77 | tokenUrl: `https://${datadogHost}/oauth2/token`, | ||
| 78 | grant: "authorization_code", | ||
| 79 | clientId: `datadog-client-${id}`, | ||
| 80 | clientSecret: "datadog-secret", | ||
| 81 | }, | ||
| 82 | }); | ||
| 83 | |||
| 84 | // An app the owner registered FOR the Atlassian MCP source. It carries | ||
| 85 | // `origin.integration`, so the picker surfaces it even though the source | ||
| 86 | // declares no static endpoints to match on. | ||
| 87 | yield* client.oauth.createClient({ | ||
| 88 | payload: { | ||
| 89 | owner: "user", | ||
| 90 | slug: atlassianSlug, | ||
| 91 | authorizationUrl: `https://${atlassianHost}/authorize`, | ||
| 92 | tokenUrl: `https://${atlassianHost}/token`, | ||
| 93 | grant: "authorization_code", | ||
| 94 | clientId: `atlassian-client-${id}`, | ||
| 95 | clientSecret: "atlassian-secret", | ||
| 96 | originIntegration: mcpSlug, | ||
| 97 | }, | ||
| 98 | }); | ||
| 99 | |||
| 100 | // Persistence guard: the integration association round-trips through the | ||
| 101 | // API so the picker can rely on it (isolates server-side persistence | ||
| 102 | // from the browser read path). | ||
| 103 | const listed = yield* client.oauth.listClients(); | ||
| 104 | const atlassianRow = listed.find((c) => String(c.slug) === String(atlassianSlug)); | ||
| 105 | expect( | ||
| 106 | atlassianRow?.origin.integration && String(atlassianRow.origin.integration), | ||
| 107 | "the manually-registered app records its integration association", | ||
| 108 | ).toBe(String(mcpSlug)); | ||
| 109 | |||
| 110 | // The Atlassian MCP source as the add flow would leave it: OAuth only. | ||
| 111 | yield* client.mcp.addServer({ | ||
| 112 | payload: { | ||
| 113 | transport: "remote", | ||
| 114 | name: `Atlassian MCP ${id}`, | ||
| 115 | endpoint: server.endpoint, | ||
| 116 | slug: mcpSlug, | ||
| 117 | authenticationTemplate: [{ kind: "oauth2" }], | ||
| 118 | }, | ||
| 119 | }); | ||
| 120 | |||
| 121 | yield* browser.session(identity, async ({ page, step }) => { | ||
| 122 | await step("Open the Atlassian MCP source's connect modal", async () => { | ||
| 123 | await page.goto(`/integrations/${mcpSlug}`, { waitUntil: "networkidle" }); | ||
| 124 | await page.getByRole("button", { name: "Add connection" }).first().click(); | ||
| 125 | // OAuth is the only method; transparent DCR shows the Connect CTA. | ||
| 126 | await page.getByRole("button", { name: "Connect", exact: true }).waitFor(); | ||
| 127 | }); | ||
| 128 | |||
| 129 | await step("DCR falls back to the bring-your-own app picker", async () => { | ||
| 130 | await page.getByRole("button", { name: "Connect", exact: true }).click(); | ||
| 131 | // The probe finds no OAuth metadata and DCR falls back to the app | ||
| 132 | // step. Either register affordance ("Register a new app" when an app | ||
| 133 | // matched, "Register app" in the empty state) means we're past DCR. | ||
| 134 | await page | ||
| 135 | .getByRole("button", { name: /^Register (app|a new app)$/ }) | ||
| 136 | .first() | ||
| 137 | .waitFor({ timeout: 30_000 }); | ||
| 138 | }); | ||
| 139 | |||
| 140 | await step("The picker lists the Atlassian app, not the Datadog one", async () => { | ||
| 141 | const dialog = page.getByRole("dialog"); | ||
| 142 | expect( | ||
| 143 | await dialog.getByText(atlassianHost).count(), | ||
| 144 | `the app registered for this integration (${atlassianHost}) is offered`, | ||
| 145 | ).toBeGreaterThan(0); | ||
| 146 | expect( | ||
| 147 | await dialog.getByText(datadogHost).count(), | ||
| 148 | `a Datadog OAuth app (${datadogHost}) must not leak into Atlassian's picker`, | ||
| 149 | ).toBe(0); | ||
| 150 | }); | ||
| 151 | }); | ||
| 152 | }), | ||
| 153 | // Never leak the source or the apps into the shared selfhost tenant. | ||
| 154 | Effect.gen(function* () { | ||
| 155 | yield* client.mcp.removeServer({ params: { slug: mcpSlug } }).pipe(Effect.ignore); | ||
| 156 | yield* client.oauth | ||
| 157 | .removeClient({ params: { slug: datadogSlug }, payload: { owner: "user" } }) | ||
| 158 | .pipe(Effect.ignore); | ||
| 159 | yield* client.oauth | ||
| 160 | .removeClient({ params: { slug: atlassianSlug }, payload: { owner: "user" } }) | ||
| 161 | .pipe(Effect.ignore); | ||
| 162 | }), | ||
| 163 | ); | ||
| 164 | }), | ||
| 165 | ), | ||
| 166 | ); | ||
| @@ -103,6 +103,7 @@ export const OAuthHandlers = HttpApiBuilder.group(ExecutorApi, "oauth", (handler · 102 unmodified lines | |||
| 103 | clientId: payload.clientId, | 103 | clientId: payload.clientId, |
| 104 | clientSecret: payload.clientSecret, | 104 | clientSecret: payload.clientSecret, |
| 105 | resource: payload.resource ?? null, | 105 | resource: payload.resource ?? null, |
| 106 | origin: { kind: "manual", integration: payload.originIntegration ?? null }, | ||
| 106 | }); | 107 | }); |
| 107 | return { client }; | 108 | return { client }; |
| 108 | }), | 109 | }), |
| @@ -65,6 +65,10 @@ const CreateClientPayload = Schema.Struct({ · 64 unmodified lines | |||
| 65 | clientId: Schema.String, | 65 | clientId: Schema.String, |
| 66 | clientSecret: Schema.String, | 66 | clientSecret: Schema.String, |
| 67 | resource: Schema.optional(Schema.NullOr(Schema.String)), | 67 | resource: Schema.optional(Schema.NullOr(Schema.String)), |
| 68 | /** Integration this app is being registered for, recorded on the client so the | ||
| 69 | * connect picker can surface it for that integration (mirrors the DCR path). | ||
| 70 | * A manual app can still be reused across integrations. */ | ||
| 71 | originIntegration: Schema.optional(Schema.NullOr(IntegrationSlug)), | ||
| 68 | }); | 72 | }); |
| 69 | 73 | ||
| 70 | const CreateClientResponse = Schema.Struct({ | 74 | const CreateClientResponse = Schema.Struct({ |
| @@ -111,7 +115,10 @@ const OAuthClientSummaryResponse = Schema.Struct({ · 40 unmodified lines | |||
| 111 | resource: Schema.optional(Schema.NullOr(Schema.String)), | 115 | resource: Schema.optional(Schema.NullOr(Schema.String)), |
| 112 | clientId: Schema.String, | 116 | clientId: Schema.String, |
| 113 | origin: Schema.Union([ | 117 | origin: Schema.Union([ |
| 114 | Schema.Struct({ kind: Schema.Literal("manual") }), | 118 | Schema.Struct({ |
| 119 | kind: Schema.Literal("manual"), | ||
| 120 | integration: Schema.optional(Schema.NullOr(IntegrationSlug)), | ||
| 121 | }), | ||
| 115 | Schema.Struct({ | 122 | Schema.Struct({ |
| 116 | kind: Schema.Literal("dynamic_client_registration"), | 123 | kind: Schema.Literal("dynamic_client_registration"), |
| 117 | integration: Schema.optional(Schema.NullOr(IntegrationSlug)), | 124 | integration: Schema.optional(Schema.NullOr(IntegrationSlug)), |
| @@ -221,7 +221,10 @@ const OAuthClientOutput = Schema.Struct({ · 220 unmodified lines | |||
| 221 | resource: Schema.optional(Schema.NullOr(Schema.String)), | 221 | resource: Schema.optional(Schema.NullOr(Schema.String)), |
| 222 | clientId: Schema.String, | 222 | clientId: Schema.String, |
| 223 | origin: Schema.Union([ | 223 | origin: Schema.Union([ |
| 224 | Schema.Struct({ kind: Schema.Literal("manual") }), | 224 | Schema.Struct({ |
| 225 | kind: Schema.Literal("manual"), | ||
| 226 | integration: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 227 | }), | ||
| 225 | Schema.Struct({ | 228 | Schema.Struct({ |
| 226 | kind: Schema.Literal("dynamic_client_registration"), | 229 | kind: Schema.Literal("dynamic_client_registration"), |
| 227 | integration: Schema.optional(Schema.NullOr(Schema.String)), | 230 | integration: Schema.optional(Schema.NullOr(Schema.String)), |
| @@ -228,7 +228,9 @@ export { · 227 unmodified lines | |||
| 228 | type OAuthGrant, | 228 | type OAuthGrant, |
| 229 | type OAuthAuthentication, | 229 | type OAuthAuthentication, |
| 230 | type OAuthClient, | 230 | type OAuthClient, |
| 231 | type OAuthClientOrigin, | ||
| 231 | type OAuthClientSummary, | 232 | type OAuthClientSummary, |
| 233 | oauthClientOriginIntegration, | ||
| 232 | type CreateOAuthClientInput, | 234 | type CreateOAuthClientInput, |
| 233 | type RegisterDynamicClientInput, | 235 | type RegisterDynamicClientInput, |
| 234 | type ConnectResult, | 236 | type ConnectResult, |
| @@ -56,12 +56,27 @@ export interface OAuthClient { · 55 unmodified lines | |||
| 56 | } | 56 | } |
| 57 | 57 | ||
| 58 | export type OAuthClientOrigin = | 58 | export type OAuthClientOrigin = |
| 59 | | { readonly kind: "manual" } | 59 | | { |
| 60 | readonly kind: "manual"; | ||
| 61 | /** Integration this app was registered FOR, when known. A user can reuse | ||
| 62 | * one app across integrations, so this is a hint (the integration whose | ||
| 63 | * connect flow created it), not an exclusive binding. Used by the connect | ||
| 64 | * picker to surface this app for its integration even when the integration | ||
| 65 | * declares no static endpoints to match on (e.g. an MCP source). */ | ||
| 66 | readonly integration?: IntegrationSlug | null; | ||
| 67 | } | ||
| 60 | | { | 68 | | { |
| 61 | readonly kind: "dynamic_client_registration"; | 69 | readonly kind: "dynamic_client_registration"; |
| 62 | readonly integration?: IntegrationSlug | null; | 70 | readonly integration?: IntegrationSlug | null; |
| 63 | }; | 71 | }; |
| 64 | 72 | ||
| 73 | /** The integration an OAuth app was registered for, from either origin kind | ||
| 74 | * (null when unknown). Centralizes the "which integration owns this app" read so | ||
| 75 | * the picker and the duplicate-DCR guard agree. */ | ||
| 76 | export const oauthClientOriginIntegration = ( | ||
| 77 | origin: OAuthClientOrigin, | ||
| 78 | ): IntegrationSlug | null => origin.integration ?? null; | ||
| 79 | |||
| 65 | export type CreateOAuthClientInput = OAuthClient & { | 80 | export type CreateOAuthClientInput = OAuthClient & { |
| 66 | readonly origin?: OAuthClientOrigin; | 81 | readonly origin?: OAuthClientOrigin; |
| 67 | }; | 82 | }; |
| @@ -229,14 +229,10 @@ const parseOAuthClientOrigin = (row: { · 228 unmodified lines | |||
| 229 | readonly origin_kind?: unknown; | 229 | readonly origin_kind?: unknown; |
| 230 | readonly origin_integration?: unknown; | 230 | readonly origin_integration?: unknown; |
| 231 | }): OAuthClientOrigin => { | 231 | }): OAuthClientOrigin => { |
| 232 | const integration = | ||
| 233 | row.origin_integration == null ? null : IntegrationSlug.make(String(row.origin_integration)); | ||
| 232 | if (row.origin_kind === "dynamic_client_registration") { | 234 | if (row.origin_kind === "dynamic_client_registration") { |
| 233 | return { | 235 | return { kind: "dynamic_client_registration", integration }; |
| 234 | kind: "dynamic_client_registration", | ||
| 235 | integration: | ||
| 236 | row.origin_integration == null | ||
| 237 | ? null | ||
| 238 | : IntegrationSlug.make(String(row.origin_integration)), | ||
| 239 | }; | ||
| 240 | } | 236 | } |
| 241 | const slug = row.slug == null ? "" : String(row.slug); | 237 | const slug = row.slug == null ? "" : String(row.slug); |
| 242 | const resource = row.resource == null ? "" : String(row.resource); | 238 | const resource = row.resource == null ? "" : String(row.resource); |
| @@ -246,9 +242,9 @@ const parseOAuthClientOrigin = (row: { · 3 unmodified lines | |||
| 246 | /(^|[-_])mcp($|[-_])/.test(slug) && | 242 | /(^|[-_])mcp($|[-_])/.test(slug) && |
| 247 | /(^|\/)mcp($|[/?#])/.test(resource) | 243 | /(^|\/)mcp($|[/?#])/.test(resource) |
| 248 | ) { | 244 | ) { |
| 249 | return { kind: "dynamic_client_registration", integration: null }; | 245 | return { kind: "dynamic_client_registration", integration }; |
| 250 | } | 246 | } |
| 251 | return { kind: "manual" }; | 247 | return { kind: "manual", integration }; |
| 252 | }; | 248 | }; |
| 253 | 249 | ||
| 254 | interface LoadedOAuthClient { | 250 | interface LoadedOAuthClient { |
| @@ -404,11 +400,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 149 unmodified lines | |||
| 404 | resource: input.resource ?? null, | 400 | resource: input.resource ?? null, |
| 405 | origin_kind: input.origin?.kind ?? "manual", | 401 | origin_kind: input.origin?.kind ?? "manual", |
| 406 | origin_integration: | 402 | origin_integration: |
| 407 | input.origin?.kind === "dynamic_client_registration" | 403 | input.origin?.integration == null ? null : String(input.origin.integration), |
| 408 | ? input.origin.integration == null | ||
| 409 | ? null | ||
| 410 | : String(input.origin.integration) | ||
| 411 | : null, | ||
| 412 | created_at: now, | 404 | created_at: now, |
| 413 | }), | 405 | }), |
| 414 | ); | 406 | ); |
| @@ -106,7 +106,9 @@ export { · 105 unmodified lines | |||
| 106 | type OAuthGrant, | 106 | type OAuthGrant, |
| 107 | type OAuthAuthentication, | 107 | type OAuthAuthentication, |
| 108 | type OAuthClient, | 108 | type OAuthClient, |
| 109 | type OAuthClientOrigin, | ||
| 109 | type OAuthClientSummary, | 110 | type OAuthClientSummary, |
| 111 | oauthClientOriginIntegration, | ||
| 110 | type CreateOAuthClientInput, | 112 | type CreateOAuthClientInput, |
| 111 | type RegisterDynamicClientInput, | 113 | type RegisterDynamicClientInput, |
| 112 | type ConnectResult, | 114 | type ConnectResult, |
| @@ -460,6 +460,7 @@ export const createOAuthClientOptimistic = oauthClientsOptimisticAtom.pipe( · 459 unmodified lines | |||
| 460 | readonly grant: OAuthGrant; | 460 | readonly grant: OAuthGrant; |
| 461 | readonly clientId: string; | 461 | readonly clientId: string; |
| 462 | readonly resource?: string | null; | 462 | readonly resource?: string | null; |
| 463 | readonly originIntegration?: IntegrationSlug | null; | ||
| 463 | }; | 464 | }; |
| 464 | }, | 465 | }, |
| 465 | ) => | 466 | ) => |
| @@ -472,7 +473,7 @@ export const createOAuthClientOptimistic = oauthClientsOptimisticAtom.pipe( · 6 unmodified lines | |||
| 472 | tokenUrl: arg.payload.tokenUrl, | 473 | tokenUrl: arg.payload.tokenUrl, |
| 473 | resource: arg.payload.resource ?? null, | 474 | resource: arg.payload.resource ?? null, |
| 474 | clientId: arg.payload.clientId, | 475 | clientId: arg.payload.clientId, |
| 475 | origin: { kind: "manual" }, | 476 | origin: { kind: "manual", integration: arg.payload.originIntegration ?? null }, |
| 476 | }; | 477 | }; |
| 477 | const index = rows.findIndex( | 478 | const index = rows.findIndex( |
| 478 | (client) => client.owner === summary.owner && client.slug === summary.slug, | 479 | (client) => client.owner === summary.owner && client.slug === summary.slug, |
| @@ -232,7 +232,7 @@ describe("runDcrConnect", () => { · 231 unmodified lines | |||
| 232 | discoveryUrl: "https://mcp.example.com/mcp", | 232 | discoveryUrl: "https://mcp.example.com/mcp", |
| 233 | owner: "user", | 233 | owner: "user", |
| 234 | integrationName: "Linear MCP", | 234 | integrationName: "Linear MCP", |
| 235 | existingSlugs: [], | 235 | existingClients: [], |
| 236 | redirectUri: "https://localhost:5394/api/oauth/callback", | 236 | redirectUri: "https://localhost:5394/api/oauth/callback", |
| 237 | integration: TEST_INTEGRATION, | 237 | integration: TEST_INTEGRATION, |
| 238 | }, | 238 | }, |
| @@ -259,6 +259,101 @@ describe("runDcrConnect", () => { · 20 unmodified lines | |||
| 259 | expect(startArgs!.owner).toBe("user"); | 259 | expect(startArgs!.owner).toBe("user"); |
| 260 | }); | 260 | }); |
| 261 | 261 | ||
| 262 | it("reuses an existing client minted for this integration instead of minting a duplicate", async () => { | ||
| 263 | // Regression guard for "Datadog, Datadog 2, … Datadog 10": a prior DCR client | ||
| 264 | // for THIS integration (same owner, same token host) must be reused, so a | ||
| 265 | // second connect runs only `start` — never `register`. | ||
| 266 | const calls: string[] = []; | ||
| 267 | let startArgs: StartArgs | null = null; | ||
| 268 | const outcome = await runDcrConnect( | ||
| 269 | { | ||
| 270 | probe: (): Promise<ProbeResult | null> => { | ||
| 271 | calls.push("probe"); | ||
| 272 | return Promise.resolve({ | ||
| 273 | authorizationUrl: "https://auth.example.com/authorize", | ||
| 274 | tokenUrl: "https://auth.example.com/token", | ||
| 275 | registrationEndpoint: "https://auth.example.com/register", | ||
| 276 | }); | ||
| 277 | }, | ||
| 278 | register: (args: RegisterArgs): Promise<OAuthClientSlug | null> => { | ||
| 279 | calls.push("register"); | ||
| 280 | return Promise.resolve(args.slug); | ||
| 281 | }, | ||
| 282 | start: (args: StartArgs): void => { | ||
| 283 | calls.push("start"); | ||
| 284 | startArgs = args; | ||
| 285 | }, | ||
| 286 | }, | ||
| 287 | { | ||
| 288 | discoveryUrl: "https://mcp.example.com/mcp", | ||
| 289 | owner: "user", | ||
| 290 | integrationName: "Linear MCP", | ||
| 291 | existingClients: [ | ||
| 292 | { | ||
| 293 | owner: "user", | ||
| 294 | slug: OAuthClientSlug.make("linear-mcp"), | ||
| 295 | grant: "authorization_code", | ||
| 296 | authorizationUrl: "https://auth.example.com/authorize", | ||
| 297 | tokenUrl: "https://auth.example.com/token", | ||
| 298 | clientId: "minted-earlier", | ||
| 299 | origin: { kind: "dynamic_client_registration", integration: TEST_INTEGRATION }, | ||
| 300 | }, | ||
| 301 | ], | ||
| 302 | integration: TEST_INTEGRATION, | ||
| 303 | }, | ||
| 304 | ); | ||
| 305 | |||
| 306 | expect(outcome.kind).toBe("started"); | ||
| 307 | // Probed (to confirm the same server), then reused — NO second registration. | ||
| 308 | expect(calls).toEqual(["probe", "start"]); | ||
| 309 | expect(String(startArgs!.client)).toBe("linear-mcp"); | ||
| 310 | }); | ||
| 311 | |||
| 312 | it("does not reuse a client belonging to a DIFFERENT integration", async () => { | ||
| 313 | // A Datadog DCR client must never be reused for an Atlassian connect, even | ||
| 314 | // though both went through DCR — provenance is keyed by integration. | ||
| 315 | const calls: string[] = []; | ||
| 316 | await runDcrConnect( | ||
| 317 | { | ||
| 318 | probe: (): Promise<ProbeResult | null> => | ||
| 319 | Promise.resolve({ | ||
| 320 | authorizationUrl: "https://auth.atlassian.com/authorize", | ||
| 321 | tokenUrl: "https://auth.atlassian.com/token", | ||
| 322 | registrationEndpoint: "https://auth.atlassian.com/register", | ||
| 323 | }), | ||
| 324 | register: (args: RegisterArgs): Promise<OAuthClientSlug | null> => { | ||
| 325 | calls.push("register"); | ||
| 326 | return Promise.resolve(args.slug); | ||
| 327 | }, | ||
| 328 | start: (): void => { | ||
| 329 | calls.push("start"); | ||
| 330 | }, | ||
| 331 | }, | ||
| 332 | { | ||
| 333 | discoveryUrl: "https://mcp.atlassian.com/mcp", | ||
| 334 | owner: "user", | ||
| 335 | integrationName: "Atlassian", | ||
| 336 | existingClients: [ | ||
| 337 | { | ||
| 338 | owner: "user", | ||
| 339 | slug: OAuthClientSlug.make("datadog"), | ||
| 340 | grant: "authorization_code", | ||
| 341 | authorizationUrl: "https://app.datadoghq.com/oauth2/authorize", | ||
| 342 | tokenUrl: "https://api.datadoghq.com/oauth2/token", | ||
| 343 | clientId: "datadog-client", | ||
| 344 | origin: { | ||
| 345 | kind: "dynamic_client_registration", | ||
| 346 | integration: IntegrationSlug.make("datadog"), | ||
| 347 | }, | ||
| 348 | }, | ||
| 349 | ], | ||
| 350 | integration: TEST_INTEGRATION, | ||
| 351 | }, | ||
| 352 | ); | ||
| 353 | // No Atlassian client exists yet → it mints a fresh one, not reuse Datadog. | ||
| 354 | expect(calls).toContain("register"); | ||
| 355 | }); | ||
| 356 | |||
| 262 | it("prefers declared scopes over probed scopes when present", async () => { | 357 | it("prefers declared scopes over probed scopes when present", async () => { |
| 263 | let registerArgs: RegisterArgs | null = null; | 358 | let registerArgs: RegisterArgs | null = null; |
| 264 | const outcome = await runDcrConnect( | 359 | const outcome = await runDcrConnect( |
| @@ -280,7 +375,7 @@ describe("runDcrConnect", () => { · 15 unmodified lines | |||
| 280 | discoveryUrl: "https://mcp.example.com/mcp", | 375 | discoveryUrl: "https://mcp.example.com/mcp", |
| 281 | owner: "user", | 376 | owner: "user", |
| 282 | integrationName: "App", | 377 | integrationName: "App", |
| 283 | existingSlugs: [], | 378 | existingClients: [], |
| 284 | declaredScopes: ["declared.scope"], | 379 | declaredScopes: ["declared.scope"], |
| 285 | integration: TEST_INTEGRATION, | 380 | integration: TEST_INTEGRATION, |
| 286 | }, | 381 | }, |
| @@ -313,7 +408,7 @@ describe("runDcrConnect", () => { · 26 unmodified lines | |||
| 313 | discoveryUrl: "https://mcp.example.com/mcp", | 408 | discoveryUrl: "https://mcp.example.com/mcp", |
| 314 | owner: "user", | 409 | owner: "user", |
| 315 | integrationName: "App", | 410 | integrationName: "App", |
| 316 | existingSlugs: [], | 411 | existingClients: [], |
| 317 | integration: TEST_INTEGRATION, | 412 | integration: TEST_INTEGRATION, |
| 318 | }, | 413 | }, |
| 319 | ); | 414 | ); |
| @@ -344,7 +439,7 @@ describe("runDcrConnect", () => { · 24 unmodified lines | |||
| 344 | discoveryUrl: "https://mcp.example.com/mcp", | 439 | discoveryUrl: "https://mcp.example.com/mcp", |
| 345 | owner: "user", | 440 | owner: "user", |
| 346 | integrationName: "App", | 441 | integrationName: "App", |
| 347 | existingSlugs: [], | 442 | existingClients: [], |
| 348 | integration: TEST_INTEGRATION, | 443 | integration: TEST_INTEGRATION, |
| 349 | }, | 444 | }, |
| 350 | ); | 445 | ); |
| @@ -374,7 +469,7 @@ describe("runDcrConnect", () => { · 23 unmodified lines | |||
| 374 | discoveryUrl: "https://mcp.example.com/mcp", | 469 | discoveryUrl: "https://mcp.example.com/mcp", |
| 375 | owner: "user", | 470 | owner: "user", |
| 376 | integrationName: "App", | 471 | integrationName: "App", |
| 377 | existingSlugs: [], | 472 | existingClients: [], |
| 378 | integration: TEST_INTEGRATION, | 473 | integration: TEST_INTEGRATION, |
| 379 | }, | 474 | }, |
| 380 | ); | 475 | ); |
| @@ -42,6 +42,7 @@ import { oauthCallbackUrl, useOAuthPopupFlow } from "../plugins/oauth-sign-in"; · 41 unmodified lines | |||
| 42 | import { | 42 | import { |
| 43 | clientDisplayName, | 43 | clientDisplayName, |
| 44 | clientHost, | 44 | clientHost, |
| 45 | getRootDomain, | ||
| 45 | uniqueClientSlug, | 46 | uniqueClientSlug, |
| 46 | useOAuthClientsForIntegration, | 47 | useOAuthClientsForIntegration, |
| 47 | type OAuthClientOption, | 48 | type OAuthClientOption, |
| @@ -471,8 +472,10 @@ type RunDcrConnectInput = { · 423 unmodified lines | |||
| 471 | readonly discoveryUrl: string; | 472 | readonly discoveryUrl: string; |
| 472 | readonly owner: Owner; | 473 | readonly owner: Owner; |
| 473 | readonly integrationName: string; | 474 | readonly integrationName: string; |
| 474 | /** The owner's existing client slugs, so the minted slug stays unique. */ | 475 | /** The owner's existing apps (UNFILTERED). Used both to keep a minted slug |
| 475 | readonly existingSlugs: readonly string[]; | 476 | * unique across all apps AND to reuse a client already minted for this |
| 477 | * integration instead of minting a duplicate on every connect. */ | ||
| 478 | readonly existingClients: readonly OAuthClientOption[]; | ||
| 476 | /** Scopes declared by the integration's method (override the probed ones). */ | 479 | /** Scopes declared by the integration's method (override the probed ones). */ |
| 477 | readonly declaredScopes?: readonly string[]; | 480 | readonly declaredScopes?: readonly string[]; |
| 478 | /** Browser-facing callback URL registered with DCR when available. */ | 481 | /** Browser-facing callback URL registered with DCR when available. */ |
| @@ -504,7 +507,28 @@ export async function runDcrConnect( · 25 unmodified lines | |||
| 504 | const registrationEndpoint = probe.registrationEndpoint; | 507 | const registrationEndpoint = probe.registrationEndpoint; |
| 505 | if (!registrationEndpoint) return { kind: "fallback", reason: "no-registration-endpoint" }; | 508 | if (!registrationEndpoint) return { kind: "fallback", reason: "no-registration-endpoint" }; |
| 506 | 509 | ||
| 507 | const slug = uniqueClientSlug(input.integrationName, input.existingSlugs); | 510 | // Reuse a client already minted for THIS integration (same owner, same |
| 511 | // discovered token host) instead of minting a duplicate every connect — the | ||
| 512 | // cause of "Datadog", "Datadog 2", … "Datadog 10" piling up. A fresh probe of | ||
| 513 | // the same server yields the same endpoints, so an existing DCR client is | ||
| 514 | // still valid; we re-run only the authorize/token flow with it. | ||
| 515 | const probedTokenRoot = getRootDomain(probe.tokenUrl); | ||
| 516 | const reusable = input.existingClients.find( | ||
| 517 | (app) => | ||
| 518 | app.owner === input.owner && | ||
| 519 | app.origin?.integration != null && | ||
| 520 | String(app.origin.integration) === String(input.integration) && | ||
| 521 | getRootDomain(app.tokenUrl) === probedTokenRoot, | ||
| 522 | ); | ||
| 523 | if (reusable) { | ||
| 524 | deps.start({ client: reusable.slug, owner: input.owner }); | ||
| 525 | return { kind: "started" }; | ||
| 526 | } | ||
| 527 | |||
| 528 | const slug = uniqueClientSlug( | ||
| 529 | input.integrationName, | ||
| 530 | input.existingClients.map((app) => String(app.slug)), | ||
| 531 | ); | ||
| 508 | const scopes = | 532 | const scopes = |
| 509 | input.declaredScopes && input.declaredScopes.length > 0 | 533 | input.declaredScopes && input.declaredScopes.length > 0 |
| 510 | ? input.declaredScopes | 534 | ? input.declaredScopes |
| @@ -827,7 +851,9 @@ function AddAccountModalView(props: AddAccountModalProps) { · 316 unmodified lines | |||
| 827 | loading: oauthLoading, | 851 | loading: oauthLoading, |
| 828 | endpointMatched: oauthEndpointMatched, | 852 | endpointMatched: oauthEndpointMatched, |
| 829 | displayRegisterCTA: oauthDisplayRegisterCTA, | 853 | displayRegisterCTA: oauthDisplayRegisterCTA, |
| 854 | allClients: oauthAllClients, | ||
| 830 | } = useOAuthClientsForIntegration({ | 855 | } = useOAuthClientsForIntegration({ |
| 856 | integration, | ||
| 831 | tokenUrl: method?.oauth?.tokenUrl, | 857 | tokenUrl: method?.oauth?.tokenUrl, |
| 832 | authorizationUrl: method?.oauth?.authorizationUrl, | 858 | authorizationUrl: method?.oauth?.authorizationUrl, |
| 833 | }); | 859 | }); |
| @@ -1143,9 +1169,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 309 unmodified lines | |||
| 1143 | discoveryUrl, | 1169 | discoveryUrl, |
| 1144 | owner: dcrOwner, | 1170 | owner: dcrOwner, |
| 1145 | integrationName, | 1171 | integrationName, |
| 1146 | existingSlugs: [...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) => | 1172 | existingClients: oauthAllClients, |
| 1147 | String(app.slug), | ||
| 1148 | ), | ||
| 1149 | declaredScopes: method.oauth?.scopes, | 1173 | declaredScopes: method.oauth?.scopes, |
| 1150 | redirectUri: oauthCallbackUrl(), | 1174 | redirectUri: oauthCallbackUrl(), |
| 1151 | integration, | 1175 | integration, |
| @@ -1201,9 +1225,8 @@ function AddAccountModalView(props: AddAccountModalProps) { · 49 unmodified lines | |||
| 1201 | <div className="px-5 py-5"> | 1225 | <div className="px-5 py-5"> |
| 1202 | <OAuthClientForm | 1226 | <OAuthClientForm |
| 1203 | integrationName={integrationName} | 1227 | integrationName={integrationName} |
| 1204 | existingSlugs={[...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) => | 1228 | originIntegration={integration} |
| 1205 | String(app.slug), | 1229 | existingSlugs={oauthAllClients.map((app: OAuthClientOption) => String(app.slug))} |
| 1206 | )} | ||
| 1207 | fixedSlug={editingClient.slug} | 1230 | fixedSlug={editingClient.slug} |
| 1208 | fixedOwner={editingClient.owner} | 1231 | fixedOwner={editingClient.owner} |
| 1209 | prefill={{ | 1232 | prefill={{ |
| @@ -1230,9 +1253,8 @@ function AddAccountModalView(props: AddAccountModalProps) { · 20 unmodified lines | |||
| 1230 | <div className="px-5 py-5"> | 1253 | <div className="px-5 py-5"> |
| 1231 | <OAuthClientForm | 1254 | <OAuthClientForm |
| 1232 | integrationName={integrationName} | 1255 | integrationName={integrationName} |
| 1233 | existingSlugs={[...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) => | 1256 | originIntegration={integration} |
| 1234 | String(app.slug), | 1257 | existingSlugs={oauthAllClients.map((app: OAuthClientOption) => String(app.slug))} |
| 1235 | )} | ||
| 1236 | prefill={{ | 1258 | prefill={{ |
| 1237 | authorizationUrl: | 1259 | authorizationUrl: |
| 1238 | oauthHandoffPrefill?.authorizationUrl ?? method.oauth?.authorizationUrl, | 1260 | oauthHandoffPrefill?.authorizationUrl ?? method.oauth?.authorizationUrl, |
| @@ -1,7 +1,12 @@ | |||
| 1 | import { useMemo, useState } from "react"; | 1 | import { useMemo, useState } from "react"; |
| 2 | import { useAtomSet } from "@effect/atom-react"; | 2 | import { useAtomSet } from "@effect/atom-react"; |
| 3 | import * as Exit from "effect/Exit"; | 3 | import * as Exit from "effect/Exit"; |
| 4 | import { OAuthClientSlug, type OAuthGrant, type Owner } from "@executor-js/sdk/shared"; | 4 | import { |
| 5 | type IntegrationSlug, | ||
| 6 | OAuthClientSlug, | ||
| 7 | type OAuthGrant, | ||
| 8 | type Owner, | ||
| 9 | } from "@executor-js/sdk/shared"; | ||
| 5 | import { toast } from "sonner"; | 10 | import { toast } from "sonner"; |
| 6 | 11 | ||
| 7 | import { createOAuthClientOptimistic, probeOAuth, registerDynamicOAuthClient } from "../api/atoms"; | 12 | import { createOAuthClientOptimistic, probeOAuth, registerDynamicOAuthClient } from "../api/atoms"; |
| @@ -53,6 +58,10 @@ export interface OAuthClientFormPrefill { · 45 unmodified lines | |||
| 53 | export function OAuthClientForm(props: { | 58 | export function OAuthClientForm(props: { |
| 54 | /** Human label for the integration this app backs (used in toasts + default name). */ | 59 | /** Human label for the integration this app backs (used in toasts + default name). */ |
| 55 | readonly integrationName: string; | 60 | readonly integrationName: string; |
| 61 | /** Slug of the integration this app is registered for. Recorded on the client | ||
| 62 | * (`origin.integration`) so the connect picker surfaces it for this integration | ||
| 63 | * even when the integration declares no static endpoints (e.g. an MCP source). */ | ||
| 64 | readonly originIntegration?: IntegrationSlug; | ||
| 56 | /** Existing client slugs, so the generated slug stays unique across apps. */ | 65 | /** Existing client slugs, so the generated slug stays unique across apps. */ |
| 57 | readonly existingSlugs: readonly string[]; | 66 | readonly existingSlugs: readonly string[]; |
| 58 | /** Endpoints/scopes declared by the integration's OAuth method. */ | 67 | /** Endpoints/scopes declared by the integration's OAuth method. */ |
| @@ -71,6 +80,7 @@ export function OAuthClientForm(props: { · 12 unmodified lines | |||
| 71 | }) { | 80 | }) { |
| 72 | const { | 81 | const { |
| 73 | integrationName, | 82 | integrationName, |
| 83 | originIntegration, | ||
| 74 | existingSlugs, | 84 | existingSlugs, |
| 75 | prefill, | 85 | prefill, |
| 76 | fixedSlug, | 86 | fixedSlug, |
| @@ -195,6 +205,7 @@ export function OAuthClientForm(props: { · 118 unmodified lines | |||
| 195 | tokenEndpointAuthMethodsSupported: authMethods, | 205 | tokenEndpointAuthMethodsSupported: authMethods, |
| 196 | clientName: name.trim(), | 206 | clientName: name.trim(), |
| 197 | redirectUri: oauthCallbackUrl(), | 207 | redirectUri: oauthCallbackUrl(), |
| 208 | originIntegration: originIntegration ?? null, | ||
| 198 | }, | 209 | }, |
| 199 | reactivityKeys: oauthClientWriteKeys, | 210 | reactivityKeys: oauthClientWriteKeys, |
| 200 | }); | 211 | }); |
| @@ -223,6 +234,7 @@ export function OAuthClientForm(props: { · 22 unmodified lines | |||
| 223 | clientId: clientId.trim(), | 234 | clientId: clientId.trim(), |
| 224 | clientSecret: clientSecret.trim(), | 235 | clientSecret: clientSecret.trim(), |
| 225 | resource, | 236 | resource, |
| 237 | originIntegration: originIntegration ?? null, | ||
| 226 | }, | 238 | }, |
| 227 | reactivityKeys: oauthClientWriteKeys, | 239 | reactivityKeys: oauthClientWriteKeys, |
| 228 | }); | 240 | }); |
| @@ -1,6 +1,11 @@ | |||
| 1 | import { useAtomValue } from "@effect/atom-react"; | 1 | import { useAtomValue } from "@effect/atom-react"; |
| 2 | import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; | 2 | import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; |
| 3 | import { OAuthClientSlug, type Owner } from "@executor-js/sdk/shared"; | 3 | import { |
| 4 | type IntegrationSlug, | ||
| 5 | OAuthClientSlug, | ||
| 6 | type OAuthClientOrigin, | ||
| 7 | type Owner, | ||
| 8 | } from "@executor-js/sdk/shared"; | ||
| 4 | import { getDomain } from "tldts"; | 9 | import { getDomain } from "tldts"; |
| 5 | 10 | ||
| 6 | import { oauthClientsOptimisticAtom } from "../api/atoms"; | 11 | import { oauthClientsOptimisticAtom } from "../api/atoms"; |
| @@ -10,11 +15,16 @@ import { oauthClientsOptimisticAtom } from "../api/atoms"; · 3 unmodified lines | |||
| 10 | // | 15 | // |
| 11 | // An owner can register MANY apps; each is a distinct owner-scoped `oauth_client` | 16 | // An owner can register MANY apps; each is a distinct owner-scoped `oauth_client` |
| 12 | // row with its own slug. The connect flow lists the apps usable for an | 17 | // row with its own slug. The connect flow lists the apps usable for an |
| 13 | // integration (owner-visible clients whose OAuth endpoints share a registrable | 18 | // integration and lets the user pick one or register a new one. An app is usable |
| 14 | // root domain with the integration's declared endpoints when known) and lets the | 19 | // for an integration when EITHER: |
| 15 | // user pick one or register a new one. User-owned apps are listed before | 20 | // - it was registered for that integration (`origin.integration`), OR |
| 16 | // workspace ones. When NOTHING matches a declared endpoint, the picker shows an | 21 | // - its OAuth endpoints share a registrable root domain with the integration's |
| 17 | // empty state + a "register an app" CTA rather than unrelated providers' apps. | 22 | // declared endpoints (so one app can be reused across sibling integrations). |
| 23 | // User-owned apps are listed before workspace ones. When the integration declares | ||
| 24 | // no endpoints AND owns no app (e.g. an MCP source whose endpoints are discovered | ||
| 25 | // at connect time), the picker shows an empty state + a "register an app" CTA | ||
| 26 | // rather than every unrelated provider's app — that broad leak was the bug behind | ||
| 27 | // "Datadog apps showing under Atlassian". | ||
| 18 | // --------------------------------------------------------------------------- | 28 | // --------------------------------------------------------------------------- |
| 19 | 29 | ||
| 20 | export interface OAuthClientOption { | 30 | export interface OAuthClientOption { |
| @@ -24,6 +34,10 @@ export interface OAuthClientOption { · 3 unmodified lines | |||
| 24 | readonly authorizationUrl: string; | 34 | readonly authorizationUrl: string; |
| 25 | readonly tokenUrl: string; | 35 | readonly tokenUrl: string; |
| 26 | readonly clientId: string; | 36 | readonly clientId: string; |
| 37 | /** Provenance — `origin.integration` is the integration this app was registered | ||
| 38 | * for (DCR or manual), which the picker uses to surface the app for that | ||
| 39 | * integration even when no static endpoints are declared to match on. */ | ||
| 40 | readonly origin?: OAuthClientOrigin; | ||
| 27 | } | 41 | } |
| 28 | 42 | ||
| 29 | const hostOf = (url: string): string | undefined => { | 43 | const hostOf = (url: string): string | undefined => { |
| @@ -39,12 +53,18 @@ const hostOf = (url: string): string | undefined => { · 9 unmodified lines | |||
| 39 | * `accounts.google.com` → `google.com`. Falls back to the full host for | 53 | * `accounts.google.com` → `google.com`. Falls back to the full host for |
| 40 | * localhost / IP literals (where `tldts.getDomain` returns null) so local-dev | 54 | * localhost / IP literals (where `tldts.getDomain` returns null) so local-dev |
| 41 | * MCP servers still match by exact host. Returns undefined for unparseable URLs. */ | 55 | * MCP servers still match by exact host. Returns undefined for unparseable URLs. */ |
| 42 | const getRootDomain = (url: string): string | undefined => { | 56 | export const getRootDomain = (url: string): string | undefined => { |
| 43 | const root = getDomain(url); | 57 | const root = getDomain(url); |
| 44 | if (root) return root.toLowerCase(); | 58 | if (root) return root.toLowerCase(); |
| 45 | return hostOf(url); | 59 | return hostOf(url); |
| 46 | }; | 60 | }; |
| 47 | 61 | ||
| 62 | /** The integration an app was registered for, from either origin kind (or | ||
| 63 | * undefined). Used to surface an app for its integration even when no static | ||
| 64 | * endpoints are declared to match on. */ | ||
| 65 | const appIntegration = (app: OAuthClientOption): string | undefined => | ||
| 66 | app.origin?.integration == null ? undefined : String(app.origin.integration); | ||
| 67 | |||
| 48 | export interface UseOAuthClientsResult { | 68 | export interface UseOAuthClientsResult { |
| 49 | /** Apps usable for this integration, user-owned first. When an endpoint was | 69 | /** Apps usable for this integration, user-owned first. When an endpoint was |
| 50 | * declared but nothing matched, this is EMPTY (the unmatched apps move to | 70 | * declared but nothing matched, this is EMPTY (the unmatched apps move to |
| @@ -73,6 +93,10 @@ export interface UseOAuthClientsResult { · 22 unmodified lines | |||
| 73 | * an endpoint was declared and nothing matched. Equals `!endpointMatched` | 93 | * an endpoint was declared and nothing matched. Equals `!endpointMatched` |
| 74 | * once loaded. */ | 94 | * once loaded. */ |
| 75 | readonly displayRegisterCTA: boolean; | 95 | readonly displayRegisterCTA: boolean; |
| 96 | /** Every owner-visible app, UNFILTERED. Callers that need the whole catalog | ||
| 97 | * (slug-collision avoidance when minting, duplicate-DCR reuse detection) read | ||
| 98 | * this rather than the integration-filtered `clients`. */ | ||
| 99 | readonly allClients: readonly OAuthClientOption[]; | ||
| 76 | } | 100 | } |
| 77 | 101 | ||
| 78 | /** Sort apps user-owned first (so the user's own apps surface before shared | 102 | /** Sort apps user-owned first (so the user's own apps surface before shared |
| @@ -83,44 +107,59 @@ const sortUserFirst = (apps: readonly OAuthClientOption[]): readonly OAuthClient · 4 unmodified lines | |||
| 83 | ); | 107 | ); |
| 84 | 108 | ||
| 85 | /** | 109 | /** |
| 86 | * Pure matcher (no React/atoms) — split owner-visible apps into the ones that | 110 | * Pure matcher (no React/atoms) — split owner-visible apps into the ones usable |
| 87 | * match the integration's declared OAuth endpoints and the ones that don't. | 111 | * for an integration and the ones that aren't. |
| 88 | * | 112 | * |
| 89 | * Matching is by REGISTRABLE ROOT DOMAIN ("tld+1"). When the integration | 113 | * An app is usable when EITHER signal holds: |
| 90 | * declares a token endpoint, an app must match by token endpoint root; the token | 114 | * - INTEGRATION: it was registered for this integration (`origin.integration` |
| 91 | * endpoint is what the SDK will call during code exchange/refresh and avoids | 115 | * equals `integration`). This is the only signal an MCP source has, since its |
| 92 | * authorize-root coincidences. When only an authorization endpoint is declared, | 116 | * OAuth endpoints are discovered at connect time and never declared. |
| 93 | * the authorize root is used as the fallback compatibility signal. Unrelated | 117 | * - ENDPOINT: the integration declares an endpoint and the app's endpoints share |
| 94 | * providers (different root) never match. | 118 | * a REGISTRABLE ROOT DOMAIN ("tld+1"). The token endpoint is preferred (it is |
| 119 | * what the SDK calls during code exchange/refresh, avoiding authorize-root | ||
| 120 | * coincidences); the authorize root is the fallback when only it is declared. | ||
| 121 | * This lets one app be reused across sibling integrations of one provider. | ||
| 95 | * | 122 | * |
| 96 | * When the integration declares no endpoints, every app is "matched" (no filter). | 123 | * When NEITHER an integration nor any endpoint is known (no context at all), the |
| 124 | * filter cannot discriminate and every app is "matched" (legacy fall-through). | ||
| 125 | * Crucially, when an integration IS known but declares no endpoints, apps are | ||
| 126 | * matched by integration only — unrelated providers' apps no longer leak in. | ||
| 97 | */ | 127 | */ |
| 98 | export function selectClientsForEndpoints( | 128 | export function selectClientsForEndpoints( |
| 99 | all: readonly OAuthClientOption[], | 129 | all: readonly OAuthClientOption[], |
| 100 | endpoints: { readonly tokenUrl?: string; readonly authorizationUrl?: string }, | 130 | endpoints: { |
| 131 | readonly integration?: string; | ||
| 132 | readonly tokenUrl?: string; | ||
| 133 | readonly authorizationUrl?: string; | ||
| 134 | }, | ||
| 101 | ): { | 135 | ): { |
| 102 | readonly matched: readonly OAuthClientOption[]; | 136 | readonly matched: readonly OAuthClientOption[]; |
| 103 | readonly unmatched: readonly OAuthClientOption[]; | 137 | readonly unmatched: readonly OAuthClientOption[]; |
| 104 | readonly endpointMatched: boolean; | 138 | readonly endpointMatched: boolean; |
| 105 | } { | 139 | } { |
| 140 | const wantedIntegration = endpoints.integration; | ||
| 106 | const wantedTokenRoot = endpoints.tokenUrl ? getRootDomain(endpoints.tokenUrl) : undefined; | 141 | const wantedTokenRoot = endpoints.tokenUrl ? getRootDomain(endpoints.tokenUrl) : undefined; |
| 107 | const wantedAuthorizationRoot = endpoints.authorizationUrl | 142 | const wantedAuthorizationRoot = endpoints.authorizationUrl |
| 108 | ? getRootDomain(endpoints.authorizationUrl) | 143 | ? getRootDomain(endpoints.authorizationUrl) |
| 109 | : undefined; | 144 | : undefined; |
| 110 | // No declared endpoints → no filter; every app is usable. | 145 | // No discriminating signal at all → no filter; every app is usable. |
| 111 | if (!wantedTokenRoot && !wantedAuthorizationRoot) { | 146 | if (!wantedIntegration && !wantedTokenRoot && !wantedAuthorizationRoot) { |
| 112 | return { matched: sortUserFirst(all), unmatched: [], endpointMatched: true }; | 147 | return { matched: sortUserFirst(all), unmatched: [], endpointMatched: true }; |
| 113 | } | 148 | } |
| 114 | const matched: OAuthClientOption[] = []; | 149 | const matched: OAuthClientOption[] = []; |
| 115 | const unmatched: OAuthClientOption[] = []; | 150 | const unmatched: OAuthClientOption[] = []; |
| 116 | for (const app of all) { | 151 | for (const app of all) { |
| 152 | const fitsIntegration = | ||
| 153 | wantedIntegration !== undefined && appIntegration(app) === wantedIntegration; | ||
| 117 | const appTokenRoot = getRootDomain(app.tokenUrl); | 154 | const appTokenRoot = getRootDomain(app.tokenUrl); |
| 118 | const appAuthorizationRoot = getRootDomain(app.authorizationUrl); | 155 | const appAuthorizationRoot = getRootDomain(app.authorizationUrl); |
| 119 | const fits = wantedTokenRoot | 156 | const fitsEndpoint = wantedTokenRoot |
| 120 | ? appTokenRoot === wantedTokenRoot | 157 | ? appTokenRoot === wantedTokenRoot |
| 121 | : appAuthorizationRoot === wantedAuthorizationRoot || | 158 | : wantedAuthorizationRoot |
| 122 | appTokenRoot === wantedAuthorizationRoot; | 159 | ? appAuthorizationRoot === wantedAuthorizationRoot || |
| 123 | if (fits) matched.push(app); | 160 | appTokenRoot === wantedAuthorizationRoot |
| 161 | : false; | ||
| 162 | if (fitsIntegration || fitsEndpoint) matched.push(app); | ||
| 124 | else unmatched.push(app); | 163 | else unmatched.push(app); |
| 125 | } | 164 | } |
| 126 | return { | 165 | return { |
| @@ -131,6 +170,7 @@ export function selectClientsForEndpoints( · 4 unmodified lines | |||
| 131 | } | 170 | } |
| 132 | 171 | ||
| 133 | export function useOAuthClientsForIntegration(opts: { | 172 | export function useOAuthClientsForIntegration(opts: { |
| 173 | readonly integration?: IntegrationSlug; | ||
| 134 | readonly tokenUrl?: string; | 174 | readonly tokenUrl?: string; |
| 135 | readonly authorizationUrl?: string; | 175 | readonly authorizationUrl?: string; |
| 136 | }): UseOAuthClientsResult { | 176 | }): UseOAuthClientsResult { |
| @@ -146,11 +186,13 @@ export function useOAuthClientsForIntegration(opts: { · 9 unmodified lines | |||
| 146 | loading: true, | 186 | loading: true, |
| 147 | endpointMatched: true, | 187 | endpointMatched: true, |
| 148 | displayRegisterCTA: false, | 188 | displayRegisterCTA: false, |
| 189 | allClients: [], | ||
| 149 | }; | 190 | }; |
| 150 | } | 191 | } |
| 151 | 192 | ||
| 152 | const all = clientsResult.value as readonly OAuthClientOption[]; | 193 | const all = clientsResult.value as readonly OAuthClientOption[]; |
| 153 | const { matched, unmatched, endpointMatched } = selectClientsForEndpoints(all, { | 194 | const { matched, unmatched, endpointMatched } = selectClientsForEndpoints(all, { |
| 195 | integration: opts.integration ? String(opts.integration) : undefined, | ||
| 154 | tokenUrl: opts.tokenUrl, | 196 | tokenUrl: opts.tokenUrl, |
| 155 | authorizationUrl: opts.authorizationUrl, | 197 | authorizationUrl: opts.authorizationUrl, |
| 156 | }); | 198 | }); |
| @@ -165,6 +207,7 @@ export function useOAuthClientsForIntegration(opts: { · 8 unmodified lines | |||
| 165 | loading: false, | 207 | loading: false, |
| 166 | endpointMatched, | 208 | endpointMatched, |
| 167 | displayRegisterCTA: !endpointMatched, | 209 | displayRegisterCTA: !endpointMatched, |
| 210 | allClients: all, | ||
| 168 | }; | 211 | }; |
| 169 | } | 212 | } |
| 170 | 213 | ||