From bbc0a47b25804b2b08118a7063e25706d9babf2e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 1 Sep 2026 23:47:55 +0000 Subject: [PATCH 1/2] feat: Login with Coder from a browser on a remote Xum server The Coder OAuth flow only supported a desktop loopback listener, so the Settings UI disabled login when served from a remote Xum server. The authorization redirect now lands on the server's own /auth/coder/callback route in browser mode (like the Gateway/MCP server flows): the service gains a server-hosted callback channel behind the existing flow manager, the HTTP layer builds the redirect URI from the validated public host, and the UI keeps waiting/cancelling through oRPC. --- docs/config/providers.mdx | 8 +- .../Sections/ProvidersSection.test.tsx | 102 +++++-- .../Settings/Sections/ProvidersSection.tsx | 258 +++++++++-------- src/common/constants/coderOAuth.ts | 8 + src/node/orpc/server.test.ts | 114 ++++++++ src/node/orpc/server.ts | 265 ++++++++---------- .../builtInSkillContent.generated.ts | 8 +- src/node/services/coderOauthService.test.ts | 205 ++++++++++++++ src/node/services/coderOauthService.ts | 245 ++++++++++++---- 9 files changed, 885 insertions(+), 328 deletions(-) diff --git a/docs/config/providers.mdx b/docs/config/providers.mdx index 1eedf8219f..4dd0216853 100644 --- a/docs/config/providers.mdx +++ b/docs/config/providers.mdx @@ -282,9 +282,11 @@ custom-named provider instances that Xum cannot discover, declare them by hand i } ``` -Login is available from the desktop app or a locally hosted Xum server. On a remote Xum -server the OAuth callback cannot reach your browser's machine (Coder does not support the -device-authorization grant), so login is disabled there. +Login works from the desktop app and from a browser connected to a Xum server, including a +remote one: in the desktop app the OAuth callback lands on a loopback listener on your +machine, while in the browser Xum registers a callback URL on the server's own origin +(`/auth/coder/callback`) so the redirect reaches the server directly. A remote server must +be reachable over HTTPS for Coder to accept that callback URL. ### Bedrock Authentication diff --git a/src/browser/features/Settings/Sections/ProvidersSection.test.tsx b/src/browser/features/Settings/Sections/ProvidersSection.test.tsx index 25c8894a16..5e43ef2214 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.test.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.test.tsx @@ -641,13 +641,13 @@ describe("ProvidersSection", () => { expect(view.setProviderConfig).toHaveBeenCalledTimes(1); }); - test("startCoderLogin hint launches the Coder OAuth flow against the configured deployment", async () => { - // Regression: the "Settings: Login with Coder" palette command passes a - // one-shot startCoderLogin hint through SettingsContext; ProvidersSection - // must consume it by actually starting the OAuth flow, not just opening - // the Providers list. The hint is injected by spying on useSettings - // (a full-plumbing variant that clicked through a live SettingsProvider - // proved order-fragile in the monolithic CI process). + /** + * Browser-mode Coder login harness: no window.api (browser, not desktop), a + * configured deployment URL, and a fetch double standing in for the Xum + * server's /auth/coder/start route. Returns the recorded start requests and + * the waitForDesktopFlow mock the flow continues on. + */ + function setupBrowserCoderLogin(opts: { hint: boolean }) { const providersConfig = createProvidersConfig(); providersConfig.coder = { apiKeySet: false, @@ -665,16 +665,37 @@ describe("ProvidersSection", () => { data: { flowId: "flow", authorizeUrl: "https://coder.example.com/oauth2/authorize" }, }) ); + const waitForDesktopFlow = mock( + // Never resolves — the user would complete the login in the browser. + (_input: { flowId: string }) => new Promise(() => undefined) + ); (client as unknown as Record).coderOauth = { startDesktopFlow, - // Never resolves — the user would complete the login in the browser. - waitForDesktopFlow: () => new Promise(() => undefined), + waitForDesktopFlow, cancelDesktopFlow: () => Promise.resolve(undefined), }; + const startRequests: URL[] = []; + const fetchDouble = (input: RequestInfo | URL) => { + const url = new URL(typeof input === "string" ? input : (input as URL).toString()); + startRequests.push(url); + return Promise.resolve( + new Response( + JSON.stringify({ + flowId: url.searchParams.get("flowId"), + authorizeUrl: "https://coder.example.com/oauth2/authorize", + }), + { status: 200, headers: { "Content-Type": "application/json" } } + ) + ); + }; + spyOn(globalThis, "fetch").mockImplementation( + Object.assign(fetchDouble, { preconnect: () => undefined }) as typeof fetch + ); + // Stateful hint: true until the section consumes it, so re-renders after // consumption do not re-trigger the login. - let startCoderLoginHint = true; + let startCoderLoginHint = opts.hint; const setProvidersStartCoderLogin = mock((start: boolean) => { startCoderLoginHint = start; }); @@ -685,7 +706,7 @@ describe("ProvidersSection", () => { close: () => undefined, setActiveSection: () => undefined, registerOnClose: () => () => undefined, - providersExpandedProvider: null, + providersExpandedProvider: opts.hint ? null : "coder", setProvidersExpandedProvider: () => undefined, providersStartCoderLogin: startCoderLoginHint, setProvidersStartCoderLogin, @@ -697,20 +718,69 @@ describe("ProvidersSection", () => { setInstructionsProjectPath: () => undefined, })); - render( + const view = render( client}> ); + return { + view, + startDesktopFlow, + waitForDesktopFlow, + startRequests, + setProvidersStartCoderLogin, + }; + } + + test("startCoderLogin hint launches the Coder OAuth flow against the configured deployment", async () => { + // Regression: the "Settings: Login with Coder" palette command passes a + // one-shot startCoderLogin hint through SettingsContext; ProvidersSection + // must consume it by actually starting the OAuth flow, not just opening + // the Providers list. The hint is injected by spying on useSettings + // (a full-plumbing variant that clicked through a live SettingsProvider + // proved order-fragile in the monolithic CI process). + const { startDesktopFlow, waitForDesktopFlow, startRequests, setProvidersStartCoderLogin } = + setupBrowserCoderLogin({ hint: true }); await waitFor(() => { - expect(startDesktopFlow).toHaveBeenCalled(); + expect(startRequests).toHaveLength(1); }); // The hint is one-shot: consumed (cleared) exactly once, one flow started. expect(setProvidersStartCoderLogin).toHaveBeenCalledWith(false); - expect(startDesktopFlow).toHaveBeenCalledTimes(1); - expect(startDesktopFlow.mock.calls[0][0]).toMatchObject({ - deploymentUrl: "https://coder.example.com", + // Browser mode: the server-hosted flow, not the desktop loopback one. + expect(startDesktopFlow).not.toHaveBeenCalled(); + expect(startRequests[0].pathname).toBe("/auth/coder/start"); + expect(startRequests[0].searchParams.get("deploymentUrl")).toBe("https://coder.example.com"); + // The flow then continues on the shared oRPC wait with the started flow ID. + await waitFor(() => { + expect(waitForDesktopFlow).toHaveBeenCalledTimes(1); + }); + const startedFlowId = startRequests[0].searchParams.get("flowId"); + expect(startedFlowId).toBeTruthy(); + expect(waitForDesktopFlow.mock.calls[0][0].flowId).toBe(startedFlowId!); + expect(startRequests).toHaveLength(1); + }); + + test("offers Login with Coder on a remote Xum server and starts the server-hosted flow", async () => { + // Regression: remote browsers used to get an explanation instead of the + // login control ("the OAuth callback must reach this machine"). The + // callback now lands on the server's own origin, so the control renders + // and the start request targets that origin. + (window as unknown as { happyDOM: { setURL: (url: string) => void } }).happyDOM.setURL( + "https://xum.example.com/" + ); + const { view, startRequests } = setupBrowserCoderLogin({ hint: false }); + + const loginButton = await view.findByRole("button", { name: "Login with Coder" }); + expect(view.queryByText(/requires the desktop app/)).toBeNull(); + + fireEvent.click(loginButton); + + await waitFor(() => { + expect(startRequests).toHaveLength(1); }); + expect(startRequests[0].origin).toBe("https://xum.example.com"); + expect(startRequests[0].pathname).toBe("/auth/coder/start"); + await view.findByText("Waiting for authorization..."); }); }); diff --git a/src/browser/features/Settings/Sections/ProvidersSection.tsx b/src/browser/features/Settings/Sections/ProvidersSection.tsx index 05cef9e623..92c9affefc 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.tsx @@ -78,6 +78,8 @@ import { } from "@/common/utils/providers/customProviders"; import type { AddCustomProviderInput, ProviderConfigInfo } from "@/common/orpc/types"; import type { ServiceTier, XAIServiceTier } from "@/common/config/schemas/providersConfig"; +import type { Result } from "@/common/types/result"; +import { CODER_OAUTH_SERVER_START_PATH } from "@/common/constants/coderOAuth"; type MuxGatewayLoginStatus = "idle" | "starting" | "waiting" | "success" | "error"; type CodexOauthFlowStatus = "idle" | "starting" | "waiting" | "error"; @@ -120,6 +122,48 @@ function getServerAuthToken(): string | null { return urlToken?.length ? urlToken : getStoredAuthToken(); } +/** + * Browser/server-mode "Login with Coder": the Xum server registers the flow + * with a redirect URI on its own origin (built server-side from the request + * host, so the OAuth callback reaches the server no matter where the browser + * runs). A raw HTTP route rather than oRPC because only the HTTP request + * carries that public host; the rest of the flow (wait/cancel) stays on oRPC. + */ +async function startCoderServerFlow( + backendBaseUrl: string, + deploymentUrl: string, + flowId: string +): Promise> { + const startUrl = new URL(`${backendBaseUrl}${CODER_OAUTH_SERVER_START_PATH}`); + startUrl.searchParams.set("deploymentUrl", deploymentUrl); + startUrl.searchParams.set("flowId", flowId); + const authToken = getServerAuthToken(); + const res = await fetch(startUrl, { + headers: authToken ? { Authorization: `Bearer ${authToken}` } : undefined, + }); + const contentType = res.headers.get("content-type") ?? ""; + if (!contentType.includes("application/json")) { + const prefix = (await res.text()).trim().slice(0, 80); + return { + success: false, + error: `Unexpected response from ${startUrl.pathname} (expected JSON, got ${ + contentType || "unknown" + }): ${prefix}`, + }; + } + const json = (await res.json()) as { authorizeUrl?: unknown; flowId?: unknown; error?: unknown }; + if (!res.ok) { + return { + success: false, + error: typeof json.error === "string" ? json.error : `HTTP ${res.status}`, + }; + } + if (typeof json.authorizeUrl !== "string" || typeof json.flowId !== "string") { + return { success: false, error: `Invalid response from ${startUrl.pathname}` }; + } + return { success: true, data: { flowId: json.flowId, authorizeUrl: json.authorizeUrl } }; +} + interface FieldConfig { key: string; label: string; @@ -1220,7 +1264,11 @@ export function ProvidersSection() { try { setCoderLoginStatus("starting"); - const startResult = await api.coderOauth.startDesktopFlow({ deploymentUrl, flowId }); + // Desktop: loopback listener on this machine. Browser (local or remote + // server): the redirect lands on the Xum server's own callback route. + const startResult = isDesktop + ? await api.coderOauth.startDesktopFlow({ deploymentUrl, flowId }) + : await startCoderServerFlow(backendBaseUrl, deploymentUrl, flowId); if (attempt !== coderLoginAttemptRef.current) { // cancelCoderLogin already cancelled this flowId; nothing to clean up. @@ -1257,7 +1305,7 @@ export function ProvidersSection() { setCoderLoginStatus("error"); setCoderLoginError(getErrorMessage(err)); } - }, [api, coderFlowId, coderDeploymentUrl, refresh]); + }, [api, coderFlowId, coderDeploymentUrl, refresh, isDesktop, backendBaseUrl]); const disconnectCoderOauth = async () => { const attempt = ++coderLoginAttemptRef.current; @@ -1335,10 +1383,8 @@ export function ProvidersSection() { // One-shot hint from the "Settings: Login with Coder" command: start the // OAuth login as soon as the providers config has loaded (startCoderLogin // reads the configured deployment URL from it — invoking earlier would - // always fail with "Set the deployment URL first."). Remote servers render - // an explanation instead of the login control, so the hint is consumed - // without starting a login there; startCoderLogin handles the remaining - // error cases (missing URL, no API) with inline feedback in the expanded + // always fail with "Set the deployment URL first."). startCoderLogin handles + // the error cases (missing URL, no API) with inline feedback in the expanded // Coder section. useEffect(() => { if (!providersStartCoderLogin || configLoading) { @@ -1346,16 +1392,8 @@ export function ProvidersSection() { } setProvidersStartCoderLogin(false); - if (!isRemoteServer) { - void startCoderLogin(); - } - }, [ - providersStartCoderLogin, - setProvidersStartCoderLogin, - configLoading, - isRemoteServer, - startCoderLogin, - ]); + void startCoderLogin(); + }, [providersStartCoderLogin, setProvidersStartCoderLogin, configLoading, startCoderLogin]); useEffect(() => { if (expandedProvider !== "mux-gateway" || !muxGatewayIsLoggedIn) { @@ -2375,117 +2413,109 @@ export function ProvidersSection() { - {isRemoteServer ? ( -

- Login with Coder requires the desktop app or a locally hosted Xum - server: the OAuth callback must reach this machine, and Coder has no - device-authorization grant for remote logins. -

- ) : ( -
-
+
+
+ + + {coderLoginStatus === "waiting" && coderAuthorizeUrl && ( + )} - {coderLoginStatus === "waiting" && coderAuthorizeUrl && ( - - )} - - {coderLoginInProgress && ( - - )} - - {coderOauthIsConnected && ( - - )} - - {coderOauthCredentialStored && ( - - )} -
- - {coderLoginStatus === "waiting" && ( -

- - Waiting for authorization... -

+ {coderLoginInProgress && ( + )} - {coderLoginStatus === "error" && coderLoginError && ( -

- Login failed: {coderLoginError} -

+ {coderOauthIsConnected && ( + )} - {coderModelRefreshState.kind === "error" && ( -

- Model refresh failed: {coderModelRefreshState.message} -

+ {coderOauthCredentialStored && ( + )}
- )} + + {coderLoginStatus === "waiting" && ( +

+ + Waiting for authorization... +

+ )} + + {coderLoginStatus === "error" && coderLoginError && ( +

+ Login failed: {coderLoginError} +

+ )} + + {coderModelRefreshState.kind === "error" && ( +

+ Model refresh failed: {coderModelRefreshState.message} +

+ )} +
)} diff --git a/src/common/constants/coderOAuth.ts b/src/common/constants/coderOAuth.ts index 21aff7ec31..248db62b2d 100644 --- a/src/common/constants/coderOAuth.ts +++ b/src/common/constants/coderOAuth.ts @@ -19,6 +19,14 @@ export const CODER_OAUTH_DISCOVERY_PATH = "/.well-known/oauth-authorization-serv /** Loopback callback path for the desktop authorization-code flow. */ export const CODER_OAUTH_CALLBACK_PATH = "/callback"; +/** + * Xum server routes for the browser/server-mode flow: the browser cannot reach + * a loopback listener on a remote Xum server, so the authorization redirect + * lands on the server itself (shared between the HTTP layer and the UI). + */ +export const CODER_OAUTH_SERVER_START_PATH = "/auth/coder/start"; +export const CODER_OAUTH_SERVER_CALLBACK_PATH = "/auth/coder/callback"; + /** Client name registered via RFC 7591 dynamic client registration. */ export const CODER_OAUTH_CLIENT_NAME = "Xum"; diff --git a/src/node/orpc/server.test.ts b/src/node/orpc/server.test.ts index 4abcd9f37b..eafa53c9cc 100644 --- a/src/node/orpc/server.test.ts +++ b/src/node/orpc/server.test.ts @@ -1164,6 +1164,9 @@ describe("createOrpcServer", () => { mcpOauthService: { handleServerCallbackAndExchange: handleSuccessfulCallback, } as unknown as ORPCContext["mcpOauthService"], + coderOauthService: { + handleServerCallback: handleSuccessfulCallback, + } as unknown as ORPCContext["coderOauthService"], }; let server: Awaited> | null = null; @@ -1180,6 +1183,14 @@ describe("createOrpcServer", () => { "Content-Type": "application/x-www-form-urlencoded", }; + const coderOauthResponse = await fetch(`${server.baseUrl}/auth/coder/callback`, { + method: "POST", + headers: callbackHeaders, + body: "state=test-state&code=test-code", + }); + expect(coderOauthResponse.status).toBe(200); + expect(coderOauthResponse.headers.get("access-control-allow-origin")).toBeNull(); + const muxGatewayResponse = await fetch(`${server.baseUrl}/auth/mux-gateway/callback`, { method: "POST", headers: callbackHeaders, @@ -1208,6 +1219,109 @@ describe("createOrpcServer", () => { } }); + test("Coder OAuth start route builds a server-hosted redirect URI and the callback renders the flow outcome", async () => { + const startCalls: Array<{ deploymentUrl: string; flowId?: string; redirectUri: string }> = []; + const callbackCalls: Array<{ + state: string | null; + code: string | null; + error: string | null; + }> = []; + const stubContext: Partial = { + coderOauthService: { + startServerFlow: (input: { + deploymentUrl: string; + flowId?: string; + redirectUri: string; + }) => { + startCalls.push(input); + return Promise.resolve({ + success: true, + data: { + flowId: input.flowId ?? "generated", + authorizeUrl: "https://coder.test/authorize", + }, + }); + }, + handleServerCallback: (input: { + state: string | null; + code: string | null; + error: string | null; + }) => { + callbackCalls.push(input); + return Promise.resolve( + input.state === "known-state" + ? { success: true, data: undefined } + : { success: false, error: "Unknown or expired OAuth state" } + ); + }, + } as unknown as ORPCContext["coderOauthService"], + }; + let server: Awaited> | null = null; + + try { + server = await createOrpcServer({ + host: "127.0.0.1", + port: 0, + context: stubContext as ORPCContext, + authToken: "test-token", + }); + + // Start requires auth: the route mints a registration on the deployment. + const unauthenticated = await fetch( + `${server.baseUrl}/auth/coder/start?deploymentUrl=https://coder.test` + ); + expect(unauthenticated.status).toBe(401); + expect(startCalls).toHaveLength(0); + + const missingDeployment = await fetch(`${server.baseUrl}/auth/coder/start`, { + headers: { Authorization: "Bearer test-token" }, + }); + expect(missingDeployment.status).toBe(400); + + // The redirect URI is derived from the request (incl. app-proxy prefix), + // never taken from the client. + const startResponse = await fetch( + `${server.baseUrl}/auth/coder/start?deploymentUrl=https://coder.test&flowId=flow-0123456789abcdef`, + { + headers: { + Authorization: "Bearer test-token", + "X-Forwarded-Prefix": APP_PROXY_BASE_PATH, + }, + } + ); + expect(startResponse.status).toBe(200); + expect(await startResponse.json()).toEqual({ + flowId: "flow-0123456789abcdef", + authorizeUrl: "https://coder.test/authorize", + }); + expect(startCalls).toEqual([ + { + deploymentUrl: "https://coder.test", + flowId: "flow-0123456789abcdef", + redirectUri: `${server.baseUrl}${APP_PROXY_BASE_PATH}/auth/coder/callback`, + }, + ]); + + // Callback: unauthenticated navigation; outcome decides the page. + const okResponse = await fetch( + `${server.baseUrl}/auth/coder/callback?state=known-state&code=test-code` + ); + expect(okResponse.status).toBe(200); + const okHtml = await okResponse.text(); + expect(okHtml).toContain("Login complete"); + expect(okHtml).toContain('"type":"coder-oauth"'); + + const failedResponse = await fetch( + `${server.baseUrl}/auth/coder/callback?state=stale-state&code=test-code` + ); + expect(failedResponse.status).toBe(400); + expect(await failedResponse.text()).toContain("Unknown or expired OAuth state"); + expect(callbackCalls.map((c) => c.state)).toEqual(["known-state", "stale-state"]); + } finally { + await server?.close(); + } + }); + test("brackets IPv6 hosts in returned URLs", async () => { // Minimal context stub - router won't be exercised by this test. const stubContext: Partial = {}; diff --git a/src/node/orpc/server.ts b/src/node/orpc/server.ts index c02e402501..04abfe4eb9 100644 --- a/src/node/orpc/server.ts +++ b/src/node/orpc/server.ts @@ -44,6 +44,11 @@ import { import { attachStreamErrorHandler, isIgnorableStreamError } from "@/node/utils/streamErrors"; import { getErrorMessage } from "@/common/utils/errors"; import { escapeHtml } from "@/node/utils/oauthUtils"; +import type { Result } from "@/common/types/result"; +import { + CODER_OAUTH_SERVER_CALLBACK_PATH, + CODER_OAUTH_SERVER_START_PATH, +} from "@/common/constants/coderOAuth"; import { assert } from "@/common/utils/assert"; import { getAppProxyBasePathFromPathname, stripAppProxyBasePath } from "@/common/appProxyBasePath"; @@ -724,6 +729,7 @@ const OAUTH_CALLBACK_ORIGIN_BYPASS_PATHS = new Set([ "/auth/mux-gateway/callback", "/auth/mux-governor/callback", "/auth/mcp-oauth/callback", + CODER_OAUTH_SERVER_CALLBACK_PATH, ]); function isOAuthCallbackNavigationRequest(req: Pick): boolean { @@ -1056,54 +1062,28 @@ export async function createOrpcServer({ } }); - // --- Xum Gateway OAuth (unauthenticated bootstrap routes) --- - // These are raw Express routes (not oRPC) because the OAuth provider cannot - // send a mux Bearer token during the redirect callback. - app.get("/auth/mux-gateway/start", async (req, res) => { - if (!(await isHttpRequestAuthenticated(req))) { - res.status(401).json({ error: "Invalid or missing auth token/session" }); - return; - } - - const redirectUri = buildPublicAbsoluteUrl(req, "/auth/mux-gateway/callback", allowHttpOrigin); - if (!redirectUri) { - res.status(400).json({ error: "Missing or invalid Host header" }); - return; - } - const { authorizeUrl, state } = context.muxGatewayOauthService.startServerFlow({ redirectUri }); - res.json({ authorizeUrl, state }); - }); - - app.all("/auth/mux-gateway/callback", async (req, res) => { - // Some providers use 307/308 redirects that preserve POST, or response_mode=form_post. - if (req.method !== "GET" && req.method !== "POST") { - res.sendStatus(405); - return; - } - - const state = getStringParamFromQueryOrBody(req, "state"); - const code = getStringParamFromQueryOrBody(req, "code"); - const error = getStringParamFromQueryOrBody(req, "error"); - const errorDescription = getStringParamFromQueryOrBody(req, "error_description") ?? undefined; - - const result = await context.muxGatewayOauthService.handleServerCallbackAndExchange({ - state, - code, - error, - errorDescription, - }); - + /** + * Browser-facing page for a server-hosted OAuth callback: reports the + * outcome, posts it to the opener (the Settings tab), and closes itself on + * success. Shared by every provider whose authorization redirect lands on + * this server rather than on a desktop loopback listener. + */ + function sendOAuthCallbackPage( + req: express.Request, + res: express.Response, + input: { type: string; state: string | null; result: Result } + ): void { const payload = { - type: "mux-gateway-oauth", - state, - ok: result.success, - error: result.success ? null : result.error, + type: input.type, + state: input.state, + ok: input.result.success, + error: input.result.success ? null : input.result.error, }; const payloadJson = escapeJsonForHtmlScript(payload); - const title = result.success ? "Login complete" : "Login failed"; - const description = result.success + const title = input.result.success ? "Login complete" : "Login failed"; + const description = input.result.success ? "You can return to Xum. You may now close this tab." : payload.error ? escapeHtml(payload.error) @@ -1135,7 +1115,7 @@ export async function createOrpcServer({

${title}

${description}

- ${result.success ? '

This tab should close automatically.

' : ""} + ${input.result.success ? '

This tab should close automatically.

' : ""}

Return to Xum

@@ -1193,9 +1173,49 @@ export async function createOrpcServer({ `; - res.status(result.success ? 200 : 400); + res.status(input.result.success ? 200 : 400); res.setHeader("Content-Type", "text/html"); res.send(html); + } + + // --- Xum Gateway OAuth (unauthenticated bootstrap routes) --- + // These are raw Express routes (not oRPC) because the OAuth provider cannot + // send a mux Bearer token during the redirect callback. + app.get("/auth/mux-gateway/start", async (req, res) => { + if (!(await isHttpRequestAuthenticated(req))) { + res.status(401).json({ error: "Invalid or missing auth token/session" }); + return; + } + + const redirectUri = buildPublicAbsoluteUrl(req, "/auth/mux-gateway/callback", allowHttpOrigin); + if (!redirectUri) { + res.status(400).json({ error: "Missing or invalid Host header" }); + return; + } + const { authorizeUrl, state } = context.muxGatewayOauthService.startServerFlow({ redirectUri }); + res.json({ authorizeUrl, state }); + }); + + app.all("/auth/mux-gateway/callback", async (req, res) => { + // Some providers use 307/308 redirects that preserve POST, or response_mode=form_post. + if (req.method !== "GET" && req.method !== "POST") { + res.sendStatus(405); + return; + } + + const state = getStringParamFromQueryOrBody(req, "state"); + const code = getStringParamFromQueryOrBody(req, "code"); + const error = getStringParamFromQueryOrBody(req, "error"); + const errorDescription = getStringParamFromQueryOrBody(req, "error_description") ?? undefined; + + const result = await context.muxGatewayOauthService.handleServerCallbackAndExchange({ + state, + code, + error, + errorDescription, + }); + + sendOAuthCallbackPage(req, res, { type: "mux-gateway-oauth", state, result }); }); // --- Xum Governor OAuth (unauthenticated bootstrap routes) --- @@ -1372,109 +1392,72 @@ export async function createOrpcServer({ errorDescription, }); - const payload = { - type: "mcp-oauth", - state, - ok: result.success, - error: result.success ? null : result.error, - }; - - const payloadJson = escapeJsonForHtmlScript(payload); - - const title = result.success ? "Login complete" : "Login failed"; - const description = result.success - ? "You can return to Xum. You may now close this tab." - : payload.error - ? escapeHtml(payload.error) - : "An unknown error occurred."; - const returnPath = getPublicAppRootPath(req, res); - const returnPathJson = escapeJsonForHtmlScript(returnPath); - const returnPathHref = escapeHtmlAttribute(returnPath); - - const html = ` - - - - - - - ${title} - - - -
-
-
-
xum
-
-
- -
-
-
-

${title}

-

${description}

- ${result.success ? '

This tab should close automatically.

' : ""} -

Return to Xum

-
-
-
-
- - - -`; + const result = await context.coderOauthService.handleServerCallback({ + state, + code, + error, + errorDescription, + }); - res.status(result.success ? 200 : 400); - res.setHeader("Content-Type", "text/html"); - res.send(html); + sendOAuthCallbackPage(req, res, { type: "coder-oauth", state, result }); }); const orpcRouter = existingRouter ?? router(authToken); diff --git a/src/node/services/agentSkills/builtInSkillContent.generated.ts b/src/node/services/agentSkills/builtInSkillContent.generated.ts index 651cbb77f4..2d48a9cd75 100644 --- a/src/node/services/agentSkills/builtInSkillContent.generated.ts +++ b/src/node/services/agentSkills/builtInSkillContent.generated.ts @@ -4559,9 +4559,11 @@ export const BUILTIN_SKILL_FILES: Record> = { "}", "```", "", - "Login is available from the desktop app or a locally hosted Xum server. On a remote Xum", - "server the OAuth callback cannot reach your browser's machine (Coder does not support the", - "device-authorization grant), so login is disabled there.", + "Login works from the desktop app and from a browser connected to a Xum server, including a", + "remote one: in the desktop app the OAuth callback lands on a loopback listener on your", + "machine, while in the browser Xum registers a callback URL on the server's own origin", + "(`/auth/coder/callback`) so the redirect reaches the server directly. A remote server must", + "be reachable over HTTPS for Coder to accept that callback URL.", "", "### Bedrock Authentication", "", diff --git a/src/node/services/coderOauthService.test.ts b/src/node/services/coderOauthService.test.ts index a87b930d7f..f995c3d635 100644 --- a/src/node/services/coderOauthService.test.ts +++ b/src/node/services/coderOauthService.test.ts @@ -4167,6 +4167,211 @@ describe("CoderOauthService", () => { }); }); + // ------------------------------------------------------------------------- + // startServerFlow / handleServerCallback (browser + remote Xum server) + // ------------------------------------------------------------------------- + + describe("startServerFlow", () => { + const SERVER_REDIRECT_URI = "https://xum.example.com/auth/coder/callback"; + + /** Deployment mocks for the server flow; `onTokens` customizes the exchange. */ + function mockServerFlowDeployment(opts: { + registerCalls?: unknown[]; + onTokens?: (init?: RequestInit) => Promise; + onRevoke?: (init?: RequestInit) => void; + }): void { + mockFetch(async (input, init) => { + const url = fetchUrl(input); + if (url === `${DEPLOYMENT_URL}/api/v2/buildinfo`) { + return jsonResponse({ version: "v2.99.0" }); + } + if (url === `${DEPLOYMENT_URL}/.well-known/oauth-authorization-server`) { + return discoveryResponse(); + } + if (url === `${DEPLOYMENT_URL}/oauth2/register`) { + opts.registerCalls?.push(JSON.parse(fetchBodyText(init))); + return jsonResponse({ client_id: "client_srv", client_secret: "secret_srv" }); + } + if (url === `${DEPLOYMENT_URL}/oauth2/tokens`) { + if (opts.onTokens) return opts.onTokens(init); + return jsonResponse({ + access_token: "at_srv", + refresh_token: "rt_srv", + expires_in: 86400, + token_type: "Bearer", + }); + } + if (url === `${DEPLOYMENT_URL}/oauth2/revoke`) { + opts.onRevoke?.(init); + return new Response(null, { status: 200 }); + } + if (url === `${DEPLOYMENT_URL}/api/v2/ai/providers`) { + return aiProvidersResponse(); + } + if (url.startsWith(`${DEPLOYMENT_URL}/api/v2/aibridge/`)) { + return jsonResponse({ data: [{ id: "claude-sonnet-4-5" }] }); + } + // No loopback listener may be involved: any 127.0.0.1 request is a bug. + return new Response(`unexpected url: ${url}`, { status: 500 }); + }); + } + + it("registers the server callback URL instead of a loopback listener and commits via handleServerCallback", async () => { + const registerCalls: unknown[] = []; + let exchangeBody: URLSearchParams | null = null; + mockServerFlowDeployment({ + registerCalls, + onTokens: (init) => { + exchangeBody = new URLSearchParams(fetchBodyText(init)); + return Promise.resolve( + jsonResponse({ + access_token: "at_srv", + refresh_token: "rt_srv", + expires_in: 86400, + token_type: "Bearer", + }) + ); + }, + }); + + const flowId = "server-flow-0123456789abcdef"; + const startResult = await service.startServerFlow({ + deploymentUrl: DEPLOYMENT_URL, + flowId, + redirectUri: SERVER_REDIRECT_URI, + }); + expect(startResult.success).toBe(true); + if (!startResult.success) return; + + const authorize = new URL(startResult.data.authorizeUrl); + expect(startResult.data.flowId).toBe(flowId); + expect(authorize.searchParams.get("state")).toBe(flowId); + // Exact redirect URI matching: the server route URL is what gets + // registered and what the authorize request names. + expect(authorize.searchParams.get("redirect_uri")).toBe(SERVER_REDIRECT_URI); + expect((registerCalls[0] as { redirect_uris: string[] }).redirect_uris).toEqual([ + SERVER_REDIRECT_URI, + ]); + + const waitPromise = service.waitForDesktopFlow(flowId, { timeoutMs: 5000 }); + + // Unknown state is refused before any exchange. + const unknown = await service.handleServerCallback({ + state: "not-a-registered-flow", + code: "code", + error: null, + }); + expect(unknown.success).toBe(false); + + // The callback route's outcome is the committed login. + const callbackResult = await service.handleServerCallback({ + state: flowId, + code: "auth_code_srv", + error: null, + }); + expect(callbackResult).toEqual(Ok(undefined)); + expect(await waitPromise).toEqual(Ok(undefined)); + + expect(exchangeBody!.get("code")).toBe("auth_code_srv"); + expect(exchangeBody!.get("redirect_uri")).toBe(SERVER_REDIRECT_URI); + expect(sha256Base64Url(exchangeBody!.get("code_verifier")!)).toBe( + authorize.searchParams.get("code_challenge")! + ); + + const coderSection = deps.providersConfig.coder as Record; + expect((coderSection.coderOauth as CoderOauthAuth).access).toBe("at_srv"); + expect(coderSection.deploymentUrl).toBe(DEPLOYMENT_URL); + + // An authorization code is single-use; so is its delivery. The finished + // flow is gone from the manager, so a replayed redirect is refused. + const replay = await service.handleServerCallback({ + state: flowId, + code: "auth_code_srv", + error: null, + }); + expect(replay.success).toBe(false); + }); + + it("fails the flow when the redirect carries an OAuth error", async () => { + mockServerFlowDeployment({}); + const startResult = await service.startServerFlow({ + deploymentUrl: DEPLOYMENT_URL, + redirectUri: SERVER_REDIRECT_URI, + }); + expect(startResult.success).toBe(true); + if (!startResult.success) return; + const { flowId } = startResult.data; + const waitPromise = service.waitForDesktopFlow(flowId, { timeoutMs: 5000 }); + + const callbackResult = await service.handleServerCallback({ + state: flowId, + code: null, + error: "access_denied", + errorDescription: "User declined", + }); + expect(callbackResult).toEqual(Err("access_denied: User declined")); + expect(await waitPromise).toEqual(Err("access_denied: User declined")); + expect(deps.providersConfig.coder?.coderOauth).toBeUndefined(); + }); + + it("settles the pending callback request when the flow is cancelled mid-exchange", async () => { + let releaseExchange!: () => void; + const exchangeGate = new Promise((resolve) => (releaseExchange = resolve)); + let exchangeStarted!: () => void; + const exchangeStartedPromise = new Promise((resolve) => (exchangeStarted = resolve)); + let revokeBody: URLSearchParams | null = null; + mockServerFlowDeployment({ + onTokens: async () => { + exchangeStarted(); + await exchangeGate; + return jsonResponse({ + access_token: "at_raced", + refresh_token: "rt_raced", + expires_in: 86400, + token_type: "Bearer", + }); + }, + onRevoke: (init) => { + revokeBody = new URLSearchParams(fetchBodyText(init)); + }, + }); + + const startResult = await service.startServerFlow({ + deploymentUrl: DEPLOYMENT_URL, + redirectUri: SERVER_REDIRECT_URI, + }); + expect(startResult.success).toBe(true); + if (!startResult.success) return; + const { flowId } = startResult.data; + + // The Express route awaits this; Cancel must resolve it rather than + // leave the browser tab hanging until the flow timeout. + const callbackPromise = service.handleServerCallback({ + state: flowId, + code: "auth_code_raced", + error: null, + }); + await exchangeStartedPromise; + await service.cancelDesktopFlow(flowId); + const callbackResult = await callbackPromise; + expect(callbackResult.success).toBe(false); + releaseExchange(); + + await waitUntil(() => revokeBody !== null); + expect(revokeBody!.get("token")).toBe("rt_raced"); + expect(deps.providersConfig.coder?.coderOauth).toBeUndefined(); + }); + + it("rejects a non-http(s) redirect URI without network calls", async () => { + mockFetch(() => Promise.reject(new Error("network must not be reached"))); + const result = await service.startServerFlow({ + deploymentUrl: DEPLOYMENT_URL, + redirectUri: "javascript:alert(1)", + }); + expect(result).toEqual(Err("Invalid OAuth redirect URI")); + }); + }); + describe("cancelDesktopFlow", () => { it("resolves waitForDesktopFlow with cancellation error", async () => { mockFetch((input) => { diff --git a/src/node/services/coderOauthService.ts b/src/node/services/coderOauthService.ts index 52d2d3475a..62f4da6345 100644 --- a/src/node/services/coderOauthService.ts +++ b/src/node/services/coderOauthService.ts @@ -15,6 +15,7 @@ * preserved structurally rather than re-derived. */ import * as crypto from "crypto"; +import type http from "node:http"; import { Duration, Effect, Schema } from "effect"; import type { Result } from "@/common/types/result"; import { Err, Ok } from "@/common/types/result"; @@ -124,6 +125,79 @@ interface CoderLoginCommitResult { persistedAuthRetained?: boolean; } +/** + * Where a login flow's authorization-code redirect lands. Desktop flows own a + * 127.0.0.1 loopback listener (`startLoopbackServer`); browser/server-mode + * flows are redirected to the Xum server's own `/auth/coder/callback` route, + * which hands the code over through a `ServerCallbackChannel`. Registration, + * exchange, and commit are channel-agnostic: only the redirect URI and the + * "answer the browser tab" hooks differ. + */ +interface CoderCallbackChannel { + redirectUri: string; + /** Loopback listener handed to the flow manager for cleanup; null for server-hosted callbacks. */ + server: http.Server | null; + /** Resolves once the redirect carrying this flow's state arrived (or reported an error). */ + result: Promise>; + sendSuccessResponse: () => void; + sendFailureResponse: (error: string) => void; + close: () => Promise; +} + +/** + * Server-mode callback channel. The Xum server's callback route delivers the + * redirect parameters via `deliver`, which resolves the pending callback + * exactly once and returns the login outcome for the route to render. That + * outcome is raced against the flow's own completion so a Cancel or flow + * timeout winning mid-exchange settles the browser request instead of leaving + * it hanging (the loopback equivalent is the patched `server.close()` in + * startLoopbackServer). + */ +class ServerCallbackChannel implements CoderCallbackChannel { + readonly server = null; + private readonly callback = createDeferred>(); + private readonly response = createDeferred>(); + private delivered = false; + readonly result = this.callback.promise; + + constructor( + readonly redirectUri: string, + private readonly flowResult: Promise> + ) {} + + /** Null when a redirect was already delivered for this flow (one-shot, like an auth code). */ + deliver(input: { + code: string | null; + error: string | null; + errorDescription?: string; + }): Promise> | null { + if (this.delivered) { + return null; + } + this.delivered = true; + if (input.error) { + this.callback.resolve( + Err(input.errorDescription ? `${input.error}: ${input.errorDescription}` : input.error) + ); + } else if (!input.code) { + this.callback.resolve(Err("Missing authorization code")); + } else { + this.callback.resolve(Ok({ code: input.code })); + } + return Promise.race([this.response.promise, this.flowResult]); + } + + sendSuccessResponse = (): void => { + this.response.resolve(Ok(undefined)); + }; + + sendFailureResponse = (error: string): void => { + this.response.resolve(Err(error)); + }; + + close = (): Promise => Promise.resolve(); +} + function sha256Base64Url(value: string): string { return crypto.createHash("sha256").update(value).digest().toString("base64url"); } @@ -261,6 +335,11 @@ export class CoderOauthService { // with an insertion timestamp for pruning. private readonly preCancelledFlowIds = new Map(); + // Server-mode flows' callback channels, keyed by flow ID (= OAuth state) so + // the unauthenticated /auth/coder/callback route can hand the redirect to + // its flow. Entries are removed when the flow settles (see registerFlow). + private readonly serverCallbacks = new Map(); + // Bumped by disconnect() so login attempts still in their pre-registration // probes (no flow entry yet — cancelAll cannot see them, and disconnect // does not know their caller-generated IDs) abort at their next checkpoint @@ -509,9 +588,58 @@ export class CoderOauthService { return Effect.uninterruptible(toWireResult(this.launchDesktopFlowEffect(input))); } + /** + * Browser/server-mode login: the same flow as startDesktopFlow, but the + * authorization redirect targets the Xum server's own callback route + * (`redirectUri`, built by the HTTP layer from the validated public host — + * never caller-supplied over RPC) instead of a loopback listener, so the + * user's browser completes the login against a remote Xum server. The flow + * is registered in the shared manager, so waitForDesktopFlow / + * cancelDesktopFlow / disconnect apply unchanged; the callback route hands + * the redirect over via handleServerCallback. + */ + async startServerFlow(input: { + deploymentUrl: string; + flowId?: string; + redirectUri: string; + }): Promise> { + return Effect.runPromise( + Effect.uninterruptible(toWireResult(this.launchDesktopFlowEffect(input))) + ); + } + + /** + * Deliver an authorization redirect received by the server callback route to + * its flow, resolving with the login outcome once the exchange + commit + * settled (or the flow was cancelled/timed out). Unauthenticated callers + * reach this: `state` alone selects the flow, and the delivered code is only + * ever exchanged with this process's PKCE verifier and client secret. + */ + async handleServerCallback(input: { + state: string | null; + code: string | null; + error: string | null; + errorDescription?: string; + }): Promise> { + if (!input.state) { + return Err("Missing OAuth state"); + } + const channel = this.serverCallbacks.get(input.state); + if (!channel || !this.desktopFlows.has(input.state)) { + return Err("Unknown or expired OAuth state"); + } + const outcome = channel.deliver(input); + if (outcome === null) { + return Err("This login was already completed"); + } + return await outcome; + } + private launchDesktopFlowEffect(input: { deploymentUrl: string; flowId?: string; + /** Server-hosted callback URL; a loopback listener is used when omitted. */ + redirectUri?: string; }): Effect.Effect<{ flowId: string; authorizeUrl: string }, CoderOauthError> { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; @@ -519,6 +647,11 @@ export class CoderOauthService { if (input.flowId !== undefined && !/^[A-Za-z0-9_-]{16,128}$/.test(input.flowId)) { return yield* Effect.fail(new CoderOauthError({ reason: "Invalid flow ID" })); } + // The HTTP layer builds this from the validated public host; anything + // else here is a programming error, not user input. + if (input.redirectUri !== undefined && parseEndpointUrl(input.redirectUri) === null) { + return yield* Effect.fail(new CoderOauthError({ reason: "Invalid OAuth redirect URI" })); + } const flowId = input.flowId ?? randomBase64Url(); // Snapshot before the first await: a disconnect() during any of the // probes below must abort this attempt (see disconnectGeneration), and a @@ -585,23 +718,31 @@ export class CoderOauthService { return yield* Effect.fail(new CoderOauthError({ reason: "Login was cancelled" })); } - // Ephemeral port: Coder requires exact redirect URI matching (OAuth 2.1), - // so the freshly bound URI is (re-)registered on the client below. - const loopback = yield* Effect.tryPromise({ - try: () => - startLoopbackServer({ - port: 0, - host: "127.0.0.1", - callbackPath: CODER_OAUTH_CALLBACK_PATH, - validateLoopback: true, - expectedState: flowId, - deferSuccessResponse: true, - }), - catch: (error) => - new CoderOauthError({ - reason: `Failed to start OAuth callback listener: ${getErrorMessage(error)}`, - }), - }); + const resultDeferred = createDeferred>(); + + // Where the authorization redirect lands. Desktop: an ephemeral-port + // loopback listener — Coder requires exact redirect URI matching (OAuth + // 2.1), so the freshly bound URI is (re-)registered on the client below. + // Server mode: the Xum server's own callback route, wired to this flow + // through serverCallbacks right after registration. + const channel: CoderCallbackChannel = + input.redirectUri !== undefined + ? new ServerCallbackChannel(input.redirectUri, resultDeferred.promise) + : yield* Effect.tryPromise({ + try: () => + startLoopbackServer({ + port: 0, + host: "127.0.0.1", + callbackPath: CODER_OAUTH_CALLBACK_PATH, + validateLoopback: true, + expectedState: flowId, + deferSuccessResponse: true, + }), + catch: (error) => + new CoderOauthError({ + reason: `Failed to start OAuth callback listener: ${getErrorMessage(error)}`, + }), + }); // Last pre-registration checkpoint: this check and register() below run // with no intervening await, so a cancel (or disconnect) is either @@ -610,12 +751,10 @@ export class CoderOauthService { self.consumePreCancelled(flowId) || self.disconnectGeneration !== initialDisconnectGeneration ) { - yield* Effect.promise(() => loopback.close()); + yield* Effect.promise(() => channel.close()); return yield* Effect.fail(new CoderOauthError({ reason: "Login was cancelled" })); } - const resultDeferred = createDeferred>(); - // The flow is registered BEFORE the client-registration round-trip: the // loopback listener is already open, and this network await would // otherwise be uncancellable (no flow ID exists yet, so neither Cancel @@ -625,7 +764,7 @@ export class CoderOauthService { // first puts the whole window under the flow timeout, and finishing the // flow (cancel/timeout/shutdown) aborts the in-flight RPC below. self.desktopFlows.register(flowId, { - server: loopback.server, + server: channel.server, resultDeferred, // Keep server-side timeout tied to flow lifetime so abandoned flows // (e.g. callers that never invoke waitForDesktopFlow) still self-clean. @@ -635,6 +774,17 @@ export class CoderOauthService { ); }, DEFAULT_DESKTOP_TIMEOUT_MS), }); + if (channel instanceof ServerCallbackChannel) { + // Same synchronous window as register(): the callback route can find + // the flow from the moment it exists. Detach when the flow settles; + // the identity guard protects a replacement flow reusing this ID. + self.serverCallbacks.set(flowId, channel); + void resultDeferred.promise.then(() => { + if (self.serverCallbacks.get(flowId) === channel) { + self.serverCallbacks.delete(flowId); + } + }); + } // Aborts every network round-trip owned by this flow (client // registration, token exchange) when the flow finishes — Cancel, flow @@ -673,7 +823,7 @@ export class CoderOauthService { self.ensureClientEffect( deploymentUrl, endpoints, - loopback.redirectUri, + channel.redirectUri, flowAbort.signal, ownsStoredClient, (clientId) => { @@ -737,12 +887,12 @@ export class CoderOauthService { const authorizeUrl = buildCoderAuthorizeUrl({ authorizationEndpoint: endpoints.authorizationEndpoint, clientId: client.clientId, - redirectUri: loopback.redirectUri, + redirectUri: channel.redirectUri, state: flowId, codeChallenge, }); - // Background fiber: wait for the loopback callback, exchange code for + // Background fiber: wait for the callback redirect, exchange code for // tokens, then commit the login. Races against resultDeferred (which // resolves on cancel/timeout) so the fiber exits cleanly if the flow is // cancelled. @@ -754,7 +904,7 @@ export class CoderOauthService { tokenEndpoint: endpoints.tokenEndpoint, client, codeVerifier, - loopback, + channel, resultDeferred, flowAbortSignal: flowAbort.signal, }) @@ -768,9 +918,9 @@ export class CoderOauthService { /** * Desktop-flow completion pipeline, forked from `launchDesktopFlowEffect`. - * Races the loopback callback against resultDeferred so that if the flow is + * Races the callback redirect against resultDeferred so that if the flow is * cancelled/timed out externally, this fiber exits cleanly instead of - * dangling on loopback.result. + * dangling on channel.result. */ private desktopCallbackPipeline(args: { flowId: string; @@ -779,7 +929,7 @@ export class CoderOauthService { tokenEndpoint: string; client: CoderOauthClient; codeVerifier: string; - loopback: Awaited>; + channel: CoderCallbackChannel; resultDeferred: ReturnType>>; flowAbortSignal: AbortSignal; }): Effect.Effect { @@ -787,7 +937,7 @@ export class CoderOauthService { const self = this; return Effect.gen(function* () { const callbackResult = yield* Effect.promise(() => - Promise.race([args.loopback.result, args.resultDeferred.promise.then((): null => null)]) + Promise.race([args.channel.result, args.resultDeferred.promise.then((): null => null)]) ); // null means the flow was finished externally (cancel/timeout). @@ -813,13 +963,13 @@ export class CoderOauthService { sessionId: randomBase64Url(16), client: args.client, code: callbackResult.data.code, - redirectUri: args.loopback.redirectUri, + redirectUri: args.channel.redirectUri, codeVerifier: args.codeVerifier, }, args.flowAbortSignal ); if (!tokenResult.success) { - args.loopback.sendFailureResponse(tokenResult.error); + args.channel.sendFailureResponse(tokenResult.error); yield* self.desktopFlows.finishEffect(args.flowId, Err(tokenResult.error)); return; } @@ -829,7 +979,7 @@ export class CoderOauthService { args.flowStartPersistedGeneration, args.deploymentUrl, tokenResult.auth, - args.loopback + args.channel ); // Model discovery runs only after the flow is committed: Cancel is no @@ -1415,10 +1565,7 @@ export class CoderOauthService { flowStartPersistedGeneration: number, deploymentUrl: string, auth: CoderOauthAuth, - loopback: Pick< - Awaited>, - "sendSuccessResponse" | "sendFailureResponse" - > + channel: Pick ): Effect.Effect { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; @@ -1433,7 +1580,7 @@ export class CoderOauthService { flowStartPersistedGeneration, deploymentUrl, auth, - loopback + channel ); // Revocation is best-effort network I/O against a possibly stalled @@ -1445,7 +1592,9 @@ export class CoderOauthService { // the flow timeout wins mid-commit, the flow manager closes the raw // loopback server, whose patched close() ends the deferred callback // response first (see startLoopbackServer) — so the awaited cancel RPC - // never hangs on server.close(). + // never hangs on server.close(). A server-mode callback request is + // settled by the same flow completion (ServerCallbackChannel.deliver + // races it against the response hooks). // Every non-committed outcome revokes the exchanged tokens: a failed // persist (unwritable providers.jsonc, lock timeout) would otherwise @@ -1485,10 +1634,7 @@ export class CoderOauthService { flowStartPersistedGeneration: number, deploymentUrl: string, auth: CoderOauthAuth, - loopback: Pick< - Awaited>, - "sendSuccessResponse" | "sendFailureResponse" - > + channel: Pick ): Effect.Effect { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; @@ -1510,7 +1656,7 @@ export class CoderOauthService { flowStartPersistedGeneration, deploymentUrl, auth, - loopback + channel ), catch: getErrorMessage, }).pipe( @@ -1518,7 +1664,7 @@ export class CoderOauthService { Effect.gen(function* () { const fullMessage = `Failed to commit Coder login: ${message}`; log.warn(`[Coder OAuth] ${fullMessage}`); - loopback.sendFailureResponse(fullMessage); + channel.sendFailureResponse(fullMessage); yield* self.desktopFlows.finishEffect(flowId, Err(fullMessage)); const failed: CoderLoginCommitResult = { outcome: "failed" }; return failed; @@ -1543,10 +1689,7 @@ export class CoderOauthService { flowStartPersistedGeneration: number, deploymentUrl: string, auth: CoderOauthAuth, - loopback: Pick< - Awaited>, - "sendSuccessResponse" | "sendFailureResponse" - > + channel: Pick ): Promise { return await this.fileLeaseManager.withCoderOauthLoginCommitLock(async () => { // The flow may have been cancelled (or timed out) while the exchange @@ -1636,7 +1779,7 @@ export class CoderOauthService { }); this.cachedAuth = null; if (!persistResult.success) { - loopback.sendFailureResponse(persistResult.error); + channel.sendFailureResponse(persistResult.error); await this.desktopFlows.finish(flowId, Err(persistResult.error)); return { outcome: "failed" as const }; } @@ -1647,7 +1790,7 @@ export class CoderOauthService { // still live in this process — finish it so the waiter and browser // callback see the failure instead of hanging until the flow // timeout. - loopback.sendFailureResponse(commitRefusalMessage); + channel.sendFailureResponse(commitRefusalMessage); await this.desktopFlows.finish(flowId, Err(commitRefusalMessage)); return { outcome: "failed" as const }; } @@ -1666,7 +1809,7 @@ export class CoderOauthService { return { outcome: "cancelled" as const, persistedAuthRetained: !cleared }; } - loopback.sendSuccessResponse(); + channel.sendSuccessResponse(); this.windowService?.focusMainWindow(); await this.desktopFlows.finish(flowId, Ok(undefined)); From f89d97db4d2060bb1768d539415143c33d8f5005 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 2 Sep 2026 00:00:42 +0000 Subject: [PATCH 2/2] fix: Coder login on plain-HTTP remote origins Dogfooding the browser flow from a non-localhost host surfaced two insecure-context gaps: crypto.randomUUID is undefined there (the flow ID, which doubles as the OAuth state, now comes from getRandomValues), and navigator.clipboard is undefined too (open the authorization page before the best-effort copy so a throw cannot swallow the navigation). --- .../Settings/Sections/ProvidersSection.tsx | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/browser/features/Settings/Sections/ProvidersSection.tsx b/src/browser/features/Settings/Sections/ProvidersSection.tsx index 92c9affefc..7aaf4d55b9 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.tsx @@ -122,6 +122,18 @@ function getServerAuthToken(): string | null { return urlToken?.length ? urlToken : getStoredAuthToken(); } +/** + * Flow ID for a Coder login. It doubles as the OAuth `state` (CSRF token), so + * it must be unguessable: derived from getRandomValues, which — unlike + * Crypto.randomUUID — is available outside secure contexts too (Xum's browser + * UI can be served from a plain-HTTP remote origin, where randomUUID is + * undefined and would throw before the login even started). + */ +function createCoderLoginFlowId(): string { + const bytes = crypto.getRandomValues(new Uint8Array(16)); + return Array.from(bytes, (byte) => byte.toString(16).padStart(2, "0")).join(""); +} + /** * Browser/server-mode "Login with Coder": the Xum server registers the flow * with a redirect URI on its own origin (built server-side from the request @@ -1259,7 +1271,7 @@ export function ProvidersSection() { // startDesktopFlow can stall on backend network calls, and Cancel must be // able to reach the attempt (the backend pre-cancels IDs it hasn't // registered yet) instead of abandoning only the frontend state. - const flowId = crypto.randomUUID(); + const flowId = createCoderLoginFlowId(); setCoderFlowId(flowId); try { @@ -2434,8 +2446,11 @@ export function ProvidersSection() { size="sm" aria-label="Copy and open Coder authorization page" onClick={() => { - void navigator.clipboard.writeText(coderAuthorizeUrl); + // Open first: navigator.clipboard is undefined outside + // secure contexts (plain-HTTP remote origins), and a throw + // there must not swallow the navigation. window.open(coderAuthorizeUrl, "_blank", "noopener"); + void navigator.clipboard?.writeText(coderAuthorizeUrl); }} className="h-8 px-3 text-xs" >