From 402f8e542379f79af1aa7951050d9362f75f8189 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Thu, 22 Oct 2026 13:15:11 +0200 Subject: [PATCH 1/5] fix(integrations): finish OAuth and chat installs as the user who started them Every connect, app-install and chat account-link callback is public, and it used to redeem its state row and store the provider's grant under whatever org and user the row named. The authorize URL is transferable, so a tenant admin could send theirs to somebody else and receive that person's GitHub install, Cloudflare/PlanetScale/Supabase/Hazel tokens or chat workspace, and a member could attach somebody else's chat account to their own Maple user. The public callbacks now complete nothing. They redirect to the dashboard with the provider's query in the URL fragment, and the signed-in integrations page finishes the flow through `oauthComplete` / `chatComplete`. Every service redeems a state row only for the session's user and org (`OAuthStateInitiator`), burning it on a mismatch, so a forwarded URL finished by another account fails without storing anything. Random state, TTL, provider scoping, PKCE and external-resource uniqueness are unchanged. --- .oxlintrc.json | 2 +- apps/api/src/routes/chat-integration.http.ts | 152 +--- .../routes/integrations-callback-page.test.ts | 54 -- .../routes/integrations-callback.http.test.ts | 70 ++ .../src/routes/integrations-callback.http.ts | 755 ++---------------- .../routes/internal/integrations-complete.ts | 268 +++++++ .../src/routes/internal/integrations.http.ts | 23 + .../forwarded-callback.scene.test.ts | 94 +++ .../page/integrations/forwarded-callback.ts | 130 +++ apps/web/src/page/integrations/message.ts | 12 + apps/web/src/page/integrations/update.ts | 38 + apps/web/src/route/search.ts | 2 + .../auth/CloudflareOAuthService.test.ts | 15 +- .../services/auth/CloudflareOAuthService.ts | 6 +- .../src/services/auth/HazelOAuthService.ts | 6 +- .../auth/PlanetScaleOAuthService.test.ts | 35 +- .../services/auth/PlanetScaleOAuthService.ts | 6 +- .../auth/SupabaseOAuthService.test.ts | 3 +- .../src/services/auth/SupabaseOAuthService.ts | 6 +- .../auth/oauth/connection-helpers.test.ts | 70 +- .../services/auth/oauth/connection-helpers.ts | 7 + .../services/auth/oauth/state-initiator.ts | 23 + .../integrations/ChatWorkspaceService.test.ts | 95 ++- .../integrations/ChatWorkspaceService.ts | 37 +- .../PlanetScaleConnectionService.test.ts | 2 +- .../PlanetScaleDiscoveryService.test.ts | 6 +- .../integrations/PlanetScaleService.test.ts | 6 +- .../SupabaseConnectionService.test.ts | 2 +- .../SupabaseInsightsService.test.ts | 6 +- .../integrations/SupabaseLogsService.test.ts | 6 +- .../supabase/scrape-targets.test.ts | 2 +- .../vcs/vendor/github/GithubConnectService.ts | 17 + .../__tests__/GithubConnectService.test.ts | 77 +- packages/domain/src/http/integrations.ts | 60 ++ 34 files changed, 1134 insertions(+), 959 deletions(-) delete mode 100644 apps/api/src/routes/integrations-callback-page.test.ts create mode 100644 apps/api/src/routes/integrations-callback.http.test.ts create mode 100644 apps/api/src/routes/internal/integrations-complete.ts create mode 100644 apps/web/src/page/integrations/forwarded-callback.scene.test.ts create mode 100644 apps/web/src/page/integrations/forwarded-callback.ts create mode 100644 packages/backend/src/services/auth/oauth/state-initiator.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index 7b3d71f022..a9bc85fe38 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -285,7 +285,7 @@ "apps/api/src/runtime/service-graph.ts", "packages/backend/src/services/integrations/vcs/VcsProviderRegistry.ts", "apps/api/src/routes/internal/integrations.http.ts", - "apps/api/src/routes/integrations-callback.http.ts", + "apps/api/src/routes/internal/integrations-complete.ts", "packages/backend/src/services/integrations/vcs/vendor/**", "packages/backend/src/services/integrations/vcs/__tests__/**" ], diff --git a/apps/api/src/routes/chat-integration.http.ts b/apps/api/src/routes/chat-integration.http.ts index d967177732..92fb925ae4 100644 --- a/apps/api/src/routes/chat-integration.http.ts +++ b/apps/api/src/routes/chat-integration.http.ts @@ -1,16 +1,13 @@ import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/http" -import { - ChatConnectorId, - type IntegrationsConfigurationError, - type IntegrationsForbiddenError, - type IntegrationsNotFoundError, - type IntegrationsPersistenceError, - type IntegrationsUpstreamError, - type IntegrationsValidationError, -} from "@maple/domain/http" +import { ChatConnectorId } from "@maple/domain/http" import { Effect, Option, Schema } from "effect" import { Env } from "@maple/backend/platform/Env" -import { ChatWorkspaceService } from "@maple/backend/services/integrations/ChatWorkspaceService" +import { + callbackQueryOf, + dashboardCallbackUrl, + type DashboardCallbackKind, + forwardToDashboard, +} from "./integrations-callback.http" /** * The two public callbacks every chat connector redirects to: @@ -19,11 +16,10 @@ import { ChatWorkspaceService } from "@maple/backend/services/integrations/ChatW * account. The platform is a path *parameter*, so a new connector needs no new route — it * registers in the connector registry and these handlers resolve it by id. * - * The browser lands here after the user approves (or declines). The connector turns the query - * into a linked workspace or a linked account, and the browser goes back to the dashboard's - * integrations page with the outcome. Both are unauthenticated: the platform issues a top-level - * redirect, so there is no session to require, and the single-use state row carries everything - * the completion needs. + * Neither completes anything. The platform issues a top-level redirect, so the browser that lands + * here proves nothing about who started the install or link; the query is forwarded to the + * dashboard, which finishes it through `chatComplete` as the signed-in user, and the state row is + * redeemed only when that user and org started it. */ const CHAT_CALLBACK_PATH = "/oauth/chat/:connector/callback" @@ -32,125 +28,33 @@ const CHAT_IDENTITY_CALLBACK_PATH = "/oauth/chat/:connector/identity/callback" const decodeConnectorParam = Schema.decodeUnknownOption(ChatConnectorId) -/** - * Outcome as a closed set of codes rather than a message: the dashboard renders - * this inside an authenticated page, and a backend- or provider-authored string - * echoed there is a phishing surface. The web maps each code to its own copy. - */ -type ChatCallbackReason = "state" | "conflict" | "unconfigured" | "unknown" | "failed" - -/** Everything either completer can fail with — the tags `callback` maps to a reason code. */ -type ChatCallbackError = - | IntegrationsValidationError - | IntegrationsForbiddenError - | IntegrationsNotFoundError - | IntegrationsConfigurationError - | IntegrationsUpstreamError - | IntegrationsPersistenceError - -/** Where the browser lands afterwards: the connector's card, plus the outcome. */ -const buildAppRedirect = (appBaseUrl: string, params: Record): string => { - const base = appBaseUrl.replace(/\/$/, "") - return `${base}/integrations?${new URLSearchParams(params).toString()}` -} - /** The dashboard's catalog id for a connector's card (`integration-catalog.tsx`). */ const cardId = (connector: string): string => `chat-${connector}` export const ChatCallbackRouter = HttpRouter.use((router) => Effect.gen(function* () { - const chat = yield* ChatWorkspaceService const env = yield* Env + const appBaseUrl = env.MAPLE_APP_BASE_URL.replace(/\/$/, "") - const redirect = (params: Record) => - HttpServerResponse.redirect(buildAppRedirect(env.MAPLE_APP_BASE_URL, params)) - - const requestUrl = (req: HttpServerRequest.HttpServerRequest) => - Option.liftThrowable(() => new URL(req.url, "http://localhost"))() - - /** - * Both callbacks, which differ only in what completes them and what they say afterwards. - * - * The outcome rides under its own query parameter — `chat` for the workspace install, - * `chat_identity` for a member's own account — and every failure maps to the same closed - * set of reason codes, because a backend- or provider-authored string echoed inside an - * authenticated page is a phishing surface. - */ - const callback = (options: { - readonly span: string - readonly outcome: string - readonly log: string - readonly complete: ( - connector: ChatConnectorId, - params: URLSearchParams, - ) => Effect.Effect - readonly params: (result: A) => Record - }) => - Effect.fn(options.span)(function* (req: HttpServerRequest.HttpServerRequest) { - const failed = (connector: string, reason: ChatCallbackReason) => - redirect({ - ...(connector === "" ? undefined : { integration: cardId(connector) }), - [options.outcome]: "error", - chat_reason: reason, - }) - + const callback = (span: string, kind: DashboardCallbackKind, outcome: string) => + Effect.fn(span)(function* (req: HttpServerRequest.HttpServerRequest) { const routeParams = yield* HttpRouter.params - const connectorOption = decodeConnectorParam(routeParams.connector) - if (Option.isNone(connectorOption)) return failed("", "unknown") - const connector = connectorOption.value - const urlOption = requestUrl(req) - if (Option.isNone(urlOption)) return failed(connector, "failed") - - // The whole query string goes to the connector: which parameters carry the - // authorization is the connector's business, not this route's. - return yield* options.complete(connector, urlOption.value.searchParams).pipe( - Effect.tapError((error) => - Effect.logError(options.log, { connector, tag: error._tag, message: error.message }), - ), - Effect.map((result) => - redirect({ integration: cardId(connector), ...options.params(result) }), - ), - Effect.catchTags({ - "@maple/http/errors/IntegrationsValidationError": () => - Effect.succeed(failed(connector, "state")), - "@maple/http/errors/IntegrationsForbiddenError": () => - Effect.succeed(failed(connector, "conflict")), - "@maple/http/errors/IntegrationsNotFoundError": () => - Effect.succeed(failed(connector, "unknown")), - "@maple/http/errors/IntegrationsConfigurationError": () => - Effect.succeed(failed(connector, "unconfigured")), - "@maple/http/errors/IntegrationsUpstreamError": () => - Effect.succeed(failed(connector, "failed")), - "@maple/http/errors/IntegrationsPersistenceError": () => - Effect.succeed(failed(connector, "failed")), - }), + const connector = decodeConnectorParam(routeParams.connector) + if (Option.isNone(connector)) { + const search = new URLSearchParams({ [outcome]: "error", chat_reason: "unknown" }) + return HttpServerResponse.redirect(`${appBaseUrl}/integrations?${search.toString()}`) + } + yield* Effect.annotateCurrentSpan({ "maple.chat.connector": connector.value }) + return forwardToDashboard( + dashboardCallbackUrl(appBaseUrl, cardId(connector.value), kind, callbackQueryOf(req)), ) }) - const handle = callback({ - span: "ChatOAuth.callback", - outcome: "chat", - log: "Chat install callback failed", - complete: (connector, params) => chat.completeInstall(connector, params), - params: (result) => ({ chat: "connected", chat_workspace: result.name }), - }) - - const handleIdentity = callback({ - span: "ChatOAuth.identityCallback", - outcome: "chat_identity", - log: "Chat account link callback failed", - complete: (connector, params) => chat.completeLink(connector, params), - params: (result) => ({ - chat_identity: "linked", - // The platform's own display string, and only when it reported one. Untrusted, - // like the workspace name — the page clamps it. - ...(result.displayName === undefined - ? undefined - : { chat_identity_name: result.displayName }), - }), - }) - - yield* router.add("GET", CHAT_CALLBACK_PATH, handle) - yield* router.add("GET", CHAT_IDENTITY_CALLBACK_PATH, handleIdentity) + yield* router.add("GET", CHAT_CALLBACK_PATH, callback("ChatOAuth.callback", "chat-install", "chat")) + yield* router.add( + "GET", + CHAT_IDENTITY_CALLBACK_PATH, + callback("ChatOAuth.identityCallback", "chat-identity", "chat_identity"), + ) }), ) diff --git a/apps/api/src/routes/integrations-callback-page.test.ts b/apps/api/src/routes/integrations-callback-page.test.ts deleted file mode 100644 index 395851ea18..0000000000 --- a/apps/api/src/routes/integrations-callback-page.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { assert, describe, it } from "@effect/vitest" -import { renderCallbackPage } from "./integrations-callback.http" - -/** - * The callback page is served from the API origin, so the stored `returnTo` is - * the one attacker-influenced value that reaches an href here. HTML escaping - * alone leaves `javascript:` intact — these pin the scheme/authority rules. - */ -const render = (returnTo: string | null, targetOrigin = "https://web.maple.dev") => - renderCallbackPage({ - status: "success", - message: "Connected", - returnTo, - messageType: "maple:integration:hazel", - label: "Hazel", - targetOrigin, - }) - -describe("renderCallbackPage returnTo sink", () => { - it("renders a dashboard-origin link for a relative path", () => { - const html = render("/integrations?connected=1") - assert.include(html, `href="https://web.maple.dev/integrations?connected=1"`) - assert.notInclude(html, "Return link blocked") - }) - - it.each([ - "javascript:alert(document.cookie)", - "JaVaScRiPt:alert(1)", - "data:text/html,", - "//evil.com/", - "/\\evil.com", - "https://evil.com/", - "%6a%61vascript:alert(1)", - ])("drops the link for %j", (value) => { - const html = render(value) - assert.notInclude(html, ` { - const html = render(null) - assert.notInclude(html, ` { - const html = render("/integrations", "*") - assert.notInclude(html, ` { + const routes = Layer.mergeAll(IntegrationsCallbackRouter, ChatCallbackRouter).pipe( + Layer.provide(Env.layer.pipe(Layer.provide(testConfig))), + ) + const { handler, dispose } = HttpRouter.toWebHandler(routes, { disableLogger: true }) + try { + return await handler(new Request(`https://api.example.com${path}`), Context.empty() as never) + } finally { + await dispose() + } +} + +describe("public OAuth callbacks", () => { + it.each([ + ["hazel", "code=c1&state=s1"], + ["cloudflare", "code=c1&state=s1"], + ["planetscale", "code=c1&state=s1"], + ["supabase", "code=c1&state=s1&iss=https%3A%2F%2Fapi.supabase.com"], + ["github", "installation_id=42&setup_action=install&code=c1&state=s1"], + ])("forwards the %s callback to the dashboard instead of completing it", async (provider, query) => { + const response = await get(`/api/integrations/${provider}/callback?${query}`) + expect(response.status).toBe(302) + expect(response.headers.get("location")).toBe( + `https://app.example.com/integrations?integration=${provider}&callback=oauth#${query}`, + ) + expect(response.headers.get("cache-control")).toBe("no-store") + expect(response.headers.get("referrer-policy")).toBe("no-referrer") + }) + + it.each([ + ["/oauth/chat/slack/callback", "chat-install"], + ["/oauth/chat/slack/identity/callback", "chat-identity"], + ])("forwards %s to the dashboard as %s", async (path, kind) => { + const response = await get(`${path}?code=c1&state=s1`) + expect(response.status).toBe(302) + expect(response.headers.get("location")).toBe( + `https://app.example.com/integrations?integration=chat-slack&callback=${kind}#code=c1&state=s1`, + ) + }) + + it("sends an unknown chat connector back with the error code, forwarding nothing", async () => { + const response = await get("/oauth/chat/Not-A-Connector/callback?code=c1&state=s1") + expect(response.status).toBe(302) + expect(response.headers.get("location")).toBe( + "https://app.example.com/integrations?chat=error&chat_reason=unknown", + ) + }) +}) diff --git a/apps/api/src/routes/integrations-callback.http.ts b/apps/api/src/routes/integrations-callback.http.ts index 1fbd6cd8f8..fa94388720 100644 --- a/apps/api/src/routes/integrations-callback.http.ts +++ b/apps/api/src/routes/integrations-callback.http.ts @@ -1,717 +1,82 @@ import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/http" -import { - type IntegrationsConfigurationError, - type IntegrationsNotConnectedError, - type IntegrationsPersistenceError, - type IntegrationsRevokedError, - type IntegrationsUpstreamError, - type IntegrationsValidationError, - validateIntegrationReturnPath, -} from "@maple/domain/http" -import { Effect, Option } from "effect" +import { Effect } from "effect" import { Env } from "@maple/backend/platform/Env" -import { CloudflareAnalyticsService } from "@maple/backend/services/integrations/CloudflareAnalyticsService" -import { CloudflareOAuthService } from "@maple/backend/services/auth/CloudflareOAuthService" -import { PlanetScaleConnectionService } from "@maple/backend/services/integrations/PlanetScaleConnectionService" -import { - PLANETSCALE_CALLBACK_PATH, - PlanetScaleOAuthService, -} from "@maple/backend/services/auth/PlanetScaleOAuthService" -import { SupabaseConnectionService } from "@maple/backend/services/integrations/SupabaseConnectionService" -import { - SUPABASE_CALLBACK_PATH, - SupabaseOAuthService, -} from "@maple/backend/services/auth/SupabaseOAuthService" -import { GithubConnectService } from "@maple/backend/services/integrations/vcs/vendor/github/GithubConnectService" -import { HazelOAuthService } from "@maple/backend/services/auth/HazelOAuthService" -import { summarizeCause } from "@maple/backend/platform/describe-cause" +import { PLANETSCALE_CALLBACK_PATH } from "@maple/backend/services/auth/PlanetScaleOAuthService" +import { SUPABASE_CALLBACK_PATH } from "@maple/backend/services/auth/SupabaseOAuthService" export const HAZEL_CALLBACK_PATH = "/api/integrations/hazel/callback" export const GITHUB_CALLBACK_PATH = "/api/integrations/github/callback" export const CLOUDFLARE_CALLBACK_PATH = "/api/integrations/cloudflare/callback" -const HAZEL_MESSAGE_TYPE = "maple:integration:hazel" -const GITHUB_MESSAGE_TYPE = "maple:integration:github" -const CLOUDFLARE_MESSAGE_TYPE = "maple:integration:cloudflare" -const PLANETSCALE_MESSAGE_TYPE = "maple:integration:planetscale" -const SUPABASE_MESSAGE_TYPE = "maple:integration:supabase" - -const escapeHtml = (value: string) => - value - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'") - -// JSON.stringify does not escape `<` or `>`, so a payload value of -// `` would terminate the inline script -// block. Escape these characters and the U+2028 / U+2029 line separators -// (which are valid line terminators in JS but not in JSON) before -// interpolating into a ` - -` +/** How the dashboard finishes a forwarded callback (`IntegrationsSearch.callback`). */ +export type DashboardCallbackKind = "oauth" | "chat-install" | "chat-identity" + +/** + * Where a provider callback goes: the dashboard's integrations page, which finishes the flow as + * the signed-in user (`oauthComplete` / `chatComplete`). The provider's query rides in the + * fragment, so the authorization code never reaches a server log, a `Referer`, or the web Worker. + */ +export const dashboardCallbackUrl = ( + appBaseUrl: string, + integration: string, + kind: DashboardCallbackKind, + query: string, +): string => { + const search = new URLSearchParams({ integration, callback: kind }) + return `${appBaseUrl.replace(/\/$/, "")}/integrations?${search.toString()}#${query}` } -const htmlResponse = (body: string, status?: number) => { - const response = HttpServerResponse.html(body) - return status === undefined ? response : HttpServerResponse.setStatus(response, status) +/** The raw query string of a callback request, without its `?`. */ +export const callbackQueryOf = (req: HttpServerRequest.HttpServerRequest): string => { + const index = req.url.indexOf("?") + return index === -1 ? "" : req.url.slice(index + 1) } -type CallbackPageParams = { - status: "success" | "error" - message: string - returnTo: string | null - targetOrigin: string -} - -type OAuthCallbackError = - | IntegrationsConfigurationError - | IntegrationsValidationError - | IntegrationsUpstreamError - | IntegrationsNotConnectedError - | IntegrationsRevokedError - | IntegrationsPersistenceError - -const OAUTH_CALLBACK_ERROR_TAGS: ReadonlySet = new Set([ - "@maple/http/errors/IntegrationsConfigurationError", - "@maple/http/errors/IntegrationsValidationError", - "@maple/http/errors/IntegrationsUpstreamError", - "@maple/http/errors/IntegrationsNotConnectedError", - "@maple/http/errors/IntegrationsRevokedError", - "@maple/http/errors/IntegrationsPersistenceError", -]) - -const isOAuthCallbackError = (error: { readonly _tag: string }): error is OAuthCallbackError => - OAUTH_CALLBACK_ERROR_TAGS.has(error._tag) +/** + * A public callback completes nothing. The authorize URL behind it is transferable, so the + * browser that lands here proves nothing about which Maple user started the flow; only the + * dashboard's session does. `no-store` and `no-referrer` keep the forwarded code out of caches + * and out of the next page's `Referer`. + */ +export const forwardToDashboard = (location: string) => + HttpServerResponse.redirect(location, { + status: 302, + headers: { "cache-control": "no-store", "referrer-policy": "no-referrer" }, + }) -/** The callback page's copy for a connect failure. */ -const callbackFailureMessage = (label: string, error: OAuthCallbackError): string => { - switch (error._tag) { - case "@maple/http/errors/IntegrationsConfigurationError": - return `${label} integration is not configured in Maple` - // Validation/upstream messages are our own sanitized strings; showing them is actionable. - case "@maple/http/errors/IntegrationsValidationError": - case "@maple/http/errors/IntegrationsUpstreamError": - return error.message - case "@maple/http/errors/IntegrationsNotConnectedError": - return `${label} connection not found. Restart the connect flow.` - case "@maple/http/errors/IntegrationsRevokedError": - return `${label} rejected the authorization. Reconnect and try again.` - case "@maple/http/errors/IntegrationsPersistenceError": - return `Failed to complete the ${label} connection` - } -} +const PROVIDER_CALLBACKS = [ + { path: HAZEL_CALLBACK_PATH, provider: "hazel" }, + { path: GITHUB_CALLBACK_PATH, provider: "github" }, + { path: CLOUDFLARE_CALLBACK_PATH, provider: "cloudflare" }, + { path: PLANETSCALE_CALLBACK_PATH, provider: "planetscale" }, + { path: SUPABASE_CALLBACK_PATH, provider: "supabase" }, +] as const export const IntegrationsCallbackRouter = HttpRouter.use((router) => Effect.gen(function* () { - const hazel = yield* HazelOAuthService - const github = yield* GithubConnectService - const cloudflare = yield* CloudflareOAuthService - const cloudflareAnalytics = yield* CloudflareAnalyticsService - const planetscaleOAuth = yield* PlanetScaleOAuthService - const planetscaleConnection = yield* PlanetScaleConnectionService - const supabaseOAuth = yield* SupabaseOAuthService - const supabaseConnection = yield* SupabaseConnectionService const env = yield* Env - const dashboardTargetOrigin = resolveDashboardTargetOrigin(env.MAPLE_APP_BASE_URL) - const hazelCallbackPage = (params: Omit) => - renderCallbackPage({ - ...params, - targetOrigin: dashboardTargetOrigin, - messageType: HAZEL_MESSAGE_TYPE, - label: "Hazel", - }) - const githubCallbackPage = (params: Omit) => - renderCallbackPage({ - ...params, - targetOrigin: dashboardTargetOrigin, - messageType: GITHUB_MESSAGE_TYPE, - label: "GitHub", - }) - - const handle = Effect.fn("IntegrationsCallbackRouter.hazelOAuthCallback", { - kind: "server", - attributes: { "http.route": HAZEL_CALLBACK_PATH, "http.request.method": "GET" }, - })(function* (req: HttpServerRequest.HttpServerRequest) { - const urlOption = Option.liftThrowable(() => new URL(req.url, "http://localhost"))() - if (Option.isNone(urlOption)) { - return htmlResponse( - hazelCallbackPage({ - status: "error", - message: "Malformed callback URL", - returnTo: null, - }), - 400, - ) - } - const url = urlOption.value - const code = url.searchParams.get("code") - const state = url.searchParams.get("state") - const oauthError = url.searchParams.get("error") - const oauthErrorDescription = url.searchParams.get("error_description") ?? oauthError - - if (oauthError) { - return htmlResponse( - hazelCallbackPage({ - status: "error", - message: oauthErrorDescription || "Hazel returned an error", - returnTo: null, - }), - 400, - ) - } - - if (!code || !state) { - return htmlResponse( - hazelCallbackPage({ - status: "error", - message: "Missing code or state in callback", - returnTo: null, - }), - 400, - ) - } - - return yield* hazel.completeConnect(code, state).pipe( - // The callback page reduces failures to short human copy — make sure the real - // cause still lands in the server log for diagnosis. - Effect.tapError((error) => - Effect.logError("Hazel OAuth completeConnect failed", { - tag: error._tag, - message: error.message, - }), - ), - Effect.map((result) => - htmlResponse( - hazelCallbackPage({ - status: "success", - message: "You can close this window and return to Maple.", - returnTo: result.returnTo, - }), - ), - ), - Effect.catchTag("@maple/http/errors/IntegrationsValidationError", (error) => - Effect.succeed( - htmlResponse( - hazelCallbackPage({ - status: "error", - message: error.message, - returnTo: null, - }), - 400, - ), - ), - ), - Effect.catchTags({ - "@maple/http/errors/IntegrationsUpstreamError": () => - Effect.succeed( - htmlResponse( - hazelCallbackPage({ - status: "error", - message: "Failed to complete Hazel connection", - returnTo: null, - }), - 400, - ), - ), - "@maple/http/errors/IntegrationsPersistenceError": () => - Effect.succeed( - htmlResponse( - hazelCallbackPage({ - status: "error", - message: "Failed to complete Hazel connection", - returnTo: null, - }), - 400, - ), - ), - }), - ) - }) - - yield* router.add("GET", HAZEL_CALLBACK_PATH, handle) - - // Server-kind with the HTTP identity attributes stamped by hand: the auto - // server span is suppressed for OAuth callbacks (it would record the - // authorization `code` and connect `state` in `url.full` / `url.query` — - // see ApiObservabilityLive), so this span is the callback's only trace root. - // It carries the route and outcome, never the query string. - const handleGithub = Effect.fn("IntegrationsCallbackRouter.githubOAuthCallback", { - kind: "server", - attributes: { "http.route": GITHUB_CALLBACK_PATH, "http.request.method": "GET" }, - })( - function* (req: HttpServerRequest.HttpServerRequest) { - const urlOption = Option.liftThrowable(() => new URL(req.url, "http://localhost"))() - if (Option.isNone(urlOption)) { - return htmlResponse( - githubCallbackPage({ - status: "error", - message: "Malformed callback URL", - returnTo: null, - }), - 400, + yield* Effect.forEach(PROVIDER_CALLBACKS, ({ path, provider }) => + router.add( + "GET", + path, + // Server-kind with the HTTP identity attributes stamped by hand: the auto server span + // is suppressed for OAuth callbacks (it would record the authorization `code` and the + // connect `state` in `url.full` / `url.query`, see ApiObservabilityLive), so this span + // is the callback's only trace root. It never carries the query string. + Effect.fn("IntegrationsCallbackRouter.oauthCallback", { + kind: "server", + attributes: { + "http.route": path, + "http.request.method": "GET", + "maple.integration.provider": provider, + }, + })(function* (req: HttpServerRequest.HttpServerRequest) { + return forwardToDashboard( + dashboardCallbackUrl(env.MAPLE_APP_BASE_URL, provider, "oauth", callbackQueryOf(req)), ) - } - const url = urlOption.value - const installationId = url.searchParams.get("installation_id") - const setupAction = url.searchParams.get("setup_action") - const state = url.searchParams.get("state") - // Present only with OAuth-on-install enabled; proves the user owns the install. - const code = url.searchParams.get("code") ?? undefined - const oauthError = url.searchParams.get("error") - const oauthErrorDescription = url.searchParams.get("error_description") ?? oauthError - - if (oauthError) { - return htmlResponse( - githubCallbackPage({ - status: "error", - message: oauthErrorDescription || "GitHub returned an error", - returnTo: null, - }), - 400, - ) - } - - // `setup_action=request` → the org requires admin approval; the - // installation is pending and carries no usable installation_id yet. - if (!installationId) { - return htmlResponse( - githubCallbackPage({ - status: "error", - message: - setupAction === "request" - ? "Installation requested — an org admin must approve it on GitHub, then reconnect." - : "Missing installation_id in callback", - returnTo: null, - }), - 400, - ) - } - - if (!state) { - return htmlResponse( - githubCallbackPage({ - status: "error", - message: - "Missing state in callback — GitHub did not return it. Restart the connection from the Maple dashboard.", - returnTo: null, - }), - 400, - ) - } - - return yield* github.completeConnect(installationId, state, code).pipe( - // The callback page reduces failures to short human copy — make sure the real - // cause still lands in the server log for diagnosis. - Effect.tapError((error) => - Effect.logError("GitHub OAuth completeConnect failed", { - tag: error._tag, - message: error.message, - }), - ), - Effect.map((result) => - htmlResponse( - githubCallbackPage({ - status: "success", - message: "You can close this window and return to Maple.", - returnTo: result.returnTo, - }), - ), - ), - Effect.catchTags({ - "@maple/http/errors/IntegrationsValidationError": (error) => - Effect.succeed( - htmlResponse( - githubCallbackPage({ - status: "error", - message: error.message, - returnTo: null, - }), - 400, - ), - ), - "@maple/http/errors/IntegrationsUpstreamError": () => - Effect.succeed( - htmlResponse( - githubCallbackPage({ - status: "error", - message: "Failed to complete GitHub connection", - returnTo: null, - }), - 400, - ), - ), - "@maple/http/errors/IntegrationsPersistenceError": () => - Effect.succeed( - htmlResponse( - githubCallbackPage({ - status: "error", - message: "Failed to complete GitHub connection", - returnTo: null, - }), - 400, - ), - ), - }), - ) - }, - // Every branch above returns a response rather than failing, so the status - // is the only signal separating a completed connect from a rejection. - Effect.tap((response) => - Effect.annotateCurrentSpan({ "http.response.status_code": response.status }), + }), ), ) - - yield* router.add("GET", GITHUB_CALLBACK_PATH, handleGithub) - - /** - * One OAuth popup callback: read `code`/`state`, run `complete`, and reduce its typed failures - * to short copy on the page (the real cause goes to the log). - */ - const oauthCallbackRoute = (options: { - readonly path: HttpRouter.PathInput - /** `maple.integration.provider` on the shared, fixed-name callback span. */ - readonly provider: string - readonly label: string - readonly messageType: string - /** Extra callback checks; a returned message refuses the callback. */ - readonly validate?: (url: URL) => string | null - readonly complete: ( - code: string, - state: string, - ) => Effect.Effect< - { readonly returnTo: string | null; readonly message: string }, - OAuthCallbackError | E - > - }) => { - const { label } = options - const page = (params: Omit, status?: number) => - htmlResponse( - renderCallbackPage({ - ...params, - targetOrigin: dashboardTargetOrigin, - messageType: options.messageType, - label, - }), - status, - ) - const errorPage = (message: string) => page({ status: "error", message, returnTo: null }, 400) - - const handle = Effect.fn("IntegrationsCallbackRouter.oauthCallback", { - kind: "server", - attributes: { - "http.route": options.path, - "http.request.method": "GET", - "maple.integration.provider": options.provider, - }, - })( - function* (req: HttpServerRequest.HttpServerRequest) { - const urlOption = Option.liftThrowable(() => new URL(req.url, "http://localhost"))() - if (Option.isNone(urlOption)) { - return errorPage("Malformed callback URL") - } - const url = urlOption.value - const code = url.searchParams.get("code") - const state = url.searchParams.get("state") - const oauthError = url.searchParams.get("error") - const oauthErrorDescription = url.searchParams.get("error_description") ?? oauthError - - if (oauthError) { - return errorPage(oauthErrorDescription || `${label} returned an error`) - } - if (!code || !state) { - return errorPage("Missing code or state in callback") - } - const refused = options.validate?.(url) ?? null - if (refused !== null) return errorPage(refused) - - return yield* options.complete(code, state).pipe( - // The callback page reduces failures to short human copy; keep the cause in the log. - Effect.tapError((error) => - Effect.logError(`${label} OAuth completeConnect failed`, { - tag: error._tag, - message: error.message, - }), - ), - Effect.map(({ returnTo, message }) => page({ status: "success", message, returnTo })), - // Other failures (e.g. PlanetScale's scrape-target errors) stay failures of the route. - Effect.catchIf(isOAuthCallbackError, (error) => - Effect.succeed(errorPage(callbackFailureMessage(label, error))), - ), - ) - }, - // Every branch returns a response, so the status is the only signal of a rejection. - Effect.tap((response) => - Effect.annotateCurrentSpan({ "http.response.status_code": response.status }), - ), - ) - return router.add("GET", options.path, handle) - } - - yield* oauthCallbackRoute({ - path: CLOUDFLARE_CALLBACK_PATH, - provider: "cloudflare", - label: "Cloudflare", - messageType: CLOUDFLARE_MESSAGE_TYPE, - complete: (code, state) => - cloudflare.completeConnect(code, state).pipe( - // Reconnect writes fresh tokens, but rows a prior revoked-token error disabled - // (`recordOrgError(..., { disable: true })`) have no other re-enable path for the - // account-scoped workers anchor row. Clearing that state must never fail the page. - Effect.tap((result) => - cloudflareAnalytics.resetOrgState(result.orgId).pipe( - Effect.catchCause((cause) => - Effect.logWarning("cloudflare post-connect state reset failed", { - orgId: result.orgId, - error: summarizeCause(cause), - }), - ), - ), - ), - // The prime poll (discovery + a first window) deliberately does NOT run here: holding - // the callback made the popup sit blank long enough to read as a hang and be closed. - // The dashboard calls `cloudflarePrime` from a tab that stays open instead. - Effect.map((result) => ({ - returnTo: result.returnTo, - message: "You can close this window and return to Maple.", - })), - ), - }) - - yield* oauthCallbackRoute({ - path: PLANETSCALE_CALLBACK_PATH, - provider: "planetscale", - label: "PlanetScale", - messageType: PLANETSCALE_MESSAGE_TYPE, - complete: (code, state) => - planetscaleOAuth.completeConnect(code, state).pipe( - // Single-org grants finish here (bind + provision the scrape target); multi-org grants - // leave the org picker to the dashboard. A finalize failure (e.g. a missing - // read_metrics_endpoints scope) surfaces on the page, where the user can act on it. - Effect.flatMap((result) => - result.organizations.length === 1 - ? planetscaleConnection - .finalizeOrgSelection(result.orgId, { - organization: result.organizations[0]!.name, - }) - .pipe( - Effect.map(() => ({ - returnTo: result.returnTo, - message: `Connected to ${result.organizations[0]!.name}. You can close this window and return to Maple.`, - })), - ) - : Effect.succeed({ - returnTo: result.returnTo, - message: - "Authorization complete. Choose which PlanetScale organization to connect back in Maple.", - }), - ), - ), - }) - - // RFC 9207 mix-up defense: Supabase names itself in `iss`; a different issuer is refused. - const supabaseIssuer = supabaseOAuth.apiBaseUrl - yield* oauthCallbackRoute({ - path: SUPABASE_CALLBACK_PATH, - provider: "supabase", - label: "Supabase", - messageType: SUPABASE_MESSAGE_TYPE, - validate: (url) => { - const issuer = url.searchParams.get("iss") - return issuer !== null && issuer.replace(/\/$/, "") !== supabaseIssuer - ? "The authorization came from an unexpected issuer. Restart the connect flow." - : null - }, - // One grant is one Supabase organization: bind it and return. The first discovery runs - // from the dashboard's sync (or the hourly cron), so the popup never waits on it. - complete: (code, state) => - supabaseOAuth.completeConnect(code, state).pipe( - Effect.flatMap((result) => - supabaseConnection - .finalizeConnect(result.orgId, result.connectedByUserId, result.organization) - .pipe( - Effect.as({ - returnTo: result.returnTo, - message: `Connected to ${result.organization.name}. You can close this window and return to Maple.`, - }), - ), - ), - ), - }) }), ) diff --git a/apps/api/src/routes/internal/integrations-complete.ts b/apps/api/src/routes/internal/integrations-complete.ts new file mode 100644 index 0000000000..fa2a5a5401 --- /dev/null +++ b/apps/api/src/routes/internal/integrations-complete.ts @@ -0,0 +1,268 @@ +import { + type ChatCallbackKind, + type ChatCallbackReason, + type ChatConnectorId, + ChatCompleteResponse, + type OAuthCallbackProvider, + IntegrationReturnPath, + OAuthCompleteResponse, +} from "@maple/domain/http" +import { Effect, Option, Schema } from "effect" +import type { OAuthStateInitiator } from "@maple/backend/services/auth/oauth/state-initiator" +import { CloudflareOAuthService } from "@maple/backend/services/auth/CloudflareOAuthService" +import { HazelOAuthService } from "@maple/backend/services/auth/HazelOAuthService" +import { PlanetScaleOAuthService } from "@maple/backend/services/auth/PlanetScaleOAuthService" +import { SupabaseOAuthService } from "@maple/backend/services/auth/SupabaseOAuthService" +import { CloudflareAnalyticsService } from "@maple/backend/services/integrations/CloudflareAnalyticsService" +import { ChatWorkspaceService } from "@maple/backend/services/integrations/ChatWorkspaceService" +import { PlanetScaleConnectionService } from "@maple/backend/services/integrations/PlanetScaleConnectionService" +import { SupabaseConnectionService } from "@maple/backend/services/integrations/SupabaseConnectionService" +import { GithubConnectService } from "@maple/backend/services/integrations/vcs/vendor/github/GithubConnectService" +import { summarizeCause } from "@maple/backend/platform/describe-cause" + +const LABELS = { + hazel: "Hazel", + github: "GitHub", + cloudflare: "Cloudflare", + planetscale: "PlanetScale", + supabase: "Supabase", +} satisfies Record + +const decodeReturnPath = Schema.decodeUnknownOption(IntegrationReturnPath) + +const CLOSE_MESSAGE = "You can close this window and return to Maple." + +type Completed = { readonly returnTo: string | null; readonly message: string } + +/** Shown copy for a failed completion. Validation and upstream messages are our own sanitized strings. */ +const failureMessage = (label: string, error: { readonly _tag: string; readonly message: string }) => { + switch (error._tag) { + case "@maple/http/errors/IntegrationsConfigurationError": + return `${label} integration is not configured in Maple` + case "@maple/http/errors/IntegrationsValidationError": + case "@maple/http/errors/IntegrationsUpstreamError": + return error.message + case "@maple/http/errors/IntegrationsNotConnectedError": + return `${label} connection not found. Restart the connect flow.` + case "@maple/http/errors/IntegrationsRevokedError": + return `${label} rejected the authorization. Reconnect and try again.` + default: + return `Failed to complete the ${label} connection` + } +} + +const CHAT_REASONS = new Map([ + ["@maple/http/errors/IntegrationsValidationError", "state"], + ["@maple/http/errors/IntegrationsForbiddenError", "conflict"], + ["@maple/http/errors/IntegrationsNotFoundError", "unknown"], + ["@maple/http/errors/IntegrationsConfigurationError", "unconfigured"], +]) + +/** + * Finishing a connect, install or account link the public callback forwarded to the dashboard. + * Every service redeems the state row only for `initiator`, the signed-in session calling here. + */ +export const makeIntegrationCompleter = Effect.gen(function* () { + const hazel = yield* HazelOAuthService + const github = yield* GithubConnectService + const cloudflare = yield* CloudflareOAuthService + const cloudflareAnalytics = yield* CloudflareAnalyticsService + const planetscaleOAuth = yield* PlanetScaleOAuthService + const planetscaleConnection = yield* PlanetScaleConnectionService + const supabaseOAuth = yield* SupabaseOAuthService + const supabaseConnection = yield* SupabaseConnectionService + const chat = yield* ChatWorkspaceService + // RFC 9207 mix-up defense: Supabase names itself in `iss`; a different issuer is refused. + const supabaseIssuer = supabaseOAuth.apiBaseUrl + + const complete = ( + initiator: OAuthStateInitiator, + provider: OAuthCallbackProvider, + code: string, + state: string, + params: URLSearchParams, + ) => { + switch (provider) { + case "hazel": + return hazel.completeConnect(initiator, code, state).pipe( + Effect.map((result): Completed => ({ + returnTo: result.returnTo, + message: CLOSE_MESSAGE, + })), + ) + case "cloudflare": + return cloudflare.completeConnect(initiator, code, state).pipe( + // Reconnect writes fresh tokens, but rows a prior revoked-token error disabled have no + // other re-enable path for the account-scoped workers anchor row. Never fails the flow. + Effect.tap((result) => + cloudflareAnalytics.resetOrgState(result.orgId).pipe( + Effect.catchCause((cause) => + Effect.logWarning("cloudflare post-connect state reset failed", { + orgId: result.orgId, + error: summarizeCause(cause), + }), + ), + ), + ), + // The prime poll runs from the dashboard (`cloudflarePrime`), never here. + Effect.map((result): Completed => ({ + returnTo: result.returnTo, + message: CLOSE_MESSAGE, + })), + ) + case "planetscale": + return planetscaleOAuth.completeConnect(initiator, code, state).pipe( + // Single-org grants finish here (bind + provision the scrape target); multi-org grants + // leave the org picker to the dashboard. + Effect.flatMap((result) => { + const only = result.organizations.length === 1 ? result.organizations[0] : undefined + return only === undefined + ? Effect.succeed({ + returnTo: result.returnTo, + message: + "Authorization complete. Choose which PlanetScale organization to connect back in Maple.", + }) + : planetscaleConnection + .finalizeOrgSelection(result.orgId, { organization: only.name }) + .pipe( + Effect.as({ + returnTo: result.returnTo, + message: `Connected to ${only.name}. ${CLOSE_MESSAGE}`, + }), + ) + }), + ) + case "supabase": { + const issuer = params.get("iss") + if (issuer !== null && issuer.replace(/\/$/, "") !== supabaseIssuer) { + return Effect.succeed( + new OAuthCompleteResponse({ + status: "error", + message: + "The authorization came from an unexpected issuer. Restart the connect flow.", + returnTo: null, + }), + ) + } + // One grant is one Supabase organization: bind it and return. + return supabaseOAuth.completeConnect(initiator, code, state).pipe( + Effect.flatMap((result) => + supabaseConnection + .finalizeConnect(result.orgId, result.connectedByUserId, result.organization) + .pipe( + Effect.as({ + returnTo: result.returnTo, + message: `Connected to ${result.organization.name}. ${CLOSE_MESSAGE}`, + }), + ), + ), + ) + } + case "github": { + // `code` is present only with OAuth-on-install enabled; it proves the user owns the install. + const installationId = params.get("installation_id") ?? "" + return github + .completeConnect(initiator, installationId, state, code === "" ? undefined : code) + .pipe( + Effect.map((result): Completed => ({ + returnTo: result.returnTo, + message: CLOSE_MESSAGE, + })), + ) + } + } + } + + const completeOAuth = Effect.fn("IntegrationCompleter.oauth")(function* ( + initiator: OAuthStateInitiator, + provider: OAuthCallbackProvider, + query: string, + ) { + yield* Effect.annotateCurrentSpan({ "maple.integration.provider": provider }) + const label = LABELS[provider] + const params = new URLSearchParams(query) + const failed = (message: string) => + new OAuthCompleteResponse({ status: "error", message, returnTo: null }) + + const oauthError = params.get("error") + if (oauthError !== null) { + return failed(params.get("error_description") || `${label} returned an error`) + } + const state = params.get("state") + if (provider === "github") { + // `setup_action=request`: the org requires admin approval, so there is no installation yet. + if (params.get("installation_id") === null) { + return failed( + params.get("setup_action") === "request" + ? "Installation requested — an org admin must approve it on GitHub, then reconnect." + : "Missing installation_id in callback", + ) + } + if (state === null) { + return failed( + "Missing state in callback — GitHub did not return it. Restart the connection from the Maple dashboard.", + ) + } + } + const code = params.get("code") ?? "" + if (state === null || (provider !== "github" && code === "")) { + return failed("Missing code or state in callback") + } + + return yield* complete(initiator, provider, code, state, params).pipe( + Effect.map((result) => + result instanceof OAuthCompleteResponse + ? result + : new OAuthCompleteResponse({ + status: "success", + message: result.message, + returnTo: Option.getOrNull(decodeReturnPath(result.returnTo)), + }), + ), + // The response reduces failures to short copy; the real cause still goes to the log. + Effect.catch((error) => + Effect.logError(`${label} OAuth completeConnect failed`, { + tag: error._tag, + message: error.message, + }).pipe(Effect.as(failed(failureMessage(label, error)))), + ), + ) + }) + + const completeChat = Effect.fn("IntegrationCompleter.chat")(function* ( + initiator: OAuthStateInitiator, + connector: ChatConnectorId, + kind: ChatCallbackKind, + query: string, + ) { + yield* Effect.annotateCurrentSpan({ "maple.chat.connector": connector, "maple.chat.callback": kind }) + const params = new URLSearchParams(query) + const completed = + kind === "install" + ? chat + .completeInstall(initiator, connector, params) + .pipe(Effect.map((result): string | null => result.name)) + : chat + .completeLink(initiator, connector, params) + .pipe(Effect.map((result) => result.displayName ?? null)) + return yield* completed.pipe( + Effect.map((name) => new ChatCompleteResponse({ status: "success", reason: null, name })), + Effect.catch((error) => + Effect.logError( + kind === "install" ? "Chat install callback failed" : "Chat account link callback failed", + { connector, tag: error._tag, message: error.message }, + ).pipe( + Effect.as( + new ChatCompleteResponse({ + status: "error", + reason: CHAT_REASONS.get(error._tag) ?? "failed", + name: null, + }), + ), + ), + ), + ) + }) + + return { completeOAuth, completeChat } as const +}) diff --git a/apps/api/src/routes/internal/integrations.http.ts b/apps/api/src/routes/internal/integrations.http.ts index 038649076f..64100808a7 100644 --- a/apps/api/src/routes/internal/integrations.http.ts +++ b/apps/api/src/routes/internal/integrations.http.ts @@ -70,6 +70,7 @@ import { GithubConnectService } from "@maple/backend/services/integrations/vcs/v import { VcsCommitService } from "@maple/backend/services/integrations/vcs/VcsCommitService" import { VcsSourceService } from "@maple/backend/services/integrations/vcs/VcsSourceService" import { HazelOAuthService } from "@maple/backend/services/auth/HazelOAuthService" +import { makeIntegrationCompleter } from "./integrations-complete" import { requireAdmin as requireAdminRole } from "@maple/backend/services/auth/auth" import { CLOUDFLARE_CALLBACK_PATH, @@ -136,6 +137,7 @@ export const HttpIntegrationsLive = HttpApiBuilder.group(MapleInternalApi, "inte const gcp = yield* GcpConnectorService const edgeCache = yield* EdgeCacheService const env = yield* Env + const completer = yield* makeIntegrationCompleter return ( handlers @@ -732,6 +734,27 @@ export const HttpIntegrationsLive = HttpApiBuilder.group(MapleInternalApi, "inte }) }).pipe(Effect.withSpan("HttpIntegrations.vcsPullRequests")), ) + .handle("oauthComplete", ({ payload }) => + Effect.gen(function* () { + const tenant = yield* CurrentTenant.Context + return yield* completer.completeOAuth( + { orgId: tenant.orgId, userId: tenant.userId }, + payload.provider, + payload.query, + ) + }), + ) + .handle("chatComplete", ({ payload }) => + Effect.gen(function* () { + const tenant = yield* CurrentTenant.Context + return yield* completer.completeChat( + { orgId: tenant.orgId, userId: tenant.userId }, + payload.connector, + payload.kind, + payload.query, + ) + }), + ) ) }), ) diff --git a/apps/web/src/page/integrations/forwarded-callback.scene.test.ts b/apps/web/src/page/integrations/forwarded-callback.scene.test.ts new file mode 100644 index 0000000000..0a83f75f04 --- /dev/null +++ b/apps/web/src/page/integrations/forwarded-callback.scene.test.ts @@ -0,0 +1,94 @@ +import { OrgId } from "@maple/domain" +import { Schema } from "effect" +import { describe, expect, test } from "vitest" +import { PageOutMessage, type PageContext } from "../contract" +import { Message } from "./message" +import { init, update } from "./update" + +const orgId = Schema.decodeSync(OrgId)("org_acme") +const context: PageContext = { orgId, pinnedNamespace: null, timeZone: "UTC" } +const names = (commands: ReadonlyArray<{ readonly name: string }> | undefined) => + (commands ?? []).map(({ name }) => name) + +describe("a provider callback forwarded to the integrations page", () => { + test.each([ + [{ integration: "github", callback: "oauth" }, "CompleteIntegrationsOAuthCallback"], + [{ integration: "cloudflare", callback: "oauth" }, "CompleteIntegrationsOAuthCallback"], + [{ integration: "chat-slack", callback: "chat-install" }, "CompleteIntegrationsChatCallback"], + [{ integration: "chat-slack", callback: "chat-identity" }, "CompleteIntegrationsChatCallback"], + ])("is finished by the signed-in page: %j", (search, command) => { + expect(names(init({ search }, context).commands)).toContain(command) + }) + + test("a callback for no known provider finishes nothing and is dropped from the URL", () => { + const started = init({ search: { integration: "prometheus", callback: "oauth" } }, context) + expect(names(started.commands)).not.toContain("CompleteIntegrationsOAuthCallback") + expect(started.outMessage).toEqual( + PageOutMessage.RequestedNavigation({ + url: "/integrations?integration=prometheus", + isReplace: true, + }), + ) + }) + + test("a success the opener received leaves the closing popup alone", () => { + const model = init({ search: { integration: "github", callback: "oauth" } }, context).model + const done = update( + model, + Message.CompletedOAuthCallback({ + provider: "github", + status: "success", + message: "You can close this window and return to Maple.", + returnTo: "/integrations?integration=github", + isRelayed: true, + }), + context, + ) + expect(done.outMessage).toBeUndefined() + expect(names(done.commands)).toEqual([]) + }) + + test("a refusal with no opener shows its cause here, then strips the callback", () => { + const model = init({ search: { integration: "github", callback: "oauth" } }, context).model + const refusal = + "This authorization was started by a different Maple user or organization. Start it again from your own account." + const done = update( + model, + Message.CompletedOAuthCallback({ + provider: "github", + status: "error", + message: refusal, + returnTo: null, + isRelayed: false, + }), + context, + ) + expect(done.outMessage).toEqual( + PageOutMessage.RequestedToast({ type: "error", title: refusal, description: "" }), + ) + expect(names(done.commands)).toContain("StripIntegrationsCallback") + expect( + update(model, Message.StrippedCallback({ href: "/integrations?integration=github" }), context) + .outMessage, + ).toEqual( + PageOutMessage.RequestedNavigation({ url: "/integrations?integration=github", isReplace: true }), + ) + }) + + test("a chat outcome lands on the card's existing chat toast params", () => { + const model = init({ search: { integration: "chat-slack", callback: "chat-install" } }, context).model + const done = update( + model, + Message.CompletedChatCallback({ + search: { integration: "chat-slack", chat: "error", chat_reason: "state" }, + }), + context, + ) + expect(done.outMessage).toEqual( + PageOutMessage.RequestedNavigation({ + url: "/integrations?integration=chat-slack&chat=error&chat_reason=state", + isReplace: true, + }), + ) + }) +}) diff --git a/apps/web/src/page/integrations/forwarded-callback.ts b/apps/web/src/page/integrations/forwarded-callback.ts new file mode 100644 index 0000000000..512c9e1c72 --- /dev/null +++ b/apps/web/src/page/integrations/forwarded-callback.ts @@ -0,0 +1,130 @@ +import { ChatCompleteRequest, OAuthCompleteRequest } from "@maple/domain/http" +import { ChatConnectorId } from "@maple/domain/primitives" +import { Duration, Effect, Schema } from "effect" +import { Command } from "foldkit" +import { MapleInternalApiClient } from "../../platform/api-clients" +import { chatConnectorOf, integrationOfSearch } from "./catalog" +import { ConnectProvider, PROVIDERS } from "./connect" +import { Message } from "./message" +import type { Model } from "./model" + +// A provider callback the API forwarded to this page (`?callback=`). The public callback completes +// nothing, because the browser that lands on it proves nothing about who started the connect; +// this page finishes it as the signed-in user, and the API refuses a session that did not start it. + +export type ForwardedCallback = + | { readonly kind: "oauth"; readonly provider: ConnectProvider } + | { readonly kind: "chat-install" | "chat-identity"; readonly connector: ChatConnectorId } + +export const forwardedCallbackOf = (search: Model["search"]): ForwardedCallback | null => { + const integration = integrationOfSearch(search.integration) + if (search.callback === "oauth") { + return Schema.is(ConnectProvider)(integration) ? { kind: "oauth", provider: integration } : null + } + if (search.callback === "chat-install" || search.callback === "chat-identity") { + const connector = integration === null ? null : chatConnectorOf(integration) + return connector === null ? null : { kind: search.callback, connector } + } + return null +} + +/** + * The provider's query, read once and dropped from the address bar before anything else runs, so + * the authorization code is in no history entry, bookmark or later `Referer`. Not a Foldkit + * navigation: the route itself does not change. + */ +const takeFragment = Effect.sync(() => { + const query = window.location.hash.replace(/^#/, "") + window.history.replaceState( + window.history.state, + "", + `${window.location.pathname}${window.location.search}`, + ) + return query +}) + +/** The popup's opener is the page that started the connect; same origin only, like its listener. */ +const relayToOpener = (provider: ConnectProvider, status: "success" | "error", message: string) => + Effect.sync(() => { + const opener = window.opener + if (opener === null || opener === window) return false + opener.postMessage({ type: `maple:integration:${provider}`, status, message }, window.location.origin) + return true + }) + +const SUCCESS_CLOSE_DELAY = Duration.millis(600) + +export const CompleteOAuthCallback = Command.define("CompleteIntegrationsOAuthCallback", { + args: { provider: ConnectProvider }, + messages: [Message.CompletedOAuthCallback], + execute: ({ provider }) => + Effect.gen(function* () { + const query = yield* takeFragment + const client = yield* MapleInternalApiClient + const response = yield* client.integrations + .oauthComplete({ payload: new OAuthCompleteRequest({ provider, query }) }) + .pipe( + Effect.orElseSucceed(() => ({ + status: "error" as const, + message: `Failed to complete the ${PROVIDERS[provider].label} connection`, + returnTo: null, + })), + ) + const isRelayed = yield* relayToOpener(provider, response.status, response.message) + // Only a success closes itself: a failure's message is the one place its cause is spelled out. + if (isRelayed && response.status === "success") { + yield* Effect.sleep(SUCCESS_CLOSE_DELAY) + yield* Effect.sync(() => window.close()) + } + return Message.CompletedOAuthCallback({ + provider, + status: response.status, + message: response.message, + returnTo: response.returnTo, + isRelayed, + }) + }), +}) + +/** The outcome as the `/integrations` search the chat toast already reads (`chatCallbackOf`). */ +const chatOutcomeSearch = ( + connector: ChatConnectorId, + kind: "install" | "identity", + outcome: { + readonly status: "success" | "error" + readonly reason: string | null + readonly name: string | null + }, +): Record => { + const key = kind === "install" ? "chat" : "chat_identity" + const nameKey = kind === "install" ? "chat_workspace" : "chat_identity_name" + return { + integration: `chat-${connector}`, + [key]: outcome.status === "success" ? (kind === "install" ? "connected" : "linked") : "error", + ...(outcome.reason === null ? undefined : { chat_reason: outcome.reason }), + ...(outcome.name === null ? undefined : { [nameKey]: outcome.name }), + } +} + +export const CompleteChatCallback = Command.define("CompleteIntegrationsChatCallback", { + args: { connector: ChatConnectorId, kind: Schema.Literals(["install", "identity"]) }, + messages: [Message.CompletedChatCallback], + execute: ({ connector, kind }) => + Effect.gen(function* () { + const query = yield* takeFragment + const client = yield* MapleInternalApiClient + const outcome = yield* client.integrations + .chatComplete({ payload: new ChatCompleteRequest({ connector, kind, query }) }) + .pipe( + Effect.orElseSucceed(() => ({ status: "error" as const, reason: "failed", name: null })), + ) + return Message.CompletedChatCallback({ search: chatOutcomeSearch(connector, kind, outcome) }) + }), +}) + +/** One OutMessage per update: the toast first, then this strips the URL. */ +export const StripCallback = Command.define("StripIntegrationsCallback", { + args: { href: Schema.String }, + messages: [Message.StrippedCallback], + execute: ({ href }) => Effect.succeed(Message.StrippedCallback({ href })), +}) diff --git a/apps/web/src/page/integrations/message.ts b/apps/web/src/page/integrations/message.ts index 7424922c34..710a93ec28 100644 --- a/apps/web/src/page/integrations/message.ts +++ b/apps/web/src/page/integrations/message.ts @@ -83,6 +83,18 @@ export const Message = defineMessageUnion({ ClickedSettingsTab: { tab: Schema.String }, /** The chat callback toast went out; the return params are stripped next (a replace). */ CompletedChatToast: {}, + /** A forwarded provider callback finished (`oauthComplete`); `isRelayed`: the opener got it. */ + CompletedOAuthCallback: { + provider: ConnectProvider, + status: Schema.Literals(["success", "error"]), + message: Schema.String, + returnTo: Schema.NullOr(Schema.String), + isRelayed: Schema.Boolean, + }, + /** A forwarded chat install or account-link callback finished (`chatComplete`). */ + CompletedChatCallback: { search: Schema.Record(Schema.String, Schema.String) }, + /** The URL cleanup after a forwarded callback's toast. */ + StrippedCallback: { href: Schema.String }, /** The header or card Connect button: a new popup attempt for the page's provider. */ ClickedConnect: {}, SucceededStartConnect: { attempt: Schema.Number }, diff --git a/apps/web/src/page/integrations/update.ts b/apps/web/src/page/integrations/update.ts index b858645e2a..487e959161 100644 --- a/apps/web/src/page/integrations/update.ts +++ b/apps/web/src/page/integrations/update.ts @@ -8,6 +8,12 @@ import * as CopyButton from "../../ui/copy-button" import * as Dialog from "../../ui/dialog" import { chatConnectorOf, integrationOfSearch } from "./catalog" import { chatCallbackOf } from "./chat-callback" +import { + CompleteChatCallback, + CompleteOAuthCallback, + forwardedCallbackOf, + StripCallback, +} from "./forwarded-callback" import { initConnect } from "./connect" import { Disconnect, DISCONNECTS } from "./disconnect" import { initGithubCard } from "./github" @@ -69,6 +75,27 @@ const handleChatCallback = (model: Model, withToast: boolean): Return => { } } +/** A provider callback the API forwarded here (`?callback=`): finish it as the signed-in user. */ +const handleForwardedCallback = (model: Model): Return => { + if (model.search.callback === undefined) return { model } + const forwarded = forwardedCallbackOf(model.search) + if (forwarded === null) { + const { callback: _, ...rest } = model.search + return navigateTo(model, hrefOfSearch(rest), true) + } + return { + model, + commands: [ + forwarded.kind === "oauth" + ? CompleteOAuthCallback({ provider: forwarded.provider }) + : CompleteChatCallback({ + connector: forwarded.connector, + kind: forwarded.kind === "chat-install" ? "install" : "identity", + }), + ], + } +} + const navigateTo = (model: Model, url: string, isReplace: boolean): Return => ({ model, outMessage: PageOutMessage.RequestedNavigation({ url, isReplace }), @@ -110,6 +137,7 @@ export const init = (route: { readonly search: Search }, context: PageContext): (next: Model) => requestCustomer(next, context), (next: Model) => enterIntegration(next, context), (next: Model) => handleChatCallback(next, false), + (next: Model) => handleForwardedCallback(next), ]) const session = isClerkAuthEnabled ? [LoadSession()] : [] return { ...loaded, commands: [ReadClock(), ...session, ...(loaded.commands ?? [])] } @@ -217,6 +245,16 @@ export const update = (model: Model, message: Message, context: PageContext): Re const callback = chatCallbackOf(model.search) return callback === null ? { model } : navigateTo(model, hrefOfSearch(callback.next), true) }, + CompletedOAuthCallback: ({ provider, status, message: text, returnTo, isRelayed }) => { + // The popup closes itself once the page that opened it has the result. + if (isRelayed && status === "success") return { model } + const href = + status === "success" && returnTo !== null ? returnTo : hrefOfSearch({ integration: provider }) + const received = Connect.receivedCallback(model, context, provider, status, text) + return { ...received, commands: [...(received.commands ?? []), StripCallback({ href })] } + }, + CompletedChatCallback: ({ search }) => navigateTo(model, hrefOfSearch(search), true), + StrippedCallback: ({ href }) => navigateTo(model, href, true), ClickedConnect: () => Connect.clickedConnect(model), ClickedChatInstall: () => Chat.clickedInstall(model), CompletedLoadCloudflareUsage: ({ key, result }) => ({ diff --git a/apps/web/src/route/search.ts b/apps/web/src/route/search.ts index bc9c37cba5..9ae2ad3e18 100644 --- a/apps/web/src/route/search.ts +++ b/apps/web/src/route/search.ts @@ -252,6 +252,8 @@ export const IntegrationsSearch = Schema.Struct({ chat_workspace: Schema.optional(Schema.String), chat_identity: Schema.optional(Schema.String), chat_identity_name: Schema.optional(Schema.String), + /** A provider callback the API forwarded here to finish as the signed-in user (query in the fragment). */ + callback: Schema.optional(Schema.String), }) export const McpAuthorizeSearch = Schema.Struct({ diff --git a/packages/backend/src/services/auth/CloudflareOAuthService.test.ts b/packages/backend/src/services/auth/CloudflareOAuthService.test.ts index 0b5eeed3a5..8fc86d608f 100644 --- a/packages/backend/src/services/auth/CloudflareOAuthService.test.ts +++ b/packages/backend/src/services/auth/CloudflareOAuthService.test.ts @@ -100,6 +100,7 @@ const withOAuthApp = { const asOrgId = Schema.decodeUnknownSync(OrgId) const asUserId = Schema.decodeUnknownSync(UserId) +const INITIATOR = { orgId: asOrgId("org_a"), userId: asUserId("user_a") } describe("CloudflareOAuthService", () => { it.effect("reports not-connected for a fresh org", () => { @@ -184,7 +185,7 @@ describe("CloudflareOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: "https://api.example.com/api/integrations/cloudflare/callback", }) - const result = yield* service.completeConnect("auth-code", state) + const result = yield* service.completeConnect(INITIATOR, "auth-code", state) assert.strictEqual(result.orgId, "org_a") const status = yield* service.getStatus(asOrgId("org_a")) @@ -221,7 +222,7 @@ describe("CloudflareOAuthService", () => { const service = yield* CloudflareOAuthService // No state row was ever seeded — a forged/expired-and-purged callback. const error = yield* service - .completeConnect("auth-code", "state-that-was-never-issued") + .completeConnect(INITIATOR, "auth-code", "state-that-was-never-issued") .pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") if (error._tag === "@maple/http/errors/IntegrationsValidationError") { @@ -244,7 +245,7 @@ describe("CloudflareOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: "https://api.example.com/api/integrations/cloudflare/callback", }) - const result = yield* service.completeConnect("auth-code", state) + const result = yield* service.completeConnect(INITIATOR, "auth-code", state) assert.strictEqual(result.orgId, "org_a") // Both accounts surface, in the grant's order. @@ -321,7 +322,7 @@ describe("CloudflareOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: "https://api.example.com/api/integrations/cloudflare/callback", }) - const error = yield* service.completeConnect("auth-code", state).pipe(Effect.flip) + const error = yield* service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") if (error._tag === "@maple/http/errors/IntegrationsValidationError") { assert.include(error.message, "refresh token") @@ -357,7 +358,7 @@ describe("CloudflareOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: "https://api.example.com/api/integrations/cloudflare/callback", }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) const result = yield* service.disconnect(asOrgId("org_a")) assert.strictEqual(result.disconnected, true) @@ -411,7 +412,7 @@ describe("CloudflareOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: "https://api.example.com/api/integrations/cloudflare/callback", }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) const [connected] = yield* owners() assert.deepStrictEqual(connected, { source: connected!.source, @@ -488,7 +489,7 @@ describe("CloudflareOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: "https://api.example.com/api/integrations/cloudflare/callback", }) - return yield* service.completeConnect("auth-code", state) + return yield* service.completeConnect(INITIATOR, "auth-code", state) }) return Effect.gen(function* () { diff --git a/packages/backend/src/services/auth/CloudflareOAuthService.ts b/packages/backend/src/services/auth/CloudflareOAuthService.ts index 3e992706c6..5cf872bde9 100644 --- a/packages/backend/src/services/auth/CloudflareOAuthService.ts +++ b/packages/backend/src/services/auth/CloudflareOAuthService.ts @@ -22,6 +22,7 @@ import { makeOAuthConnectionHelpers, OAUTH_STATE_TTL_MS, } from "./oauth/connection-helpers" +import type { OAuthStateInitiator } from "./oauth/state-initiator" const CLOUDFLARE_PROVIDER = "cloudflare" @@ -144,6 +145,8 @@ export interface CloudflareOAuthServiceApi { IntegrationsValidationError | IntegrationsPersistenceError > readonly completeConnect: ( + /** The signed-in user finishing the flow; must be the one that started it. */ + initiator: OAuthStateInitiator, code: string, state: string, ) => Effect.Effect< @@ -268,11 +271,12 @@ export class CloudflareOAuthService extends Context.Service< }) const completeConnect = Effect.fn("CloudflareOAuthService.completeConnect")(function* ( + initiator: OAuthStateInitiator, code: string, state: string, ) { const config = yield* resolveConfig(env) - const stateRow = yield* oauth.requireStateRow(state) + const stateRow = yield* oauth.requireStateRow(state, initiator) yield* oauth.deleteAuthState(state) if (!stateRow.codeVerifier) { diff --git a/packages/backend/src/services/auth/HazelOAuthService.ts b/packages/backend/src/services/auth/HazelOAuthService.ts index 3eb77466ec..cbd6ab65be 100644 --- a/packages/backend/src/services/auth/HazelOAuthService.ts +++ b/packages/backend/src/services/auth/HazelOAuthService.ts @@ -17,6 +17,7 @@ import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/http" import { Env, type EnvConfig } from "@maple/backend/platform/Env" import { Database } from "@maple/backend/platform/DatabaseLive" import { makeOAuthConnectionHelpers, OAUTH_STATE_TTL_MS, toUpstreamError } from "./oauth/connection-helpers" +import type { OAuthStateInitiator } from "./oauth/state-initiator" const HAZEL_PROVIDER = "hazel" @@ -132,6 +133,8 @@ export interface HazelOAuthServiceApi { IntegrationsValidationError | IntegrationsUpstreamError | IntegrationsPersistenceError > readonly completeConnect: ( + /** The signed-in user finishing the flow; must be the one that started it. */ + initiator: OAuthStateInitiator, code: string, state: string, ) => Effect.Effect< @@ -359,11 +362,12 @@ export class HazelOAuthService extends Context.Service { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - const result = yield* service.completeConnect("auth-code", state) + const result = yield* service.completeConnect(INITIATOR, "auth-code", state) assert.strictEqual(result.orgId, "org_a") assert.deepStrictEqual( result.organizations.map((org) => org.name), @@ -278,7 +279,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) const row = yield* Effect.promise(() => queryFirstRow<{ external_user_id: string }>( testDb, @@ -299,7 +300,7 @@ describe("PlanetScaleOAuthService", () => { return Effect.gen(function* () { const service = yield* PlanetScaleOAuthService const error = yield* service - .completeConnect("auth-code", "state-that-was-never-issued") + .completeConnect(INITIATOR, "auth-code", "state-that-was-never-issued") .pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") assert.isFalse(yield* service.hasConnection(asOrgId("org_a"))) @@ -315,7 +316,7 @@ describe("PlanetScaleOAuthService", () => { }) yield* TestClock.adjust("11 minutes") - const error = yield* service.completeConnect("auth-code", state).pipe(Effect.flip) + const error = yield* service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") assert.include(error.message, "expired") assert.isFalse(yield* service.hasConnection(asOrgId("org_a"))) @@ -329,7 +330,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - const error = yield* service.completeConnect("auth-code", state).pipe(Effect.flip) + const error = yield* service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") if (error._tag === "@maple/http/errors/IntegrationsValidationError") { assert.include(error.message, "refresh token") @@ -351,7 +352,7 @@ describe("PlanetScaleOAuthService", () => { }) // The first listing 401s (auth→API propagation lag); the retry after the // 2s backoff succeeds and the connect completes normally. - const fiber = yield* Effect.forkChild(service.completeConnect("auth-code", state), { + const fiber = yield* Effect.forkChild(service.completeConnect(INITIATOR, "auth-code", state), { startImmediately: true, }) yield* TestClock.adjust("2 seconds") @@ -376,7 +377,7 @@ describe("PlanetScaleOAuthService", () => { callbackUrl: CALLBACK_URL, }) const fiber = yield* Effect.forkChild( - service.completeConnect("auth-code", state).pipe(Effect.flip), + service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip), { startImmediately: true }, ) yield* TestClock.adjust("2 seconds") @@ -405,7 +406,7 @@ describe("PlanetScaleOAuthService", () => { // The v1 API keeps rejecting a token the auth server introspects as // active — "reconnect" would be a lie, so this must NOT be RevokedError. const fiber = yield* Effect.forkChild( - service.completeConnect("auth-code", state).pipe(Effect.flip), + service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip), { startImmediately: true }, ) yield* TestClock.adjust("2 seconds") @@ -434,7 +435,7 @@ describe("PlanetScaleOAuthService", () => { callbackUrl: CALLBACK_URL, }) const fiber = yield* Effect.forkChild( - service.completeConnect("auth-code", state).pipe(Effect.flip), + service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip), { startImmediately: true }, ) yield* TestClock.adjust("2 seconds") @@ -458,7 +459,7 @@ describe("PlanetScaleOAuthService", () => { callbackUrl: CALLBACK_URL, }) const fiber = yield* Effect.forkChild( - service.completeConnect("auth-code", state).pipe(Effect.flip), + service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip), { startImmediately: true }, ) yield* TestClock.adjust("2 seconds") @@ -482,7 +483,7 @@ describe("PlanetScaleOAuthService", () => { callbackUrl: CALLBACK_URL, }) const fiber = yield* Effect.forkChild( - service.completeConnect("auth-code", state).pipe(Effect.flip), + service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip), { startImmediately: true }, ) yield* TestClock.adjust("2 seconds") @@ -505,7 +506,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - const error = yield* service.completeConnect("auth-code", state).pipe(Effect.flip) + const error = yield* service.completeConnect(INITIATOR, "auth-code", state).pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") if (error._tag === "@maple/http/errors/IntegrationsValidationError") { assert.include(error.message, "no organizations") @@ -529,7 +530,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) const listed = yield* service.listOrganizations(asOrgId("org_a")) assert.deepStrictEqual( listed.map((org) => org.name), @@ -547,7 +548,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) // The grant dies upstream: the org listing starts answering 401. That // must surface as revoked, not a generic upstream failure — the picker @@ -582,7 +583,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) const first = yield* service.getValidAccessToken(asOrgId("org_a")) @@ -603,7 +604,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) // Populate the memo first — that is the state the invalidation has to clear. yield* service.getValidAccessToken(asOrgId("org_a")) @@ -621,7 +622,7 @@ describe("PlanetScaleOAuthService", () => { const { state } = yield* service.startConnect(asOrgId("org_a"), asUserId("user_a"), { callbackUrl: CALLBACK_URL, }) - yield* service.completeConnect("auth-code", state) + yield* service.completeConnect(INITIATOR, "auth-code", state) const result = yield* service.disconnect(asOrgId("org_a")) assert.strictEqual(result.disconnected, true) diff --git a/packages/backend/src/services/auth/PlanetScaleOAuthService.ts b/packages/backend/src/services/auth/PlanetScaleOAuthService.ts index 78608f7e98..2e8972cad0 100644 --- a/packages/backend/src/services/auth/PlanetScaleOAuthService.ts +++ b/packages/backend/src/services/auth/PlanetScaleOAuthService.ts @@ -24,6 +24,7 @@ import { runPlanetScale, } from "@maple/backend/services/integrations/planetscale/api" import { makeOAuthConnectionHelpers, OAUTH_STATE_TTL_MS, toUpstreamError } from "./oauth/connection-helpers" +import type { OAuthStateInitiator } from "./oauth/state-initiator" const PLANETSCALE_PROVIDER = "planetscale" @@ -142,6 +143,8 @@ export interface PlanetScaleOAuthServiceApi { * second API round-trip. */ readonly completeConnect: ( + /** The signed-in user finishing the flow; must be the one that started it. */ + initiator: OAuthStateInitiator, code: string, state: string, ) => Effect.Effect< @@ -359,11 +362,12 @@ export class PlanetScaleOAuthService extends Context.Service< }) const completeConnect = Effect.fn("PlanetScaleOAuthService.completeConnect")(function* ( + initiator: OAuthStateInitiator, code: string, state: string, ) { const config = yield* resolveConfig(env) - const stateRow = yield* oauth.requireStateRow(state) + const stateRow = yield* oauth.requireStateRow(state, initiator) yield* oauth.deleteAuthState(state) const tokenResponse = yield* oauth.exchangeAuthorizationCode(config, code, stateRow.redirectUri) diff --git a/packages/backend/src/services/auth/SupabaseOAuthService.test.ts b/packages/backend/src/services/auth/SupabaseOAuthService.test.ts index b330342e9e..25672886ec 100644 --- a/packages/backend/src/services/auth/SupabaseOAuthService.test.ts +++ b/packages/backend/src/services/auth/SupabaseOAuthService.test.ts @@ -61,6 +61,7 @@ const listWithGrant = Effect.gen(function* () { const asOrgId = Schema.decodeUnknownSync(OrgId) const asUserId = Schema.decodeUnknownSync(UserId) const ORG = asOrgId("org_a") +const INITIATOR = { orgId: ORG, userId: asUserId("user_a") } const CALLBACK_URL = "https://api.example.com/api/integrations/supabase/callback" /** Start the flow and hand the fake the PKCE challenge the authorize URL carried, as Supabase would. */ @@ -72,7 +73,7 @@ const connect = (fake: ReturnType) => returnTo: "/integrations/supabase", }) fake.state.codeChallenge = new URL(redirectUrl).searchParams.get("code_challenge") - return yield* service.completeConnect("4a0c7b4e-code", state) + return yield* service.completeConnect(INITIATOR, "4a0c7b4e-code", state) }) describe("SupabaseOAuthService", () => { diff --git a/packages/backend/src/services/auth/SupabaseOAuthService.ts b/packages/backend/src/services/auth/SupabaseOAuthService.ts index f53ced3aaf..106ac784be 100644 --- a/packages/backend/src/services/auth/SupabaseOAuthService.ts +++ b/packages/backend/src/services/auth/SupabaseOAuthService.ts @@ -29,6 +29,7 @@ import { OAUTH_STATE_TTL_MS, toUpstreamError, } from "./oauth/connection-helpers" +import type { OAuthStateInitiator } from "./oauth/state-initiator" const SUPABASE_PROVIDER = "supabase" @@ -92,6 +93,8 @@ export interface SupabaseOAuthServiceApi { * Supabase organization; it is returned so the caller can bind it. */ readonly completeConnect: ( + /** The signed-in user finishing the flow; must be the one that started it. */ + initiator: OAuthStateInitiator, code: string, state: string, ) => Effect.Effect< @@ -186,11 +189,12 @@ export class SupabaseOAuthService extends Context.Service { const fetch = fetchLayer(() => jsonResponse({ error: "unexpected" }, 500)) return Effect.gen(function* () { const helpers = yield* makeHelpers() - const error = yield* helpers.requireStateRow("state-that-was-never-issued").pipe(Effect.flip) + const error = yield* helpers + .requireStateRow("state-that-was-never-issued", INITIATOR) + .pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") }).pipe(Effect.provide(provideBase(testDb, fetch))) }) @@ -407,7 +410,9 @@ describe("requireStateRow provider scoping", () => { }), ), ) - const error = yield* helpers.requireStateRow("state-of-another-provider").pipe(Effect.flip) + const error = yield* helpers + .requireStateRow("state-of-another-provider", INITIATOR) + .pipe(Effect.flip) assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") }).pipe( Effect.provide( @@ -419,3 +424,64 @@ describe("requireStateRow provider scoping", () => { ) }) }) + +describe("requireStateRow initiator binding", () => { + const insertState = (helpers: Effect.Success>, state: string) => + helpers.dbExecute((db) => + db.run( + PG.insertInto(OAuthAuthStates).values({ + state, + orgId: ORG, + provider: "test-provider", + initiatedByUserId: USER, + redirectUri: "https://api.example.com/callback", + returnTo: null, + codeVerifier: null, + createdAt: Date.now(), + expiresAt: Date.now() + 60_000, + }), + ), + ) + + it.live.each([ + ["another user in the same org", { orgId: ORG, userId: asUserId("user_somebody_else") }], + ["the same user in another org", { orgId: asOrgId("org_somebody_else"), userId: USER }], + ])("refuses a forwarded authorization finished by %s, and burns the state", ([, finisher]) => { + const testDb = createTestDb(trackedDbs) + return Effect.gen(function* () { + const helpers = yield* makeHelpers() + yield* insertState(helpers, "forwarded-state") + const error = yield* helpers.requireStateRow("forwarded-state", finisher).pipe(Effect.flip) + assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") + assert.include(error.message, "different Maple user or organization") + // The initiator cannot redeem it afterwards either: the forwarded state is spent. + const replay = yield* helpers.requireStateRow("forwarded-state", INITIATOR).pipe(Effect.flip) + assert.include(replay.message, "not recognized") + }).pipe( + Effect.provide( + provideBase( + testDb, + fetchLayer(() => freshTokens()), + ), + ), + ) + }) + + it.live("returns the row to the user and org that started the flow", () => { + const testDb = createTestDb(trackedDbs) + return Effect.gen(function* () { + const helpers = yield* makeHelpers() + yield* insertState(helpers, "own-state") + const row = yield* helpers.requireStateRow("own-state", INITIATOR) + assert.strictEqual(row.orgId, ORG) + assert.strictEqual(row.initiatedByUserId, USER) + }).pipe( + Effect.provide( + provideBase( + testDb, + fetchLayer(() => freshTokens()), + ), + ), + ) + }) +}) diff --git a/packages/backend/src/services/auth/oauth/connection-helpers.ts b/packages/backend/src/services/auth/oauth/connection-helpers.ts index ce9af464a8..85da6fb80c 100644 --- a/packages/backend/src/services/auth/oauth/connection-helpers.ts +++ b/packages/backend/src/services/auth/oauth/connection-helpers.ts @@ -35,6 +35,7 @@ import { import type { DatabaseApi } from "@maple/backend/platform/DatabaseLive" import { deleteSourcesOf, createIntegrationSource } from "@maple/backend/services/org/integration-sources" import { makeDbExecute, makePersistenceErrorMapper } from "@maple/backend/platform/db-execute" +import { isStateInitiator, stateInitiatorMismatch, type OAuthStateInitiator } from "./state-initiator" import type { EnvConfig } from "@maple/backend/platform/Env" export const OAUTH_STATE_TTL_MS = 10 * 60_000 // 10 minutes @@ -205,8 +206,10 @@ export const makeOAuthConnectionHelpers = (options: MakeOAuthConnectionHelpersOp const deleteAuthState = (state: string) => dbExecute((db) => db.run(PG.deleteFrom(OAuthAuthStates).where(($) => [$.state.eq(state)]))) + /** The live state row, redeemable only by the Maple user and org that started the flow. */ const requireStateRow = Effect.fn("OAuthConnectionHelpers.requireStateRow")(function* ( state: string, + initiator: OAuthStateInitiator, ) { const rows = yield* dbExecute((db) => db.run( @@ -232,6 +235,10 @@ export const makeOAuthConnectionHelpers = (options: MakeOAuthConnectionHelpersOp }), ) } + if (!isStateInitiator(row, initiator)) { + yield* deleteAuthState(state) + return yield* Effect.fail(stateInitiatorMismatch()) + } return row satisfies OAuthAuthStateRow }) diff --git a/packages/backend/src/services/auth/oauth/state-initiator.ts b/packages/backend/src/services/auth/oauth/state-initiator.ts new file mode 100644 index 0000000000..75329a420f --- /dev/null +++ b/packages/backend/src/services/auth/oauth/state-initiator.ts @@ -0,0 +1,23 @@ +import { IntegrationsValidationError } from "@maple/domain/http" +import type { OrgId, UserId } from "@maple/domain/primitives" + +/** + * The signed-in Maple user finishing a connect, install or account link. A state row is only + * redeemed by the user and org that created it: the authorize URL is transferable, so the state + * alone must never decide whose tenant receives the provider's grant. + */ +export interface OAuthStateInitiator { + readonly orgId: OrgId + readonly userId: UserId +} + +export const isStateInitiator = ( + row: { readonly orgId: string; readonly initiatedByUserId: string }, + initiator: OAuthStateInitiator, +): boolean => row.orgId === initiator.orgId && row.initiatedByUserId === initiator.userId + +export const stateInitiatorMismatch = () => + new IntegrationsValidationError({ + message: + "This authorization was started by a different Maple user or organization. Start it again from your own account.", + }) diff --git a/packages/backend/src/services/integrations/ChatWorkspaceService.test.ts b/packages/backend/src/services/integrations/ChatWorkspaceService.test.ts index 0e4ddd1fff..20d2a02cbc 100644 --- a/packages/backend/src/services/integrations/ChatWorkspaceService.test.ts +++ b/packages/backend/src/services/integrations/ChatWorkspaceService.test.ts @@ -37,6 +37,7 @@ import { ChatConnectorRegistry, ChatWorkspaceService } from "./ChatWorkspaceServ const ORG = Schema.decodeSync(OrgId)("org_chat_1") const OTHER_ORG = Schema.decodeSync(OrgId)("org_chat_2") const USER = Schema.decodeSync(UserId)("user_chat_1") +const INITIATOR = { orgId: ORG, userId: USER } const OTHER_USER = Schema.decodeSync(UserId)("user_chat_2") const TEST_CONNECTOR = Schema.decodeSync(ChatConnectorId)("testchat") const IDENTITY_CONNECTOR = Schema.decodeSync(ChatConnectorId)("testchatid") @@ -436,6 +437,7 @@ describe("ChatWorkspaceService", () => { const { url } = yield* chat.beginInstall(ORG, USER, TEST_CONNECTOR, CALLBACK) const state = stateFrom(url) const linked = yield* chat.completeInstall( + INITIATOR, TEST_CONNECTOR, callback(state, "workspace-7", "Acme"), ) @@ -448,32 +450,67 @@ describe("ChatWorkspaceService", () => { ) // Single-use: the same callback replayed is not a second link. const replay = yield* chat - .completeInstall(TEST_CONNECTOR, callback(state, "workspace-7", "Acme")) + .completeInstall(INITIATOR, TEST_CONNECTOR, callback(state, "workspace-7", "Acme")) .pipe(Effect.flip) assert.strictEqual(replay._tag, "@maple/http/errors/IntegrationsValidationError") }).pipe(Effect.provide(makeLayer(testDb))) }), ) + it.effect("refuses an install URL finished by another Maple user, and links nothing", () => + Effect.gen(function* () { + const testDb = createTestDb(trackedDbs) + yield* Effect.gen(function* () { + const chat = yield* ChatWorkspaceService + const { url } = yield* chat.beginInstall(ORG, USER, TEST_CONNECTOR, CALLBACK) + const state = stateFrom(url) + const failure = yield* chat + .completeInstall( + { orgId: OTHER_ORG, userId: OTHER_USER }, + TEST_CONNECTOR, + callback(state, "workspace-forwarded", "Victim"), + ) + .pipe(Effect.flip) + assert.strictEqual(failure._tag, "@maple/http/errors/IntegrationsValidationError") + assert.lengthOf((yield* chat.list(ORG))[0]?.workspaces ?? [], 0) + assert.lengthOf((yield* chat.list(OTHER_ORG))[0]?.workspaces ?? [], 0) + // Spent: the initiator cannot redeem the forwarded callback afterwards. + const replay = yield* chat + .completeInstall( + INITIATOR, + TEST_CONNECTOR, + callback(state, "workspace-forwarded", "Victim"), + ) + .pipe(Effect.flip) + assert.strictEqual(replay._tag, "@maple/http/errors/IntegrationsValidationError") + assert.lengthOf((yield* chat.list(ORG))[0]?.workspaces ?? [], 0) + }).pipe(Effect.provide(makeLayer(testDb))) + }), + ) + it.effect("refuses a callback with no state, an unknown state, and an expired one", () => Effect.gen(function* () { const testDb = createTestDb(trackedDbs) yield* Effect.gen(function* () { const chat = yield* ChatWorkspaceService const missing = yield* chat - .completeInstall(TEST_CONNECTOR, new URLSearchParams({ workspace: "workspace-8" })) + .completeInstall( + INITIATOR, + TEST_CONNECTOR, + new URLSearchParams({ workspace: "workspace-8" }), + ) .pipe(Effect.flip) assert.strictEqual(missing._tag, "@maple/http/errors/IntegrationsValidationError") const unknown = yield* chat - .completeInstall(TEST_CONNECTOR, callback("never-issued", "workspace-8")) + .completeInstall(INITIATOR, TEST_CONNECTOR, callback("never-issued", "workspace-8")) .pipe(Effect.flip) assert.strictEqual(unknown._tag, "@maple/http/errors/IntegrationsValidationError") const { url } = yield* chat.beginInstall(ORG, USER, TEST_CONNECTOR, CALLBACK) yield* TestClock.adjust(STATE_TTL_MS + 1) const expired = yield* chat - .completeInstall(TEST_CONNECTOR, callback(stateFrom(url), "workspace-8")) + .completeInstall(INITIATOR, TEST_CONNECTOR, callback(stateFrom(url), "workspace-8")) .pipe(Effect.flip) assert.strictEqual(expired._tag, "@maple/http/errors/IntegrationsValidationError") const statuses = yield* chat.list(ORG) @@ -490,7 +527,7 @@ describe("ChatWorkspaceService", () => { const chat = yield* ChatWorkspaceService const { url } = yield* chat.beginInstall(ORG, USER, TEST_CONNECTOR, CALLBACK) const failure = yield* chat - .completeInstall(TEST_CONNECTOR, callback(stateFrom(url), "workspace-9")) + .completeInstall(INITIATOR, TEST_CONNECTOR, callback(stateFrom(url), "workspace-9")) .pipe(Effect.flip) assert.strictEqual(failure._tag, "@maple/http/errors/IntegrationsForbiddenError") // The other org keeps it, and the bot still resolves it to them. @@ -514,6 +551,7 @@ describe("ChatWorkspaceService", () => { yield* chat.updateSettings(ORG, id, { approver: "moderators" }) const { url } = yield* chat.beginInstall(ORG, USER, TEST_CONNECTOR, CALLBACK) yield* chat.completeInstall( + INITIATOR, TEST_CONNECTOR, callback(stateFrom(url), "workspace-10", "Renamed"), ) @@ -599,7 +637,7 @@ describe("ChatWorkspaceService", () => { const { url } = yield* chat.beginInstall(ORG, USER, TEST_CONNECTOR, CALLBACK) const params = callback(stateFrom(url), "workspace-cred", "Acme") params.set("credentials", '{"token":"a-workspaces-own-token"}') - yield* chat.completeInstall(TEST_CONNECTOR, params) + yield* chat.completeInstall(INITIATOR, TEST_CONNECTOR, params) const resolved = yield* chat.resolve(TEST_CONNECTOR, "workspace-cred") assert.strictEqual( @@ -625,7 +663,11 @@ describe("ChatWorkspaceService", () => { yield* Effect.gen(function* () { const chat = yield* ChatWorkspaceService const { url } = yield* chat.beginInstall(ORG, USER, TEST_CONNECTOR, CALLBACK) - yield* chat.completeInstall(TEST_CONNECTOR, callback(stateFrom(url), "workspace-plain")) + yield* chat.completeInstall( + INITIATOR, + TEST_CONNECTOR, + callback(stateFrom(url), "workspace-plain"), + ) const resolved = yield* chat.resolve(TEST_CONNECTOR, "workspace-plain") assert.isTrue(Option.isSome(resolved)) assert.isUndefined(Option.getOrUndefined(resolved)?.credentials) @@ -666,6 +708,26 @@ describe("ChatWorkspaceService", () => { }), ) + it.effect("refuses a link URL finished by another Maple user, and links no account", () => + Effect.gen(function* () { + const testDb = createTestDb(trackedDbs) + yield* Effect.gen(function* () { + const chat = yield* ChatWorkspaceService + const { url } = yield* chat.beginLink(ORG, USER, IDENTITY_CONNECTOR, IDENTITY_CALLBACK) + const failure = yield* chat + .completeLink( + { orgId: ORG, userId: OTHER_USER }, + IDENTITY_CONNECTOR, + identityCallback(stateFrom(url), "account-victim"), + ) + .pipe(Effect.flip) + assert.strictEqual(failure._tag, "@maple/http/errors/IntegrationsValidationError") + assert.isUndefined((yield* chat.list(ORG, USER))[0]?.identity) + assert.isUndefined((yield* chat.list(ORG, OTHER_USER))[0]?.identity) + }).pipe(Effect.provide(makeLayer(testDb, { registry: [identityConnector] }))) + }), + ) + it.effect("binds the chat account to the user who STARTED the link, not the callback", () => Effect.gen(function* () { const testDb = createTestDb(trackedDbs) @@ -673,6 +735,7 @@ describe("ChatWorkspaceService", () => { const chat = yield* ChatWorkspaceService const { url } = yield* chat.beginLink(ORG, USER, IDENTITY_CONNECTOR, IDENTITY_CALLBACK) const linked = yield* chat.completeLink( + INITIATOR, IDENTITY_CONNECTOR, // A rival Maple user named in the callback, which is exactly the thing // that must not be read: the state row is the only authority here. @@ -706,13 +769,21 @@ describe("ChatWorkspaceService", () => { const chat = yield* ChatWorkspaceService const install = yield* chat.beginInstall(ORG, USER, IDENTITY_CONNECTOR, CALLBACK) const asLink = yield* chat - .completeLink(IDENTITY_CONNECTOR, identityCallback(stateFrom(install.url), "account-2")) + .completeLink( + INITIATOR, + IDENTITY_CONNECTOR, + identityCallback(stateFrom(install.url), "account-2"), + ) .pipe(Effect.flip) assert.strictEqual(asLink._tag, "@maple/http/errors/IntegrationsValidationError") const link = yield* chat.beginLink(ORG, USER, IDENTITY_CONNECTOR, IDENTITY_CALLBACK) const asInstall = yield* chat - .completeInstall(IDENTITY_CONNECTOR, callback(stateFrom(link.url), "workspace-11")) + .completeInstall( + INITIATOR, + IDENTITY_CONNECTOR, + callback(stateFrom(link.url), "workspace-11"), + ) .pipe(Effect.flip) assert.strictEqual(asInstall._tag, "@maple/http/errors/IntegrationsValidationError") @@ -731,7 +802,11 @@ describe("ChatWorkspaceService", () => { const { url } = yield* chat.beginLink(ORG, USER, IDENTITY_CONNECTOR, IDENTITY_CALLBACK) yield* TestClock.adjust(STATE_TTL_MS + 1) const expired = yield* chat - .completeLink(IDENTITY_CONNECTOR, identityCallback(stateFrom(url), "account-3")) + .completeLink( + INITIATOR, + IDENTITY_CONNECTOR, + identityCallback(stateFrom(url), "account-3"), + ) .pipe(Effect.flip) assert.strictEqual(expired._tag, "@maple/http/errors/IntegrationsValidationError") const statuses = yield* chat.list(ORG, USER) diff --git a/packages/backend/src/services/integrations/ChatWorkspaceService.ts b/packages/backend/src/services/integrations/ChatWorkspaceService.ts index 382d0cc6c2..4520e2658e 100644 --- a/packages/backend/src/services/integrations/ChatWorkspaceService.ts +++ b/packages/backend/src/services/integrations/ChatWorkspaceService.ts @@ -30,6 +30,11 @@ import { makeDbExecute } from "@maple/backend/platform/db-execute" import { Env } from "@maple/backend/platform/Env" import { sealChatWorkspaceCredentials } from "@maple/backend/services/integrations/chat-workspace-credentials" import { OAuthStateRepository } from "@maple/backend/services/auth/OAuthStateRepository" +import { + isStateInitiator, + stateInitiatorMismatch, + type OAuthStateInitiator, +} from "@maple/backend/services/auth/oauth/state-initiator" import { linkChatIdentity, listChatIdentities, @@ -135,6 +140,8 @@ export interface ChatWorkspaceServiceApi { IntegrationsNotFoundError | IntegrationsConfigurationError | IntegrationsPersistenceError > readonly completeInstall: ( + /** The signed-in user finishing the flow; must be the one that started it. */ + initiator: OAuthStateInitiator, connectorId: ChatConnectorId, params: URLSearchParams, ) => Effect.Effect< @@ -157,6 +164,8 @@ export interface ChatWorkspaceServiceApi { IntegrationsNotFoundError | IntegrationsConfigurationError | IntegrationsPersistenceError > readonly completeLink: ( + /** The signed-in user finishing the flow; must be the one that started it. */ + initiator: OAuthStateInitiator, connectorId: ChatConnectorId, params: URLSearchParams, ) => Effect.Effect< @@ -349,17 +358,14 @@ const make: Effect.Effect< return { url } }) - // The same known gap the other OAuth install service in this directory - // documents: `state` is unguessable, single-use and TTL-bounded, but it is not - // bound to the browser that started the install, so an attacker who gets a - // chat-workspace manager to complete THEIR authorize URL links that manager's - // workspace to the attacker's org. Closing it is the same architecture call, - // unmade for the same reason — that service's `completeInstall` lists the three - // options. What is closed here: the workspace identity comes from the - // connector, which must read it from whatever the platform bound to the - // callback credential, and a workspace already linked to another org is + // `state` is unguessable, single-use and TTL-bounded, and it is redeemed only by the Maple + // user and org that started the install: the authorize URL is transferable, so a manager who + // completes somebody else's URL is refused rather than linking their workspace to that org. + // The workspace identity comes from the connector, which must read it from whatever the + // platform bound to the callback credential, and a workspace already linked to another org is // rejected below rather than moved. const completeInstall = Effect.fn("ChatWorkspaceService.completeInstall")(function* ( + initiator: OAuthStateInitiator, connectorId: ChatConnectorId, params: URLSearchParams, ) { @@ -390,6 +396,7 @@ const make: Effect.Effect< }), ) } + if (!isStateInitiator(row, initiator)) return yield* Effect.fail(stateInitiatorMismatch()) const orgId = yield* decodeOrgId(row.orgId).pipe( Effect.mapError( (error) => @@ -520,14 +527,13 @@ const make: Effect.Effect< /** * Bind the chat account that just authorized to the Maple user who started the link. * - * That user comes from the state row's `initiatedByUserId`, never from the session on the - * callback request. The callback is a top-level redirect the chat platform issues, so the - * browser that opens it need not be the browser that began the link — binding on a cookie - * would let somebody finish a link in a different session, onto whichever Maple user happened - * to be signed in there. The state row is the only thing that knows both halves, and it is - * unguessable, single-use and TTL-bounded. + * The platform's callback is public, so it only hands the query to the dashboard; the signed-in + * dashboard finishes here, and the state row is redeemed only when that session is the user + * and org the row names. A link URL forwarded to somebody else therefore cannot attach their + * chat account to the sender's Maple user. */ const completeLink = Effect.fn("ChatWorkspaceService.completeLink")(function* ( + initiator: OAuthStateInitiator, connectorId: ChatConnectorId, params: URLSearchParams, ) { @@ -558,6 +564,7 @@ const make: Effect.Effect< new IntegrationsValidationError({ message: "Link state expired — start the link again" }), ) } + if (!isStateInitiator(row, initiator)) return yield* Effect.fail(stateInitiatorMismatch()) const orgId = yield* decodeOrgId(row.orgId).pipe( Effect.mapError( (error) => diff --git a/packages/backend/src/services/integrations/PlanetScaleConnectionService.test.ts b/packages/backend/src/services/integrations/PlanetScaleConnectionService.test.ts index 881cb49134..91127a4f04 100644 --- a/packages/backend/src/services/integrations/PlanetScaleConnectionService.test.ts +++ b/packages/backend/src/services/integrations/PlanetScaleConnectionService.test.ts @@ -165,7 +165,7 @@ const storeGrant = (orgId: OrgId) => const { state } = yield* oauth.startConnect(orgId, asUserId("user_1"), { callbackUrl: CALLBACK_URL, }) - return yield* oauth.completeConnect("auth-code", state) + return yield* oauth.completeConnect({ orgId, userId: asUserId("user_1") }, "auth-code", state) }) /** Each integration source with the scrape targets and connections that send as it. */ diff --git a/packages/backend/src/services/integrations/PlanetScaleDiscoveryService.test.ts b/packages/backend/src/services/integrations/PlanetScaleDiscoveryService.test.ts index 6873e025f4..fcdd8b4e72 100644 --- a/packages/backend/src/services/integrations/PlanetScaleDiscoveryService.test.ts +++ b/packages/backend/src/services/integrations/PlanetScaleDiscoveryService.test.ts @@ -518,7 +518,11 @@ describe("PlanetScaleDiscoveryService", () => { const { state } = yield* oauth.startConnect(asOrgId("org_1"), asUserId("user_1"), { callbackUrl: "https://api.example.com/api/integrations/planetscale/callback", }) - yield* oauth.completeConnect("auth-code", state) + yield* oauth.completeConnect( + { orgId: asOrgId("org_1"), userId: asUserId("user_1") }, + "auth-code", + state, + ) const created = yield* targets.create( asOrgId("org_1"), new CreateScrapeTargetRequest({ diff --git a/packages/backend/src/services/integrations/PlanetScaleService.test.ts b/packages/backend/src/services/integrations/PlanetScaleService.test.ts index b729bb18cf..5639248b98 100644 --- a/packages/backend/src/services/integrations/PlanetScaleService.test.ts +++ b/packages/backend/src/services/integrations/PlanetScaleService.test.ts @@ -129,7 +129,11 @@ const connect = (orgId: string) => const { state } = yield* oauth.startConnect(asOrgId(orgId), asUserId("user_1"), { callbackUrl: "https://api.example.com/api/integrations/planetscale/callback", }) - yield* oauth.completeConnect("auth-code", state) + yield* oauth.completeConnect( + { orgId: asOrgId(orgId), userId: asUserId("user_1") }, + "auth-code", + state, + ) yield* connections.finalizeOrgSelection(asOrgId(orgId), { organization: "acme" }) }) diff --git a/packages/backend/src/services/integrations/SupabaseConnectionService.test.ts b/packages/backend/src/services/integrations/SupabaseConnectionService.test.ts index 69b77f07b4..75bd3b2e92 100644 --- a/packages/backend/src/services/integrations/SupabaseConnectionService.test.ts +++ b/packages/backend/src/services/integrations/SupabaseConnectionService.test.ts @@ -70,7 +70,7 @@ const finalize = (fake: Fake) => callbackUrl: "https://api.example.com/api/integrations/supabase/callback", }) fake.state.codeChallenge = new URL(redirectUrl).searchParams.get("code_challenge") - const granted = yield* oauth.completeConnect("code", state) + const granted = yield* oauth.completeConnect({ orgId: ORG, userId: USER }, "code", state) return yield* connection.finalizeConnect( granted.orgId, granted.connectedByUserId, diff --git a/packages/backend/src/services/integrations/SupabaseInsightsService.test.ts b/packages/backend/src/services/integrations/SupabaseInsightsService.test.ts index 3de76affab..b1b6f6e1e0 100644 --- a/packages/backend/src/services/integrations/SupabaseInsightsService.test.ts +++ b/packages/backend/src/services/integrations/SupabaseInsightsService.test.ts @@ -115,7 +115,11 @@ const connect = (world: World) => }, ) world.state.codeChallenge = new URL(redirectUrl).searchParams.get("code_challenge") - const granted = yield* oauth.completeConnect("code", state) + const granted = yield* oauth.completeConnect( + { orgId: ORG, userId: Schema.decodeUnknownSync(UserId)("user_a") }, + "code", + state, + ) yield* connection.finalizeConnect(granted.orgId, granted.connectedByUserId, granted.organization) yield* connection.sync(granted.orgId) }) diff --git a/packages/backend/src/services/integrations/SupabaseLogsService.test.ts b/packages/backend/src/services/integrations/SupabaseLogsService.test.ts index 05869615e3..25f8bb7498 100644 --- a/packages/backend/src/services/integrations/SupabaseLogsService.test.ts +++ b/packages/backend/src/services/integrations/SupabaseLogsService.test.ts @@ -142,7 +142,11 @@ const connectWithLogs = (world: World) => callbackUrl: "https://api.example.com/api/integrations/supabase/callback", }) world.supabase.state.codeChallenge = new URL(redirectUrl).searchParams.get("code_challenge") - const granted = yield* oauth.completeConnect("code", state) + const granted = yield* oauth.completeConnect( + { orgId: ORG, userId: asUserId("user_a") }, + "code", + state, + ) yield* connection.finalizeConnect(granted.orgId, granted.connectedByUserId, granted.organization) yield* connection.sync(granted.orgId) yield* (yield* SupabaseLogsService).setLogsEnabled(ORG, asRef(REF_A), true) diff --git a/packages/backend/src/services/integrations/supabase/scrape-targets.test.ts b/packages/backend/src/services/integrations/supabase/scrape-targets.test.ts index 23ebc6cb08..192f41ca3d 100644 --- a/packages/backend/src/services/integrations/supabase/scrape-targets.test.ts +++ b/packages/backend/src/services/integrations/supabase/scrape-targets.test.ts @@ -69,7 +69,7 @@ const finalize = (fake: Fake) => }, ) fake.state.codeChallenge = new URL(redirectUrl).searchParams.get("code_challenge") - const granted = yield* oauth.completeConnect("code", state) + const granted = yield* oauth.completeConnect({ orgId: ORG, userId: USER }, "code", state) return yield* (yield* SupabaseConnectionService).finalizeConnect( granted.orgId, granted.connectedByUserId, diff --git a/packages/backend/src/services/integrations/vcs/vendor/github/GithubConnectService.ts b/packages/backend/src/services/integrations/vcs/vendor/github/GithubConnectService.ts index 86163866f5..0c5f38ef1e 100644 --- a/packages/backend/src/services/integrations/vcs/vendor/github/GithubConnectService.ts +++ b/packages/backend/src/services/integrations/vcs/vendor/github/GithubConnectService.ts @@ -25,6 +25,11 @@ import { BACKFILL_WINDOW_MS } from "@maple/backend/services/integrations/vcs/Vcs import { VcsSyncQueue } from "@maple/backend/services/integrations/vcs/VcsSyncQueue" import { GithubAppClient, type GithubAppError } from "./GithubAppClient" import { githubWebBaseUrl } from "./github-hosts" +import { + isStateInitiator, + stateInitiatorMismatch, + type OAuthStateInitiator, +} from "@maple/backend/services/auth/oauth/state-initiator" // The dashboard connect flow for the GitHub App. Bridges a real GitHub // installation into a `vcs_installations` row owned by a Maple org, then hands @@ -86,6 +91,8 @@ export interface GithubConnectServiceApi { IntegrationsValidationError | IntegrationsPersistenceError > readonly completeConnect: ( + /** The signed-in user finishing the install; must be the one that started it. */ + initiator: OAuthStateInitiator, installationId: string, state: string, /** @@ -270,6 +277,7 @@ export class GithubConnectService extends Context.Service cleanupTestDbs(trackedDbs)) @@ -68,11 +70,7 @@ const connectResponders = (...rest: Array<() => Response>) => [ // Wire GithubConnectService over an in-memory PGlite (real repo + state repo), a // real GithubAppClient backed by the stubbed HttpClient, and a recording queue. -const connectLayer = ( - testDb: TestDb, - http: Layer.Layer, - sent: Array, -) => { +const connectLayer = (testDb: TestDb, http: Layer.Layer, sent: Array) => { const env = testEnv(GITHUB_APP_CONFIG) const data = Layer.mergeAll( VcsRepository.layer, @@ -120,7 +118,7 @@ describe("GithubConnectService", () => { returnTo: "https://web.localhost/integrations?integration=github", }) - const result = yield* svc.completeConnect("42", state, TEST_CODE) + const result = yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) assert.strictEqual(result.orgId, orgId) assert.strictEqual(result.returnTo, "https://web.localhost/integrations?integration=github") @@ -159,7 +157,7 @@ describe("GithubConnectService", () => { callbackUrl: "https://tunnel.example/cb", }) - const exit = yield* svc.completeConnect("42", state, TEST_CODE).pipe(Effect.exit) + const exit = yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE).pipe(Effect.exit) assert.ok(findError(exit) instanceof IntegrationsValidationError) const found = yield* repo.resolveInstallation("github", "42") @@ -180,7 +178,7 @@ describe("GithubConnectService", () => { }) // No `code` argument → rejected before any installation row is written. - const exit = yield* svc.completeConnect("42", state).pipe(Effect.exit) + const exit = yield* svc.completeConnect(INITIATOR, "42", state).pipe(Effect.exit) assert.ok(findError(exit) instanceof IntegrationsValidationError) assert.ok(Option.isNone(yield* repo.resolveInstallation("github", "42"))) assert.strictEqual(sent.length, 0) @@ -199,7 +197,7 @@ describe("GithubConnectService", () => { callbackUrl: "https://tunnel.example/cb", }) - const exit = yield* svc.completeConnect("42", state, TEST_CODE).pipe(Effect.exit) + const exit = yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE).pipe(Effect.exit) assert.ok(findError(exit) instanceof IntegrationsValidationError) assert.ok(Option.isNone(yield* repo.resolveInstallation("github", "42"))) assert.strictEqual(sent.length, 0) @@ -219,14 +217,14 @@ describe("GithubConnectService", () => { const first = yield* svc.startConnect(orgId, asUserId("user_1"), { callbackUrl: "https://tunnel.example/cb", }) - yield* svc.completeConnect("42", first.state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", first.state, TEST_CODE) assert.ok(Option.isSome(yield* repo.resolveInstallation("github", "42"))) // Second connect from the SAME org, no code → still allowed (reconnect). const second = yield* svc.startConnect(orgId, asUserId("user_1"), { callbackUrl: "https://tunnel.example/cb", }) - const result = yield* svc.completeConnect("42", second.state) + const result = yield* svc.completeConnect(INITIATOR, "42", second.state) assert.strictEqual(result.orgId, orgId) const found = yield* repo.resolveInstallation("github", "42") assert.ok(Option.isSome(found)) @@ -238,13 +236,48 @@ describe("GithubConnectService", () => { }).pipe(Effect.provide(connectLayer(testDb, http, sent))) }) + it.effect( + "completeConnect refuses an install URL finished by another Maple user, calling GitHub never", + () => { + const testDb = createTestDb(trackedDbs) + const sent: Array = [] + let calls = 0 + const http = scriptedHttp( + connectResponders().map((respond) => () => { + calls += 1 + return respond() + }), + ) + return Effect.gen(function* () { + const svc = yield* GithubConnectService + const repo = yield* VcsRepository + const { state } = yield* svc.startConnect(asOrgId("org_test"), asUserId("user_1"), { + callbackUrl: "https://tunnel.example/cb", + }) + + // The victim finishes the attacker's install URL from their own Maple session. + const victim = { orgId: asOrgId("org_victim"), userId: asUserId("user_victim") } + const exit = yield* svc.completeConnect(victim, "42", state, TEST_CODE).pipe(Effect.exit) + assert.ok(findError(exit) instanceof IntegrationsValidationError) + assert.strictEqual(calls, 0) + assert.ok(Option.isNone(yield* repo.resolveInstallation("github", "42"))) + assert.strictEqual(sent.length, 0) + + // The forwarded state is spent, so the attacker cannot finish it afterwards either. + const replay = yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE).pipe(Effect.exit) + assert.ok(findError(replay) instanceof IntegrationsValidationError) + assert.ok(Option.isNone(yield* repo.resolveInstallation("github", "42"))) + }).pipe(Effect.provide(connectLayer(testDb, http, sent))) + }, + ) + it.effect("completeConnect rejects an unrecognized state without calling GitHub", () => { const testDb = createTestDb(trackedDbs) const sent: Array = [] const http = scriptedHttp(connectResponders()) return Effect.gen(function* () { const svc = yield* GithubConnectService - const exit = yield* svc.completeConnect("42", "not-a-real-state").pipe(Effect.exit) + const exit = yield* svc.completeConnect(INITIATOR, "42", "not-a-real-state").pipe(Effect.exit) assert.ok(findError(exit) instanceof IntegrationsValidationError) assert.strictEqual(sent.length, 0) }).pipe(Effect.provide(connectLayer(testDb, http, sent))) @@ -265,7 +298,7 @@ describe("GithubConnectService", () => { // Advance past the 10-minute STATE_TTL_MS before the callback returns. yield* TestClock.adjust("11 minutes") - const exit = yield* svc.completeConnect("42", state, TEST_CODE).pipe(Effect.exit) + const exit = yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE).pipe(Effect.exit) assert.ok(findError(exit) instanceof IntegrationsValidationError) assert.ok(Option.isNone(yield* repo.resolveInstallation("github", "42"))) assert.strictEqual(sent.length, 0) @@ -283,11 +316,11 @@ describe("GithubConnectService", () => { }) // First callback succeeds and enqueues exactly one created sync job. - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) assert.strictEqual(sent.length, 1) // Replaying the now-consumed state is rejected; no second job is enqueued. - const exit = yield* svc.completeConnect("42", state, TEST_CODE).pipe(Effect.exit) + const exit = yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE).pipe(Effect.exit) assert.ok(findError(exit) instanceof IntegrationsValidationError) assert.strictEqual(sent.length, 1) }).pipe(Effect.provide(connectLayer(testDb, http, sent))) @@ -307,7 +340,7 @@ describe("GithubConnectService", () => { const { state } = yield* svc.startConnect(orgId, userId, { callbackUrl: "https://tunnel.example/api/integrations/github/callback", }) - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) yield* upsertReposFor(repo, "42", [ { externalRepoId: "7", @@ -367,7 +400,7 @@ describe("GithubConnectService", () => { const { state } = yield* svc.startConnect(orgId, userId, { callbackUrl: "https://tunnel.example/api/integrations/github/callback", }) - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) yield* upsertReposFor(repo, "42", [ { externalRepoId: "7", @@ -413,7 +446,7 @@ describe("GithubConnectService", () => { const { state } = yield* svc.startConnect(orgId, userId, { callbackUrl: "https://tunnel.example/api/integrations/github/callback", }) - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) yield* upsertReposFor(repo, "42", [ { externalRepoId: "7", @@ -458,7 +491,7 @@ describe("GithubConnectService", () => { const { state } = yield* svc.startConnect(orgId, userId, { callbackUrl: "https://tunnel.example/api/integrations/github/callback", }) - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) // Two repos: "7" (to delete) and "8" (kept), each with a commit. yield* upsertReposFor(repo, "42", [ @@ -543,7 +576,7 @@ describe("GithubConnectService", () => { const { state } = yield* svc.startConnect(orgId, userId, { callbackUrl: "https://tunnel.example/cb", }) - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) yield* upsertReposFor(repo, "42", [ { externalRepoId: "7", @@ -588,7 +621,7 @@ describe("GithubConnectService", () => { const { state } = yield* svc.startConnect(orgId, asUserId("user_1"), { callbackUrl: "https://tunnel.example/cb", }) - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) yield* upsertReposFor(repo, "42", [ { externalRepoId: "7", @@ -739,7 +772,7 @@ describe("GithubConnectService", () => { const { state } = yield* svc.startConnect(orgId, userId, { callbackUrl: "https://tunnel.example/cb", }) - yield* svc.completeConnect("42", state, TEST_CODE) + yield* svc.completeConnect(INITIATOR, "42", state, TEST_CODE) yield* upsertReposFor(repo, "42", [ { externalRepoId: "7", diff --git a/packages/domain/src/http/integrations.ts b/packages/domain/src/http/integrations.ts index c9403f7585..c95f71a82d 100644 --- a/packages/domain/src/http/integrations.ts +++ b/packages/domain/src/http/integrations.ts @@ -9,6 +9,7 @@ import { SourceId, SupabaseProjectRef, UserId, + ChatConnectorId, } from "../primitives" import { SessionAuthorization } from "./current-tenant" import { HttpTaggedError } from "./error-policy" @@ -1334,6 +1335,51 @@ export type IntegrationHttpError = | IntegrationsUpstreamError | IntegrationsPersistenceError +/** The popup providers whose public OAuth callback hands the query to the dashboard to finish. */ +export const OAuthCallbackProvider = Schema.Literals([ + "hazel", + "github", + "cloudflare", + "planetscale", + "supabase", +]) +export type OAuthCallbackProvider = typeof OAuthCallbackProvider.Type + +/** The provider's callback query string, forwarded verbatim by the public callback. */ +const CallbackQuery = Schema.String.check(Schema.isMaxLength(8 * 1024)) + +export class OAuthCompleteRequest extends Schema.Class("OAuthCompleteRequest")({ + provider: OAuthCallbackProvider, + query: CallbackQuery, +}) {} + +/** Failures are an outcome, not an error: the copy is what the popup shows. */ +export class OAuthCompleteResponse extends Schema.Class("OAuthCompleteResponse")({ + status: Schema.Literals(["success", "error"]), + message: Schema.String, + returnTo: Schema.NullOr(IntegrationReturnPath), +}) {} + +export const ChatCallbackKind = Schema.Literals(["install", "identity"]) +export type ChatCallbackKind = typeof ChatCallbackKind.Type + +/** A closed set rather than a message: the dashboard maps each code to its own copy. */ +export const ChatCallbackReason = Schema.Literals(["state", "conflict", "unconfigured", "unknown", "failed"]) +export type ChatCallbackReason = typeof ChatCallbackReason.Type + +export class ChatCompleteRequest extends Schema.Class("ChatCompleteRequest")({ + connector: ChatConnectorId, + kind: ChatCallbackKind, + query: CallbackQuery, +}) {} + +export class ChatCompleteResponse extends Schema.Class("ChatCompleteResponse")({ + status: Schema.Literals(["success", "error"]), + reason: Schema.NullOr(ChatCallbackReason), + /** The workspace or account display name the platform reported. Untrusted. */ + name: Schema.NullOr(Schema.String), +}) {} + /** * Dashboard control surface for the Hazel, Cloudflare, Railway and GitHub * integrations, plus VCS lookups. Internal transport: session-only, absent from @@ -1733,5 +1779,19 @@ export class IntegrationsApiGroup extends HttpApiGroup.make("integrations") ], }), ) + .add( + // The public callbacks only forward the provider's query here. The signed-in session + // finishing the flow must be the user and org that started it (`OAuthStateInitiator`). + HttpApiEndpoint.post("oauthComplete", "/oauth/complete", { + payload: OAuthCompleteRequest, + success: OAuthCompleteResponse, + }), + ) + .add( + HttpApiEndpoint.post("chatComplete", "/chat/complete", { + payload: ChatCompleteRequest, + success: ChatCompleteResponse, + }), + ) .prefix("/internal/integrations") .middleware(SessionAuthorization) {} From 0677be1f24d8c13410cd45f5a72c42ace18d4fb2 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Thu, 22 Oct 2026 13:27:37 +0200 Subject: [PATCH 2/5] test(integrations): cover the callback completer; log refused completions The completer that wires the dashboard session into each provider's completion had no test: cover the initiator hand-off per provider, the failure-to-copy and chat reason mappings, the pre-exchange refusals (issuer mismatch, missing code/state, provider error, pending GitHub approval) and the PlanetScale single-org finalize. Refusals before any exchange now log a warning with a fixed reason and stamp maple.integration.outcome, as do completions and failures, so a refused completion leaves a trace. The initiator-binding test asserts its finisher differs from the initiator in exactly one field. --- .oxlintrc.json | 1 + .../internal/integrations-complete.test.ts | 296 ++++++++++++++++++ .../routes/internal/integrations-complete.ts | 71 +++-- .../auth/oauth/connection-helpers.test.ts | 49 +-- 4 files changed, 366 insertions(+), 51 deletions(-) create mode 100644 apps/api/src/routes/internal/integrations-complete.test.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index a9bc85fe38..ae2795287e 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -286,6 +286,7 @@ "packages/backend/src/services/integrations/vcs/VcsProviderRegistry.ts", "apps/api/src/routes/internal/integrations.http.ts", "apps/api/src/routes/internal/integrations-complete.ts", + "apps/api/src/routes/internal/integrations-complete.test.ts", "packages/backend/src/services/integrations/vcs/vendor/**", "packages/backend/src/services/integrations/vcs/__tests__/**" ], diff --git a/apps/api/src/routes/internal/integrations-complete.test.ts b/apps/api/src/routes/internal/integrations-complete.test.ts new file mode 100644 index 0000000000..e35197b68a --- /dev/null +++ b/apps/api/src/routes/internal/integrations-complete.test.ts @@ -0,0 +1,296 @@ +import { assert, describe, it } from "@effect/vitest" +import { + IntegrationsForbiddenError, + IntegrationsPersistenceError, + IntegrationsUpstreamError, + IntegrationsValidationError, + OrgId, + PlanetScaleIntegrationStatus, + SupabaseIntegrationStatus, + UserId, +} from "@maple/domain/http" +import { ChatConnectorId } from "@maple/domain/primitives" +import { Effect, Layer, Schema } from "effect" +import { CloudflareOAuthService } from "@maple/backend/services/auth/CloudflareOAuthService" +import { HazelOAuthService } from "@maple/backend/services/auth/HazelOAuthService" +import { PlanetScaleOAuthService } from "@maple/backend/services/auth/PlanetScaleOAuthService" +import { SupabaseOAuthService } from "@maple/backend/services/auth/SupabaseOAuthService" +import type { OAuthStateInitiator } from "@maple/backend/services/auth/oauth/state-initiator" +import { CloudflareAnalyticsService } from "@maple/backend/services/integrations/CloudflareAnalyticsService" +import { ChatWorkspaceService } from "@maple/backend/services/integrations/ChatWorkspaceService" +import { PlanetScaleConnectionService } from "@maple/backend/services/integrations/PlanetScaleConnectionService" +import { SupabaseConnectionService } from "@maple/backend/services/integrations/SupabaseConnectionService" +import { GithubConnectService } from "@maple/backend/services/integrations/vcs/vendor/github/GithubConnectService" +import { makeIntegrationCompleter } from "./integrations-complete" + +const ORG = Schema.decodeSync(OrgId)("org_session") +const SESSION: OAuthStateInitiator = { orgId: ORG, userId: Schema.decodeSync(UserId)("user_session") } +const SLACK = Schema.decodeSync(ChatConnectorId)("slack") +const SUPABASE_API = "https://api.supabase.com" +const MISMATCH = + "This authorization was started by a different Maple user or organization. Start it again from your own account." + +/** Every call a stub received, so a test can assert who the services were asked to finish for. */ +type Calls = Array<{ readonly service: string; readonly args: ReadonlyArray }> + +/** What every OAuth completion can fail with; chat installs add `IntegrationsForbiddenError`. */ +type Failure = IntegrationsValidationError | IntegrationsUpstreamError | IntegrationsPersistenceError + +const PS_STATUS = new PlanetScaleIntegrationStatus({ + connected: true, + pendingOrgSelection: false, + organization: "acme-0", + connectedByUserId: SESSION.userId, + detectedPermissions: null, + metricsAuth: "oauth", + scrapeTarget: null, + lastInventoryAt: null, + lastInventoryError: null, + revokedAt: null, + expiresAt: null, +}) + +const SB_STATUS = new SupabaseIntegrationStatus({ + configured: true, + connected: true, + organization: null, + connectedByUserId: SESSION.userId, + connectedAt: null, + revoked: false, + lastDiscoveryAt: null, + lastDiscoveryError: null, + projects: [], +}) + +const harness = ( + calls: Calls, + options: { + readonly fail?: Failure + readonly chatFail?: Failure | IntegrationsForbiddenError + readonly psOrgs?: number + } = {}, +) => { + const record = (service: string, args: ReadonlyArray) => + Effect.sync(() => calls.push({ service, args })) + const outcome = (value: A) => + options.fail === undefined ? Effect.succeed(value) : Effect.fail(options.fail) + const granted = { orgId: ORG, returnTo: "/integrations?integration=hazel" } + return Layer.mergeAll( + Layer.mock(HazelOAuthService, { + completeConnect: (...args) => record("hazel", args).pipe(Effect.andThen(outcome(granted))), + }), + Layer.mock(CloudflareOAuthService, { + completeConnect: (...args) => + record("cloudflare", args).pipe(Effect.andThen(outcome({ ...granted, returnTo: null }))), + }), + Layer.mock(CloudflareAnalyticsService, { + resetOrgState: (orgId) => record("cloudflare.reset", [orgId]), + }), + Layer.mock(PlanetScaleOAuthService, { + completeConnect: (...args) => + record("planetscale", args).pipe( + Effect.andThen( + outcome({ + ...granted, + organizations: Array.from({ length: options.psOrgs ?? 1 }, (_, i) => ({ + id: `ps_${i}`, + name: `acme-${i}`, + })), + }), + ), + ), + }), + Layer.mock(PlanetScaleConnectionService, { + finalizeOrgSelection: (orgId, request) => + record("planetscale.finalize", [orgId, request.organization]).pipe(Effect.as(PS_STATUS)), + }), + Layer.mock(SupabaseOAuthService, { + isConfigured: true, + apiBaseUrl: SUPABASE_API, + completeConnect: (...args) => + record("supabase", args).pipe( + Effect.andThen( + outcome({ + ...granted, + connectedByUserId: SESSION.userId, + organization: { id: "sb_1", slug: "acme", name: "Acme" }, + }), + ), + ), + }), + Layer.mock(SupabaseConnectionService, { + finalizeConnect: (...args) => record("supabase.finalize", args).pipe(Effect.as(SB_STATUS)), + }), + Layer.mock(GithubConnectService, { + completeConnect: (...args) => + record("github", args).pipe( + Effect.andThen(outcome({ orgId: ORG, returnTo: "https://evil.example/" })), + ), + }), + Layer.mock(ChatWorkspaceService, { + completeInstall: (...args) => + record("chat.install", args).pipe( + Effect.andThen( + options.chatFail === undefined + ? Effect.succeed({ orgId: ORG, name: "Acme HQ" }) + : Effect.fail(options.chatFail), + ), + ), + completeLink: (...args) => + record("chat.link", args).pipe(Effect.andThen(outcome({ orgId: ORG, displayName: "ada" }))), + }), + ) +} + +const run = ( + calls: Calls, + use: (completer: Effect.Success) => Effect.Effect, + options?: Parameters[1], +) => Effect.flatMap(makeIntegrationCompleter, use).pipe(Effect.provide(harness(calls, options))) + +describe("makeIntegrationCompleter", () => { + it.effect.each([ + ["hazel", "code=c1&state=s1"], + ["cloudflare", "code=c1&state=s1"], + ["planetscale", "code=c1&state=s1"], + ["supabase", `code=c1&state=s1&iss=${encodeURIComponent(`${SUPABASE_API}/`)}`], + ] as const)( + "finishes %s for the signed-in session, with the forwarded code and state", + ([provider, query]) => { + const calls: Calls = [] + return Effect.gen(function* () { + const response = yield* run(calls, (completer) => + completer.completeOAuth(SESSION, provider, query), + ) + assert.strictEqual(response.status, "success") + assert.deepStrictEqual(calls[0], { service: provider, args: [SESSION, "c1", "s1"] }) + }) + }, + ) + + it.effect("finishes a GitHub install for the session and drops a return link off the dashboard", () => { + const calls: Calls = [] + return Effect.gen(function* () { + const response = yield* run(calls, (completer) => + completer.completeOAuth( + SESSION, + "github", + "installation_id=42&setup_action=install&code=c1&state=s1", + ), + ) + assert.strictEqual(response.status, "success") + assert.isNull(response.returnTo) + assert.deepStrictEqual(calls[0], { service: "github", args: [SESSION, "42", "s1", "c1"] }) + }) + }) + + it.effect("shows the service's refusal of a session that did not start the flow", () => { + const calls: Calls = [] + return Effect.gen(function* () { + const response = yield* run( + calls, + (completer) => completer.completeOAuth(SESSION, "hazel", "code=c1&state=s1"), + { fail: new IntegrationsValidationError({ message: MISMATCH }) }, + ) + assert.deepStrictEqual( + { status: response.status, message: response.message, returnTo: response.returnTo }, + { status: "error", message: MISMATCH, returnTo: null }, + ) + }) + }) + + it.effect.each([ + [ + new IntegrationsUpstreamError({ message: "Token exchange failed: 400" }), + "Token exchange failed: 400", + ], + [ + new IntegrationsPersistenceError({ message: "db down" }), + "Failed to complete the Cloudflare connection", + ], + ] as const)("reduces a %s failure to its copy", ([error, copy]) => { + const calls: Calls = [] + return Effect.gen(function* () { + const response = yield* run( + calls, + (completer) => completer.completeOAuth(SESSION, "cloudflare", "code=c1&state=s1"), + { fail: error }, + ) + assert.strictEqual(response.message, copy) + }) + }) + + it.effect.each([ + ["supabase", "code=c1&state=s1&iss=https%3A%2F%2Fevil.example", "unexpected issuer"], + ["hazel", "state=s1", "Missing code or state"], + ["hazel", "error=access_denied&error_description=Denied", "Denied"], + ["github", "setup_action=request&state=s1", "an org admin must approve it"], + ] as const)("refuses a %s callback before calling any service: %s", ([provider, query, copy]) => { + const calls: Calls = [] + return Effect.gen(function* () { + const response = yield* run(calls, (completer) => + completer.completeOAuth(SESSION, provider, query), + ) + assert.strictEqual(response.status, "error") + assert.include(response.message, copy) + assert.lengthOf(calls, 0) + }) + }) + + it.effect("binds a single-org PlanetScale grant and leaves several to the picker", () => { + const one: Calls = [] + const many: Calls = [] + return Effect.gen(function* () { + yield* run(one, (completer) => + completer.completeOAuth(SESSION, "planetscale", "code=c1&state=s1"), + ) + assert.deepStrictEqual(one[1], { service: "planetscale.finalize", args: [ORG, "acme-0"] }) + const picker = yield* run( + many, + (completer) => completer.completeOAuth(SESSION, "planetscale", "code=c1&state=s1"), + { psOrgs: 2 }, + ) + assert.include(picker.message, "Choose which PlanetScale organization") + assert.lengthOf(many, 1) + }) + }) + + it.effect("finishes a chat install and an account link for the session", () => { + const calls: Calls = [] + return Effect.gen(function* () { + const installed = yield* run(calls, (completer) => + completer.completeChat(SESSION, SLACK, "install", "code=c1&state=s1"), + ) + const linked = yield* run(calls, (completer) => + completer.completeChat(SESSION, SLACK, "identity", "code=c2&state=s2"), + ) + assert.deepStrictEqual([installed.name, linked.name], ["Acme HQ", "ada"]) + assert.deepStrictEqual( + calls.map(({ service, args }) => [service, args[0], args[1]]), + [ + ["chat.install", SESSION, SLACK], + ["chat.link", SESSION, SLACK], + ], + ) + }) + }) + + it.effect.each([ + [new IntegrationsValidationError({ message: MISMATCH }), "state"], + [new IntegrationsForbiddenError({ message: "linked elsewhere" }), "conflict"], + [new IntegrationsPersistenceError({ message: "db down" }), "failed"], + ] as const)("maps a chat failure to its reason code: %s", ([error, reason]) => { + const calls: Calls = [] + return Effect.gen(function* () { + const response = yield* run( + calls, + (completer) => completer.completeChat(SESSION, SLACK, "install", "code=c1&state=s1"), + { chatFail: error }, + ) + assert.deepStrictEqual( + { status: response.status, reason: response.reason, name: response.name }, + { status: "error", reason, name: null }, + ) + }) + }) +}) diff --git a/apps/api/src/routes/internal/integrations-complete.ts b/apps/api/src/routes/internal/integrations-complete.ts index fa2a5a5401..cb4c0a32e1 100644 --- a/apps/api/src/routes/internal/integrations-complete.ts +++ b/apps/api/src/routes/internal/integrations-complete.ts @@ -72,7 +72,6 @@ export const makeIntegrationCompleter = Effect.gen(function* () { const supabaseOAuth = yield* SupabaseOAuthService const supabaseConnection = yield* SupabaseConnectionService const chat = yield* ChatWorkspaceService - // RFC 9207 mix-up defense: Supabase names itself in `iss`; a different issuer is refused. const supabaseIssuer = supabaseOAuth.apiBaseUrl const complete = ( @@ -133,17 +132,6 @@ export const makeIntegrationCompleter = Effect.gen(function* () { }), ) case "supabase": { - const issuer = params.get("iss") - if (issuer !== null && issuer.replace(/\/$/, "") !== supabaseIssuer) { - return Effect.succeed( - new OAuthCompleteResponse({ - status: "error", - message: - "The authorization came from an unexpected issuer. Restart the connect flow.", - returnTo: null, - }), - ) - } // One grant is one Supabase organization: bind it and return. return supabaseOAuth.completeConnect(initiator, code, state).pipe( Effect.flatMap((result) => @@ -183,48 +171,69 @@ export const makeIntegrationCompleter = Effect.gen(function* () { const params = new URLSearchParams(query) const failed = (message: string) => new OAuthCompleteResponse({ status: "error", message, returnTo: null }) + /** A callback refused before any exchange: logged with a fixed reason, never the query. */ + const refuse = (reason: string, message: string) => + Effect.logWarning(`${label} OAuth callback refused`, { provider, reason }).pipe( + Effect.andThen(Effect.annotateCurrentSpan({ "maple.integration.outcome": reason })), + Effect.as(failed(message)), + ) - const oauthError = params.get("error") - if (oauthError !== null) { - return failed(params.get("error_description") || `${label} returned an error`) + if (params.get("error") !== null) { + return yield* refuse( + "provider_error", + params.get("error_description") || `${label} returned an error`, + ) } const state = params.get("state") if (provider === "github") { // `setup_action=request`: the org requires admin approval, so there is no installation yet. if (params.get("installation_id") === null) { - return failed( - params.get("setup_action") === "request" - ? "Installation requested — an org admin must approve it on GitHub, then reconnect." - : "Missing installation_id in callback", - ) + return yield* params.get("setup_action") === "request" + ? refuse( + "install_requested", + "Installation requested — an org admin must approve it on GitHub, then reconnect.", + ) + : refuse("missing_installation_id", "Missing installation_id in callback") } if (state === null) { - return failed( + return yield* refuse( + "missing_state", "Missing state in callback — GitHub did not return it. Restart the connection from the Maple dashboard.", ) } } const code = params.get("code") ?? "" if (state === null || (provider !== "github" && code === "")) { - return failed("Missing code or state in callback") + return yield* refuse("missing_code_or_state", "Missing code or state in callback") + } + // RFC 9207 mix-up defense: Supabase names itself in `iss`; a different issuer is refused. + const issuer = params.get("iss") + if (provider === "supabase" && issuer !== null && issuer.replace(/\/$/, "") !== supabaseIssuer) { + return yield* refuse( + "issuer_mismatch", + "The authorization came from an unexpected issuer. Restart the connect flow.", + ) } return yield* complete(initiator, provider, code, state, params).pipe( - Effect.map((result) => - result instanceof OAuthCompleteResponse - ? result - : new OAuthCompleteResponse({ - status: "success", - message: result.message, - returnTo: Option.getOrNull(decodeReturnPath(result.returnTo)), - }), + Effect.tap(() => Effect.annotateCurrentSpan({ "maple.integration.outcome": "completed" })), + Effect.map( + (result) => + new OAuthCompleteResponse({ + status: "success", + message: result.message, + returnTo: Option.getOrNull(decodeReturnPath(result.returnTo)), + }), ), // The response reduces failures to short copy; the real cause still goes to the log. Effect.catch((error) => Effect.logError(`${label} OAuth completeConnect failed`, { tag: error._tag, message: error.message, - }).pipe(Effect.as(failed(failureMessage(label, error)))), + }).pipe( + Effect.andThen(Effect.annotateCurrentSpan({ "maple.integration.outcome": "failed" })), + Effect.as(failed(failureMessage(label, error))), + ), ), ) }) diff --git a/packages/backend/src/services/auth/oauth/connection-helpers.test.ts b/packages/backend/src/services/auth/oauth/connection-helpers.test.ts index e81e726745..e6220fa904 100644 --- a/packages/backend/src/services/auth/oauth/connection-helpers.test.ts +++ b/packages/backend/src/services/auth/oauth/connection-helpers.test.ts @@ -13,6 +13,7 @@ import { OAuthTokenResponseSchema, type OAuthTokenEndpointConfig, } from "./connection-helpers" +import type { OAuthStateInitiator } from "./state-initiator" const trackedDbs: TestDb[] = [] afterEach(() => cleanupTestDbs(trackedDbs)) @@ -443,29 +444,37 @@ describe("requireStateRow initiator binding", () => { ), ) - it.live.each([ + const finishers: ReadonlyArray = [ ["another user in the same org", { orgId: ORG, userId: asUserId("user_somebody_else") }], ["the same user in another org", { orgId: asOrgId("org_somebody_else"), userId: USER }], - ])("refuses a forwarded authorization finished by %s, and burns the state", ([, finisher]) => { - const testDb = createTestDb(trackedDbs) - return Effect.gen(function* () { - const helpers = yield* makeHelpers() - yield* insertState(helpers, "forwarded-state") - const error = yield* helpers.requireStateRow("forwarded-state", finisher).pipe(Effect.flip) - assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") - assert.include(error.message, "different Maple user or organization") - // The initiator cannot redeem it afterwards either: the forwarded state is spent. - const replay = yield* helpers.requireStateRow("forwarded-state", INITIATOR).pipe(Effect.flip) - assert.include(replay.message, "not recognized") - }).pipe( - Effect.provide( - provideBase( - testDb, - fetchLayer(() => freshTokens()), + ] + // `@effect/vitest`'s `each` is `it.for`: the whole row is the first argument. + it.live.each(finishers)( + "refuses a forwarded authorization finished by %s, and burns the state", + ([, finisher]) => { + const testDb = createTestDb(trackedDbs) + return Effect.gen(function* () { + // A real initiator that differs in exactly one field, so the refusal is the comparison's. + assert.isTrue(finisher.orgId === ORG || finisher.userId === USER) + assert.isFalse(finisher.orgId === ORG && finisher.userId === USER) + const helpers = yield* makeHelpers() + yield* insertState(helpers, "forwarded-state") + const error = yield* helpers.requireStateRow("forwarded-state", finisher).pipe(Effect.flip) + assert.strictEqual(error._tag, "@maple/http/errors/IntegrationsValidationError") + assert.include(error.message, "different Maple user or organization") + // The initiator cannot redeem it afterwards either: the forwarded state is spent. + const replay = yield* helpers.requireStateRow("forwarded-state", INITIATOR).pipe(Effect.flip) + assert.include(replay.message, "not recognized") + }).pipe( + Effect.provide( + provideBase( + testDb, + fetchLayer(() => freshTokens()), + ), ), - ), - ) - }) + ) + }, + ) it.live("returns the row to the user and org that started the flow", () => { const testDb = createTestDb(trackedDbs) From 7188dc23ead9286061a19fa96eab7dd444fa60d3 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Thu, 22 Oct 2026 13:54:46 +0200 Subject: [PATCH 3/5] fix(integrations): keep the response status on the callback spans The callback spans are the only trace root for these routes; the forwarding rewrite dropped http.response.status_code from them. Every branch is a 302. --- apps/api/src/routes/chat-integration.http.ts | 2 ++ apps/api/src/routes/integrations-callback.http.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/apps/api/src/routes/chat-integration.http.ts b/apps/api/src/routes/chat-integration.http.ts index 92fb925ae4..5ff9b5df5e 100644 --- a/apps/api/src/routes/chat-integration.http.ts +++ b/apps/api/src/routes/chat-integration.http.ts @@ -38,6 +38,8 @@ export const ChatCallbackRouter = HttpRouter.use((router) => const callback = (span: string, kind: DashboardCallbackKind, outcome: string) => Effect.fn(span)(function* (req: HttpServerRequest.HttpServerRequest) { + // Every branch is a 302: this span is the callback's only trace root (ApiObservabilityLive). + yield* Effect.annotateCurrentSpan({ "http.response.status_code": 302 }) const routeParams = yield* HttpRouter.params const connector = decodeConnectorParam(routeParams.connector) if (Option.isNone(connector)) { diff --git a/apps/api/src/routes/integrations-callback.http.ts b/apps/api/src/routes/integrations-callback.http.ts index fa94388720..8ef759bc8a 100644 --- a/apps/api/src/routes/integrations-callback.http.ts +++ b/apps/api/src/routes/integrations-callback.http.ts @@ -72,6 +72,7 @@ export const IntegrationsCallbackRouter = HttpRouter.use((router) => "maple.integration.provider": provider, }, })(function* (req: HttpServerRequest.HttpServerRequest) { + yield* Effect.annotateCurrentSpan({ "http.response.status_code": 302 }) return forwardToDashboard( dashboardCallbackUrl(env.MAPLE_APP_BASE_URL, provider, "oauth", callbackQueryOf(req)), ) From b53403760d2b57a768271d426c343eabc6bbbedc Mon Sep 17 00:00:00 2001 From: Makisuo Date: Fri, 23 Oct 2026 00:43:43 +0200 Subject: [PATCH 4/5] fix(integrations): give the chat callbacks a server span The auto server span is suppressed for these paths, so the handler span is the trace root; make it server-kind with http.route like the provider callbacks. --- apps/api/src/routes/chat-integration.http.ts | 23 +++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/apps/api/src/routes/chat-integration.http.ts b/apps/api/src/routes/chat-integration.http.ts index 5ff9b5df5e..ea07b44fee 100644 --- a/apps/api/src/routes/chat-integration.http.ts +++ b/apps/api/src/routes/chat-integration.http.ts @@ -36,9 +36,13 @@ export const ChatCallbackRouter = HttpRouter.use((router) => const env = yield* Env const appBaseUrl = env.MAPLE_APP_BASE_URL.replace(/\/$/, "") - const callback = (span: string, kind: DashboardCallbackKind, outcome: string) => - Effect.fn(span)(function* (req: HttpServerRequest.HttpServerRequest) { - // Every branch is a 302: this span is the callback's only trace root (ApiObservabilityLive). + const callback = (span: string, path: string, kind: DashboardCallbackKind, outcome: string) => + // Server-kind like `IntegrationsCallbackRouter`: the auto server span is suppressed for these + // paths (ApiObservabilityLive), so this is the callback's only trace root. Every branch is a 302. + Effect.fn(span, { + kind: "server", + attributes: { "http.route": path, "http.request.method": "GET" }, + })(function* (req: HttpServerRequest.HttpServerRequest) { yield* Effect.annotateCurrentSpan({ "http.response.status_code": 302 }) const routeParams = yield* HttpRouter.params const connector = decodeConnectorParam(routeParams.connector) @@ -52,11 +56,20 @@ export const ChatCallbackRouter = HttpRouter.use((router) => ) }) - yield* router.add("GET", CHAT_CALLBACK_PATH, callback("ChatOAuth.callback", "chat-install", "chat")) + yield* router.add( + "GET", + CHAT_CALLBACK_PATH, + callback("ChatOAuth.callback", CHAT_CALLBACK_PATH, "chat-install", "chat"), + ) yield* router.add( "GET", CHAT_IDENTITY_CALLBACK_PATH, - callback("ChatOAuth.identityCallback", "chat-identity", "chat_identity"), + callback( + "ChatOAuth.identityCallback", + CHAT_IDENTITY_CALLBACK_PATH, + "chat-identity", + "chat_identity", + ), ) }), ) From 37bcf0478bee1790f15ac0eb5cfef2a791dc4179 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Fri, 23 Oct 2026 01:02:15 +0200 Subject: [PATCH 5/5] fix(integrations): never cache the callback hand-off, and trace chat callbacks as server spans The redirect into the dashboard carries the provider's code in its fragment, so it is sent with no-store and no-referrer. The chat callbacks' auto server span is suppressed, so their handler span is now server-kind with http.route, like the provider callbacks. --- apps/api/src/routes/chat-integration.http.ts | 30 ++++++++++++++----- .../routes/integrations-callback.http.test.ts | 5 ++++ .../src/routes/integrations-callback.http.ts | 11 ++++++- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/apps/api/src/routes/chat-integration.http.ts b/apps/api/src/routes/chat-integration.http.ts index 31f2dc4d47..429f76dceb 100644 --- a/apps/api/src/routes/chat-integration.http.ts +++ b/apps/api/src/routes/chat-integration.http.ts @@ -1,7 +1,8 @@ -import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/http" +import { HttpRouter, HttpServerRequest } from "effect/http" import { ChatConnectorId, encodeOAuthReturnHash, type OAuthReturnFlow } from "@maple/domain/http" import { Effect, Option, Schema } from "effect" import { Env } from "@maple/backend/platform/Env" +import { forwardToDashboard } from "./integrations-callback.http" /** * The two public callbacks every chat connector redirects to: @@ -30,12 +31,16 @@ export const ChatCallbackRouter = HttpRouter.use((router) => /** Where the browser lands: the integrations page, with the outcome or the return to finish. */ const toDashboard = (search: Record, hash = "") => - HttpServerResponse.redirect( - `${appBaseUrl}/integrations?${new URLSearchParams(search).toString()}${hash}`, - ) + forwardToDashboard(`${appBaseUrl}/integrations?${new URLSearchParams(search).toString()}${hash}`) - const callback = (span: string, flow: OAuthReturnFlow, outcome: string) => - Effect.fn(span)(function* (req: HttpServerRequest.HttpServerRequest) { + // Server-kind like `IntegrationsCallbackRouter`: the auto server span is suppressed for these + // paths (ApiObservabilityLive), so this span is the callback's only trace root. + const callback = (span: string, path: string, flow: OAuthReturnFlow, outcome: string) => + Effect.fn(span, { + kind: "server", + attributes: { "http.route": path, "http.request.method": "GET" }, + })(function* (req: HttpServerRequest.HttpServerRequest) { + yield* Effect.annotateCurrentSpan({ "http.response.status_code": 302 }) const routeParams = yield* HttpRouter.params const connector = decodeConnectorParam(routeParams.connector) const url = Option.liftThrowable(() => new URL(req.url, "http://localhost"))() @@ -59,11 +64,20 @@ export const ChatCallbackRouter = HttpRouter.use((router) => ) }) - yield* router.add("GET", CHAT_CALLBACK_PATH, callback("ChatOAuth.callback", "chat_install", "chat")) + yield* router.add( + "GET", + CHAT_CALLBACK_PATH, + callback("ChatOAuth.callback", CHAT_CALLBACK_PATH, "chat_install", "chat"), + ) yield* router.add( "GET", CHAT_IDENTITY_CALLBACK_PATH, - callback("ChatOAuth.identityCallback", "chat_identity", "chat_identity"), + callback( + "ChatOAuth.identityCallback", + CHAT_IDENTITY_CALLBACK_PATH, + "chat_identity", + "chat_identity", + ), ) }), ) diff --git a/apps/api/src/routes/integrations-callback.http.test.ts b/apps/api/src/routes/integrations-callback.http.test.ts index 34a72b99cd..71e497c410 100644 --- a/apps/api/src/routes/integrations-callback.http.test.ts +++ b/apps/api/src/routes/integrations-callback.http.test.ts @@ -60,6 +60,9 @@ describe("provider callbacks", () => { // Straight to the page that finishes it, not one the dashboard redirects away from. const location = new URL(response.headers.get("location") ?? "") expect(location.origin + location.pathname).toBe("https://app.example.com/sources/new") + // The location carries the provider's code: never cached, never sent on as a Referer. + expect(response.headers.get("cache-control")).toBe("no-store") + expect(response.headers.get("referrer-policy")).toBe("no-referrer") expect(location.searchParams.get("integration")).toBe("cloudflare") const handed = Option.getOrThrow(decodeOAuthReturnHash(location.hash)) expect(handed.flow).toBe("cloudflare") @@ -109,6 +112,8 @@ describe("provider callbacks", () => { expect(response.status).toBe(302) const location = new URL(response.headers.get("location") ?? "") expect(location.searchParams.get("integration")).toBe("chat-slack") + expect(response.headers.get("cache-control")).toBe("no-store") + expect(response.headers.get("referrer-policy")).toBe("no-referrer") const handed = Option.getOrThrow(decodeOAuthReturnHash(location.hash)) expect(handed.flow).toBe("chat_identity") expect(handed.connector).toBe("slack") diff --git a/apps/api/src/routes/integrations-callback.http.ts b/apps/api/src/routes/integrations-callback.http.ts index 8f1d2921b8..20403cad15 100644 --- a/apps/api/src/routes/integrations-callback.http.ts +++ b/apps/api/src/routes/integrations-callback.http.ts @@ -288,6 +288,15 @@ const ROUTES: ReadonlyArray = [ }, ] +/** + * The redirect that hands a callback query to the dashboard. The location carries the provider's + * code, so no cache may keep it and the landing request sends no `Referer`. + */ +export const forwardToDashboard = (location: string) => + HttpServerResponse.redirect(location, { + headers: { "cache-control": "no-store", "referrer-policy": "no-referrer" }, + }) + /** * The provider callbacks. They redeem nothing: this request carries no Maple session, so it * cannot tell whose browser came back. The query goes to the dashboard in a URL fragment, and @@ -343,7 +352,7 @@ export const IntegrationsCallbackRouter = HttpRouter.use((router) => flow: route.flow, query: url.search.replace(/^\?/, ""), }) - return HttpServerResponse.redirect(`${appBaseUrl}${route.page}?${search}${hash}`) + return forwardToDashboard(`${appBaseUrl}${route.page}?${search}${hash}`) }, // Every branch returns a response, so the status is the only signal of a rejection. Effect.tap((response) =>