diff --git a/src/auth/browser-flow.ts b/src/auth/browser-flow.ts index a605d51..fa9675d 100644 --- a/src/auth/browser-flow.ts +++ b/src/auth/browser-flow.ts @@ -25,6 +25,8 @@ export const CALLBACK_PORT = 18765; export const CALLBACK_PATH = "/callback"; const DEFAULT_SCOPES = "read manage trade user"; const DEFAULT_TIMEOUT_MS = 5 * 60 * 1000; +/** Total wall-clock deadline for the /oauth2/token POST including body read. */ +const TOKEN_EXCHANGE_TIMEOUT_MS = 30 * 1000; export interface BrowserFlowOptions { clientId: string; @@ -59,9 +61,10 @@ export async function runBrowserFlow( authUrl.searchParams.set("state", state); authUrl.searchParams.set("scope", scopes); - const { code, receivedState } = await listenForCallback({ + const { code } = await listenForCallback({ port: CALLBACK_PORT, path: CALLBACK_PATH, + expectedState: state, timeoutMs, onReady: () => { opts.onAuthUrl?.(authUrl.toString()); @@ -70,12 +73,6 @@ export async function runBrowserFlow( }, }); - if (receivedState !== state) { - throw new Error( - "OAuth state mismatch — possible CSRF attempt. Aborted without storing a token.", - ); - } - return exchangeCodeForToken({ tokenUrl: `${webUrl}/oauth2/token`, code, @@ -87,13 +84,14 @@ export async function runBrowserFlow( interface CallbackListenerArgs { port: number; path: string; + expectedState: string; timeoutMs: number; onReady: () => void; } function listenForCallback( args: CallbackListenerArgs, -): Promise<{ code: string; receivedState: string }> { +): Promise<{ code: string }> { return new Promise((resolve, reject) => { const server = http.createServer((req, res) => { if (!req.url) { @@ -127,10 +125,24 @@ function listenForCallback( return; } + // Validate state INSIDE the listener so the browser shows the same + // outcome as the CLI. Doing this after sending the success page + // would tell the user (or attacker) "✓ Logged in" while the CLI + // simultaneously aborts — misleading and security-adjacent. + if (receivedState !== args.expectedState) { + res.writeHead(400, { "content-type": "text/html; charset=utf-8" }); + res.end(errorPage("OAuth state mismatch — possible CSRF attempt. Aborted without storing a token.")); + cleanup(); + reject(new Error( + "OAuth state mismatch — possible CSRF attempt. Aborted without storing a token.", + )); + return; + } + res.writeHead(200, { "content-type": "text/html; charset=utf-8" }); res.end(successPage()); cleanup(); - resolve({ code, receivedState }); + resolve({ code }); }); const timer = setTimeout(() => { @@ -186,23 +198,50 @@ async function exchangeCodeForToken( // server accepts this because the app row has an empty client_secret // and bshaffer's `allow_public_clients` is true by default. + // Total deadline covers connect + headers + body read. AbortSignal.timeout + // would only cover until the response arrived; the body could still hang. + const controller = new AbortController(); + const timer = setTimeout( + () => controller.abort(new Error("token exchange timed out")), + TOKEN_EXCHANGE_TIMEOUT_MS, + ); + let res: Response; + let text: string; try { - res = await fetch(args.tokenUrl, { - method: "POST", - headers: { - "content-type": "application/x-www-form-urlencoded", - accept: "application/json", - }, - body: body.toString(), - }); - } catch (err) { - throw new Error( - `Could not reach the token endpoint (${args.tokenUrl}): ${(err as Error).message}`, - ); + try { + res = await fetch(args.tokenUrl, { + method: "POST", + headers: { + "content-type": "application/x-www-form-urlencoded", + accept: "application/json", + }, + body: body.toString(), + signal: controller.signal, + }); + } catch (err) { + if (controller.signal.aborted) { + throw new Error( + `Token exchange to ${args.tokenUrl} timed out after ${TOKEN_EXCHANGE_TIMEOUT_MS / 1000}s.`, + ); + } + throw new Error( + `Could not reach the token endpoint (${args.tokenUrl}): ${(err as Error).message}`, + ); + } + try { + text = await res.text(); + } catch (err) { + if (controller.signal.aborted) { + throw new Error( + `Token exchange to ${args.tokenUrl} timed out reading the response after ${TOKEN_EXCHANGE_TIMEOUT_MS / 1000}s.`, + ); + } + throw err; + } + } finally { + clearTimeout(timer); } - - const text = await res.text(); let parsed: Record | null = null; if (text.length > 0) { try {