diff --git a/src/domain/net/proxy.ts b/src/domain/net/proxy.ts new file mode 100644 index 00000000..87dd331c --- /dev/null +++ b/src/domain/net/proxy.ts @@ -0,0 +1,115 @@ +/** + * Reads one answer from Electron's `session.resolveProxy(url)` (issue #481). + * + * Chromium answers with a PAC-style, semicolon-separated list, most-preferred + * entry first: `"PROXY host:port"`, `"HTTPS host:port"`, `"SOCKS5 host:port"`, + * `"SOCKS4 host:port"` or `"DIRECT"`. Only the first entry is ever read: a + * fallback entry exists for when the first is unreachable, which is a retry + * policy this launcher does not implement, so acting on it would silently + * try a proxy the OS itself only offered as a second choice. + * + * `"HTTPS host:port"` names a proxy reached over its own TLS connection. + * `src/ipc/network.ts` can tunnel one (see {@link parseProxyUrl}, used on the + * `HTTPS_PROXY`/`HTTP_PROXY` fallback path), but Chromium's own answer for it + * comes back `unsupported` here regardless: a PAC answer never carries more + * than the host and port, and this function's job is only to say what the OS + * answered, not to guess at a scheme it was never told to trust for this + * particular proxy. `socks` still gets its own shape rather than folding into + * `unsupported` too: a SOCKS entry is not garbage, only unimplemented. + */ +export type ProxyResolution = + | { kind: "direct" } + | { kind: "http"; host: string; port: number } + | { kind: "https"; host: string; port: number } + | { kind: "socks"; host: string; port: number } + | { kind: "unsupported" } + +const PROXY_ENTRY = /^(PROXY|SOCKS4|SOCKS5|SOCKS)\s+([^\s:]+):(\d{1,5})$/i + +export function parseProxyResolution(text: string): ProxyResolution { + const first = text.split(";")[0]?.trim() ?? "" + if (first === "" || /^direct$/i.test(first)) return { kind: "direct" } + + const match = PROXY_ENTRY.exec(first) + if (!match) return { kind: "unsupported" } + + const scheme = match[1] ?? "" + const host = match[2] ?? "" + const port = Number(match[3]) + if (host === "" || !Number.isInteger(port) || port < 1 || port > 65_535) return { kind: "unsupported" } + + return scheme.toUpperCase() === "PROXY" ? { kind: "http", host, port } : { kind: "socks", host, port } +} + +/** + * Reads one `HTTPS_PROXY`/`HTTP_PROXY`-style URL (issue #481's environment fallback, + * `src/ipc/network.ts`'s `environmentProxyResolution`), keeping the scheme the URL + * itself names instead of discarding it: an `http:` or `https:` proxy URL both come + * back tunnelable (`connectThroughProxy` reaches an `https:` one over its own TLS + * connection before ever sending the CONNECT), and a `socks:`/`socks4:`/`socks5:` one + * comes back as the same recognised-but-unimplemented `socks` shape + * {@link parseProxyResolution} gives a PAC SOCKS answer, rather than being silently + * folded into `http` the way string surgery on the hostname alone used to. Anything + * else that still parses as a URL (an unknown scheme) comes back `unsupported`; a + * value that fails to parse as a URL at all comes back `undefined`, the caller's cue + * to fall back to the direct path rather than fail a login over a malformed + * environment variable. + */ +export function parseProxyUrl(raw: string): ProxyResolution | undefined { + let proxyUrl: URL + try { + proxyUrl = new URL(raw) + } catch { + return undefined + } + + const host = proxyUrl.hostname + if (host === "") return undefined + + switch (proxyUrl.protocol) { + case "http:": + return { kind: "http", host, port: Number(proxyUrl.port) || 80 } + case "https:": + return { kind: "https", host, port: Number(proxyUrl.port) || 443 } + case "socks:": + case "socks4:": + case "socks5:": + return { kind: "socks", host, port: Number(proxyUrl.port) || 1080 } + default: + return { kind: "unsupported" } + } +} + +/** + * `NO_PROXY=a.example.com,.b.example.com,*,auth.vintagestory.at:443` (issue #481's + * environment fallback, `src/ipc/network.ts`'s `environmentProxyResolution`): does + * `url` match any entry? An entry is an exact host, a domain suffix (leading dot + * optional), `*` for every host, or any of those with a `:port` suffix that narrows + * the bypass to that port alone. An entry without a port bypasses every port for that + * host, matching curl; an entry with one only bypasses `url`'s own effective port + * (its `URL.port`, defaulted the same way a missing one is defaulted elsewhere in + * this file: 80 for `http:`, 443 for `https:`), so `auth.vintagestory.at:443` matches + * `https://auth.vintagestory.at` even though neither ever spells the port out. + * Comparison is case-insensitive and tolerant of stray whitespace around an entry. + */ +export function matchesNoProxy(url: URL, noProxy: string | undefined): boolean { + if (!noProxy) return false + const targetHost = url.hostname.toLowerCase() + const targetPort = Number(url.port) || (url.protocol === "http:" ? 80 : 443) + + return noProxy + .split(",") + .map((entry) => entry.trim().toLowerCase()) + .filter(Boolean) + .some((entry) => { + if (entry === "*") return true + + const portSeparator = entry.lastIndexOf(":") + const entryPort = portSeparator === -1 ? undefined : Number(entry.slice(portSeparator + 1)) + const hasEntryPort = entryPort !== undefined && Number.isInteger(entryPort) && entryPort > 0 + if (hasEntryPort && entryPort !== targetPort) return false + + const entryHost = (hasEntryPort ? entry.slice(0, portSeparator) : entry).replace(/^\./, "") + return targetHost === entryHost || targetHost.endsWith(`.${entryHost}`) + }) +} diff --git a/src/ipc/handlers/loginFailureReason.ts b/src/ipc/handlers/loginFailureReason.ts index 4bd18042..3da7c6e3 100644 --- a/src/ipc/handlers/loginFailureReason.ts +++ b/src/ipc/handlers/loginFailureReason.ts @@ -66,7 +66,14 @@ export class AccountStorageFailure extends Error { export const NETWORK_MESSAGES = new Map([ ["Network request timed out", "timeout"], ["Network response is too large", "response-too-large"], - ["Network response was aborted", "response-aborted"] + ["Network response was aborted", "response-aborted"], + // The system proxy login now goes through (issue #481): Chromium's own proxy resolution + // carries no credentials for this transport to answer a 407 with, and a SOCKS (or otherwise + // unroutable) proxy answer has no client here. Both are thrown by + // `requestBoundedTextViaNode` before or during the CONNECT tunnel, never after, so neither can + // be confused with a refusal from the auth service itself. + ["Login proxy requires authentication", "proxy-auth-required"], + ["Login proxy is not supported", "proxy-unsupported"] ]) /** The literals `assertSecureStorage` throws, reached through {@link AccountStorageFailure}. */ @@ -247,6 +254,13 @@ export type LoginFailureFamily = "network-unreachable" | "certificate-error" | " * connection, or a round trip that ran out of time. This is also where a * proxied network without the proxy configured lands, so the sentence this * family picks is the one that tells a player to check one. + * + * `proxy-auth-required` and `proxy-unsupported` (issue #481) join it for the + * same reason: both mean the login never reached the service either, only + * for a proxy-shaped cause this launcher cannot resolve on its own (a proxy + * asking for credentials nothing here can supply, or a SOCKS/unrouted answer + * with no client for it), and the family's own sentence already names a + * proxy as something to check. */ const NETWORK_UNREACHABLE_REASONS = new Set([ "timeout", @@ -265,7 +279,9 @@ const NETWORK_UNREACHABLE_REASONS = new Set([ "network-ENETDOWN", "network-EPROTO", "network-ERR_SOCKET_CONNECTION_TIMEOUT", - "network-ERR_STREAM_PREMATURE_CLOSE" + "network-ERR_STREAM_PREMATURE_CLOSE", + "proxy-auth-required", + "proxy-unsupported" ]) /** The TLS codes {@link NETWORK_CODES} lists: a certificate this machine will not accept. */ diff --git a/src/ipc/network.ts b/src/ipc/network.ts index 6f90b0c5..9f44d445 100644 --- a/src/ipc/network.ts +++ b/src/ipc/network.ts @@ -1,10 +1,29 @@ -import { net } from "electron" -import { request as httpRequest } from "node:http" +import { net, session } from "electron" +import { Agent, request as httpRequest } from "node:http" import { request as httpsRequest } from "node:https" +import type { IncomingMessage } from "node:http" +import type { Socket } from "node:net" +import { connect as tlsConnect } from "node:tls" +import { matchesNoProxy, parseProxyResolution, parseProxyUrl } from "@domain/net/proxy" +import type { ProxyResolution } from "@domain/net/proxy" import { MAX_RESPONSE_BYTES } from "@src/ipc/validation" +import { logMessage } from "@src/utils/logManager" const REQUEST_TIMEOUT_MS = 15_000 +/** + * The two literals the login proxy path throws, read back by + * `src/ipc/handlers/loginFailureReason.ts` (issue #481) so the renderer can + * tell a player a proxy is the reason, without either literal ever carrying + * a host: Chromium's `resolveProxy` answer never carries proxy credentials, + * so a 407 is the only answer a login through a proxy can give for one, and + * a SOCKS (or otherwise unrouted) proxy is refused before a socket is ever + * opened, for the same reason `requestBoundedTextViaNode` never got a SOCKS + * client: nothing in this codebase speaks that protocol. + */ +const PROXY_AUTH_REQUIRED_MESSAGE = "Login proxy requires authentication" +const PROXY_UNSUPPORTED_MESSAGE = "Login proxy is not supported" + const DEFAULT_ACCEPT_HEADER = "application/json, text/plain;q=0.9" type BoundedRequestOptions = { @@ -165,12 +184,24 @@ export function requestBoundedBuffer(url: URL, options: BoundedRequestOptions = * `timeoutMs` exists only so tests can trip the timeout branch without a real * 15-second wait; no production caller sets it, so every real call still gets * `REQUEST_TIMEOUT_MS`. + * + * Also honours the system proxy (issue #481), which this transport never + * used to read: `session.defaultSession.resolveProxy` is asked for `url` + * first, the same question Electron's own `net.request` answers itself for + * every other call in this file. An HTTP proxy answer is tunneled through + * with a plain CONNECT (see {@link connectThroughProxy}); `DIRECT` keeps + * this function's byte-for-byte prior behaviour, falling back to + * `HTTPS_PROXY`/`HTTP_PROXY` (`NO_PROXY` respected) only when the session + * itself found nothing to use; a SOCKS answer, an HTTPS-secured proxy answer, + * or anything else this transport cannot route through fails the request up + * front with {@link PROXY_UNSUPPORTED_MESSAGE}, before a socket is ever + * opened. The whole decision, and the tunnel handshake, sit inside the same + * `timeoutMs` wall clock this function already bounded every request with. */ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptions & { timeoutMs?: number } = {}): Promise { const method = options.method ?? "GET" const maxBytes = options.maxBytes ?? MAX_RESPONSE_BYTES const timeoutMs = options.timeoutMs ?? REQUEST_TIMEOUT_MS - const transport = url.protocol === "http:" ? httpRequest : httpsRequest const headers: Record = { Accept: options.accept ?? DEFAULT_ACCEPT_HEADER } if (options.body !== undefined) headers["Content-Type"] = "application/x-www-form-urlencoded" @@ -179,19 +210,39 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio let settled = false let responseBytes = 0 const chunks: Buffer[] = [] + // Whichever request is in flight right now (the CONNECT, or the real one), so the + // one timeout below can abort it without knowing which phase it landed in. + let abortInFlight = (): void => {} + + const finish = (error?: Error): void => { + if (settled) return + settled = true + clearTimeout(timeout) + + if (error) { + reject(error) + } else { + resolve(Buffer.concat(chunks).toString("utf8")) + } + } + + const timeout = setTimeout(() => { + abortInFlight() + finish(new Error("Network request timed out")) + }, timeoutMs) - const request = transport(url, { method, headers }, (response) => { + function onResponse(response: IncomingMessage): void { const contentLengthHeader = response.headers["content-length"] const contentLength = Number(contentLengthHeader) if (Number.isFinite(contentLength) && contentLength > maxBytes) { - request.destroy() + abortInFlight() finish(new Error("Network response is too large")) return } if (response.statusCode === undefined || response.statusCode < 200 || response.statusCode >= 300) { - request.destroy() + abortInFlight() finish(new Error(`Network request failed with status ${response.statusCode ?? "unknown"}`)) return } @@ -201,7 +252,7 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio responseBytes += chunkBuffer.length if (responseBytes > maxBytes) { - request.destroy() + abortInFlight() finish(new Error("Network response is too large")) return } @@ -211,31 +262,172 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio response.on("end", () => finish()) response.on("aborted", () => finish(new Error("Network response was aborted"))) response.on("error", (error) => finish(error)) - }) - - const timeout = setTimeout(() => { - request.destroy() - finish(new Error("Network request timed out")) - }, timeoutMs) - - const finish = (error?: Error): void => { - if (settled) return - settled = true - clearTimeout(timeout) + } - if (error) { - reject(error) - } else { - resolve(Buffer.concat(chunks).toString("utf8")) + // A tunneled request is always sent with node:http's own `request`, `agent` and all, + // never `https.request`: the agent already hands back a socket doing TLS on its own + // for an `https:` target (see connectThroughProxy), and a second TLS layer on top is + // not what `https.request` would give it anyway. The URL itself is never passed + // through in that case either: a `URL` carries its own `protocol`, and `http.request` + // refuses one that disagrees with the agent's ("https:" against `TunnelAgent`'s + // inherited "http:"), so the pieces `http.request` actually needs are read off it by + // hand instead. The direct branch is untouched: same call as before this issue. + function send(agent?: Agent): void { + const request = agent + ? httpRequest({ hostname: url.hostname, port: url.port || (url.protocol === "http:" ? 80 : 443), path: `${url.pathname}${url.search}`, method, headers, agent }, onResponse) + : (url.protocol === "http:" ? httpRequest : httpsRequest)(url, { method, headers }, onResponse) + + abortInFlight = (): void => { + request.destroy() } + request.on("error", (error) => finish(error)) + if (options.body !== undefined) request.end(options.body) + else request.end() } - request.on("error", (error) => finish(error)) + decideProxy(url) + .then((decision) => { + if (settled) return + if (decision.kind === "blocked") return finish(decision.error) + if (decision.kind === "direct") return send() + + return connectThroughProxy(decision.proxy, url, (abort) => { + abortInFlight = abort + }).then((agent) => { + if (!settled) send(agent) + }) + }) + .catch((error: unknown) => finish(error instanceof Error ? error : new Error(String(error)))) + }) +} - if (options.body !== undefined) { - request.end(options.body) - } else { - request.end() - } +/** What {@link requestBoundedTextViaNode} does once it knows the proxy for `url`. `secure` marks a proxy reached over its own TLS connection (an `https:` proxy URL), as opposed to a plain CONNECT. */ +type ProxyDecision = { kind: "direct" } | { kind: "tunnel"; proxy: { host: string; port: number; secure: boolean } } | { kind: "blocked"; error: Error } + +/** + * Asks Electron's default session how `url` should be reached, the same + * question `net.request` answers for itself, and turns the answer into what + * {@link requestBoundedTextViaNode} does next. + * + * A session that fails to answer (no window ever created one, or some + * Electron-internal error) is treated as `DIRECT`: a proxy question this + * transport cannot even ask is not a reason to refuse a login that used to + * work before this issue existed. + * + * Every branch logs exactly one fixed token and nothing else, never the + * host `url` names or the proxy resolved for it (issue #481): `proxy-used` + * and `proxy-direct` here; a blocked resolution logs nothing of its own; its + * `proxy-unsupported` (or a CONNECT 407's `proxy-auth-required`) reaches the + * log the same way every other login failure does, through the reason + * `src/ipc/handlers/loginFailureReason.ts` classifies the thrown error into. + */ +async function decideProxy(url: URL): Promise { + const pacAnswer = await session.defaultSession.resolveProxy(url.toString()).catch(() => "DIRECT") + const resolution = parseProxyResolution(pacAnswer) + const effective = resolution.kind === "direct" ? (environmentProxyResolution(url) ?? resolution) : resolution + + if (effective.kind === "direct") { + logMessage("debug", "[back] [ipc] [network.ts] [PROXY] proxy-direct") + return { kind: "direct" } + } + if (effective.kind === "http" || effective.kind === "https") { + logMessage("debug", "[back] [ipc] [network.ts] [PROXY] proxy-used") + return { kind: "tunnel", proxy: { host: effective.host, port: effective.port, secure: effective.kind === "https" } } + } + // socks, or a shape this transport does not recognise: neither has a client here. + return { kind: "blocked", error: new Error(PROXY_UNSUPPORTED_MESSAGE) } +} + +/** + * `HTTPS_PROXY`/`HTTP_PROXY`, read only when the session itself answered + * `DIRECT`: Node's `http(s).request` never consults either variable on its + * own, unlike `net.request`, which is why this transport needed one at all. + * `NO_PROXY` is checked first and wins outright, matching curl and every + * other tool that honours the trio; the match itself, port included, is + * {@link matchesNoProxy}'s job. Parsing the proxy URL, scheme included, is + * {@link parseProxyUrl}'s job (both `src/domain/net/proxy.ts`): this function + * only reads the environment and wires the two together. + */ +function environmentProxyResolution(url: URL): ProxyResolution | undefined { + if (matchesNoProxy(url, process.env.NO_PROXY ?? process.env.no_proxy)) return undefined + + const raw = process.env.HTTPS_PROXY ?? process.env.https_proxy ?? process.env.HTTP_PROXY ?? process.env.http_proxy + if (!raw) return undefined + + return parseProxyUrl(raw) +} + +/** + * An `http.Agent` that hands back one already-established socket instead of + * ever opening a connection of its own: the tunnel {@link connectThroughProxy} + * just built. `http.request` (never `https.request`, even for an `https:` + * target) is what this agent is meant for: once the socket it returns is + * already doing TLS (see {@link connectThroughProxy}), the request built on + * top of it only ever writes plaintext HTTP/1.1 onto a stream, and that + * stream encrypts on the way out on its own. + */ +class TunnelAgent extends Agent { + private readonly socket: Socket + + constructor(socket: Socket) { + super({ keepAlive: false }) + this.socket = socket + } + + override createConnection(): Socket { + return this.socket + } +} + +/** + * Opens the login request's actual connection through an HTTP or HTTPS proxy + * with a CONNECT tunnel: `proxy.secure` picks `node:https`'s `request` over + * `node:http`'s to send the CONNECT itself over its own TLS connection to the + * proxy (an `https:` proxy URL, issue #481's `HTTPS_PROXY` scheme fix), and + * once the proxy answers 200 the raw socket it hands back either is the + * connection (a plain-`http` test target) or gets wrapped in a second, separate + * TLS layer to the real origin (an `https:` target, every real login). No new + * dependency: `node:tls`'s `connect({ socket })` upgrading an already-open + * socket, and the tiny {@link TunnelAgent} above, are both standard library. + * + * `onAbort` is handed the one thing worth cancelling at each point in time, + * so `requestBoundedTextViaNode`'s single timeout can reach whichever phase + * is actually in flight without knowing which one that is. + * + * A 407 is the only status this maps by itself, to {@link PROXY_AUTH_REQUIRED_MESSAGE}: + * Chromium's proxy resolution never carries credentials for this transport to send back, + * so an authenticated proxy cannot be satisfied and the player is told plainly rather than + * left on a generic connection failure. Known limit, not a bug: there is no prompt for a + * proxy password anywhere in this launcher. + */ +function connectThroughProxy(proxy: { host: string; port: number; secure: boolean }, url: URL, onAbort: (abort: () => void) => void): Promise { + const targetPort = Number(url.port) || (url.protocol === "http:" ? 80 : 443) + const sendConnect = proxy.secure ? httpsRequest : httpRequest + + return new Promise((resolve, reject) => { + const connectRequest = sendConnect({ + host: proxy.host, + port: proxy.port, + method: "CONNECT", + path: `${url.hostname}:${targetPort}`, + headers: { Host: `${url.hostname}:${targetPort}` } + }) + + onAbort(() => connectRequest.destroy()) + connectRequest.on("error", reject) + connectRequest.on("connect", (response, socket) => { + if (response.statusCode === 200) { + resolve(socket) + return + } + + socket.destroy() + reject(response.statusCode === 407 ? new Error(PROXY_AUTH_REQUIRED_MESSAGE) : new Error(`Login proxy CONNECT failed with status ${response.statusCode ?? "unknown"}`)) + }) + connectRequest.end() + }).then((rawSocket) => { + const tunneledSocket = url.protocol === "http:" ? rawSocket : tlsConnect({ socket: rawSocket, servername: url.hostname }) + onAbort(() => tunneledSocket.destroy()) + return new TunnelAgent(tunneledSocket) }) } diff --git a/tests/domain/net/proxy.test.ts b/tests/domain/net/proxy.test.ts new file mode 100644 index 00000000..d1bdce30 --- /dev/null +++ b/tests/domain/net/proxy.test.ts @@ -0,0 +1,84 @@ +import assert from "node:assert/strict" +import { describe, it } from "vitest" + +import { matchesNoProxy, parseProxyResolution } from "@domain/net/proxy" + +describe("parseProxyResolution reads one session.resolveProxy answer (#481)", () => { + it("reads DIRECT, case and whitespace insensitive", () => { + assert.deepEqual(parseProxyResolution("DIRECT"), { kind: "direct" }) + assert.deepEqual(parseProxyResolution("direct"), { kind: "direct" }) + assert.deepEqual(parseProxyResolution(" DIRECT "), { kind: "direct" }) + }) + + it("reads a PROXY entry as an http proxy", () => { + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:8080"), { kind: "http", host: "10.0.0.1", port: 8080 }) + assert.deepEqual(parseProxyResolution("proxy office-proxy.local:3128"), { kind: "http", host: "office-proxy.local", port: 3128 }) + }) + + it("reads a SOCKS4 or SOCKS5 entry as socks, unimplemented but recognised", () => { + assert.deepEqual(parseProxyResolution("SOCKS5 10.0.0.1:1080"), { kind: "socks", host: "10.0.0.1", port: 1080 }) + assert.deepEqual(parseProxyResolution("SOCKS4 10.0.0.1:1080"), { kind: "socks", host: "10.0.0.1", port: 1080 }) + assert.deepEqual(parseProxyResolution("SOCKS 10.0.0.1:1080"), { kind: "socks", host: "10.0.0.1", port: 1080 }) + }) + + it("only reads the first entry, tolerant of a fallback list and stray whitespace", () => { + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:8080; DIRECT"), { kind: "http", host: "10.0.0.1", port: 8080 }) + assert.deepEqual(parseProxyResolution(" PROXY 10.0.0.1:8080 ;SOCKS5 10.0.0.2:1080"), { kind: "http", host: "10.0.0.1", port: 8080 }) + assert.deepEqual(parseProxyResolution("DIRECT; PROXY 10.0.0.1:8080"), { kind: "direct" }) + }) + + it("cannot tunnel an HTTPS-secured proxy, so it comes back unsupported like a truly unknown scheme", () => { + assert.deepEqual(parseProxyResolution("HTTPS 10.0.0.1:443"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("QUIC 10.0.0.1:443"), { kind: "unsupported" }) + }) + + it("treats garbage and out-of-range ports as unsupported rather than throwing", () => { + assert.deepEqual(parseProxyResolution("not a proxy answer"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:99999"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:0"), { kind: "unsupported" }) + }) + + it("treats an empty answer as direct rather than throwing", () => { + assert.deepEqual(parseProxyResolution(""), { kind: "direct" }) + assert.deepEqual(parseProxyResolution(" "), { kind: "direct" }) + }) +}) + +describe("matchesNoProxy reads one NO_PROXY entry list (#481)", () => { + it("matches a bare host by exact name", () => { + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), "auth.vintagestory.at"), true) + assert.equal(matchesNoProxy(new URL("https://other.example.com"), "auth.vintagestory.at"), false) + }) + + it("matches a host with a port only when the target's own effective port agrees", () => { + // https://auth.vintagestory.at never spells out :443, but that is its effective port. + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), "auth.vintagestory.at:443"), true) + assert.equal(matchesNoProxy(new URL("http://auth.vintagestory.at"), "auth.vintagestory.at:443"), false) + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at:8443"), "auth.vintagestory.at:443"), false) + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at:443"), "auth.vintagestory.at:443"), true) + }) + + it("matches a domain suffix, leading dot optional", () => { + assert.equal(matchesNoProxy(new URL("https://api.vintagestory.at"), ".vintagestory.at"), true) + assert.equal(matchesNoProxy(new URL("https://api.vintagestory.at"), "vintagestory.at"), true) + assert.equal(matchesNoProxy(new URL("https://vintagestory.at.evil.com"), "vintagestory.at"), false) + }) + + it("matches every host on the * entry", () => { + assert.equal(matchesNoProxy(new URL("https://anything.example.com"), "*"), true) + }) + + it("tolerates spaces around an entry in a comma-separated list", () => { + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), "other.example.com, auth.vintagestory.at ,third.example.com"), true) + }) + + it("matches regardless of case", () => { + assert.equal(matchesNoProxy(new URL("https://Auth.VintageStory.at"), "AUTH.vintagestory.AT"), true) + }) + + it("returns false for an empty or unset NO_PROXY", () => { + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), undefined), false) + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), ""), false) + }) +}) diff --git a/tests/ipc/helpers/electronMock.ts b/tests/ipc/helpers/electronMock.ts index 0d4c3ad5..57f5a5f0 100644 --- a/tests/ipc/helpers/electronMock.ts +++ b/tests/ipc/helpers/electronMock.ts @@ -23,7 +23,10 @@ export { createTrustedEvent, createUntrustedEvent } from "./trustedEvent" */ const DEFAULT_APP_VERSION = "0.0.0-test" -const state = { userDataPath: "", appVersion: DEFAULT_APP_VERSION } +// "DIRECT" so `requestBoundedTextViaNode` (src/ipc/network.ts, issue #481) keeps behaving +// exactly as it did before it started asking, in every test that never calls +// `setElectronProxyResolution` itself. +const state = { userDataPath: "", appVersion: DEFAULT_APP_VERSION, proxyResolution: "DIRECT" } const namedPaths: Record = {} /** @@ -68,6 +71,16 @@ export function setElectronPath(name: "appData" | "home" | "appRoot", path: stri namedPaths[name] = path } +/** + * Points `session.defaultSession.resolveProxy(url)` at `answer`, the PAC-style string Electron's + * real session answers with (`"DIRECT"`, `"PROXY host:port"`, `"SOCKS5 host:port"`, ...). Tests for + * `requestBoundedTextViaNode`'s proxy support (issue #481) set this in place of a real Chromium + * session, which nothing under `tests/` runs. + */ +export function setElectronProxyResolution(answer: string): void { + state.proxyResolution = answer +} + /** * Stands in for the `electron` module so a main-process adapter can be imported * under plain Node instead of a running Electron process. @@ -137,5 +150,9 @@ vi.mock("electron", () => { const dialog = { showSaveDialog: vi.fn(), showOpenDialog: vi.fn() } const shell = { showItemInFolder: vi.fn(), openPath: vi.fn(), openExternal: vi.fn() } - return { app, ipcMain, dialog, shell } + // `resolveProxy` is the one `session` member `requestBoundedTextViaNode` reads (issue #481); + // nothing else on a real `Session` is touched by anything under test. + const session = { defaultSession: { resolveProxy: async (): Promise => state.proxyResolution } } + + return { app, ipcMain, dialog, shell, session } }) diff --git a/tests/ipc/helpers/tlsFixtures.ts b/tests/ipc/helpers/tlsFixtures.ts new file mode 100644 index 00000000..ffad88b9 --- /dev/null +++ b/tests/ipc/helpers/tlsFixtures.ts @@ -0,0 +1,155 @@ +import { vi } from "vitest" +import { generateKeyPairSync, sign as cryptoSign } from "node:crypto" + +/** + * Everything the proxy tests need to exercise real TLS: a self-signed certificate + * generated fresh at test time (issue #481's HTTPS-through-CONNECT coverage), and a + * way to make `src/ipc/network.ts` trust it for the one test that generated it, + * without touching that module's own call sites or the process-wide certificate + * store. `NODE_EXTRA_CA_CERTS` cannot do the latter: Node only reads it once, at + * process start, before a single line of test code has run, so setting it here would + * either do nothing or leak into every other test file sharing the process. + * `node:tls`'s `connect` and `node:https`'s `request` are what network.ts's tunnel + * wraps a socket with and what it CONNECTs to a secure proxy through, so mocking + * exactly those two, everything else forwarded untouched, gets a real handshake and a + * real certificate check against a CA only this file's tests ever set. + */ + +const trust = vi.hoisted(() => ({ ca: undefined as string | undefined })) + +vi.mock("node:tls", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + connect: (options: Record, ...rest: unknown[]): ReturnType => { + const merged = trust.ca === undefined ? options : { ...options, ca: trust.ca } + return (actual.connect as (...args: unknown[]) => ReturnType)(merged, ...rest) + } + } +}) + +vi.mock("node:https", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + request: (options: Record, ...rest: unknown[]): ReturnType => { + const merged = trust.ca === undefined ? options : { ...options, ca: trust.ca } + return (actual.request as (...args: unknown[]) => ReturnType)(merged, ...rest) + } + } +}) + +/** Trusts `pem` for every `node:tls`/`node:https` call this file mocks, until cleared. Call with no argument (an `afterEach` should) to stop trusting it. */ +export function setTrustedCa(pem?: string): void { + trust.ca = pem +} + +/** + * Builds a minimal DER encoder for exactly the ASN.1 shapes an X.509 certificate + * needs: no library, because Node ships no certificate-issuing API of its own, only + * `X509Certificate` to read one back. + */ +function derLength(n: number): Buffer { + if (n < 0x80) return Buffer.from([n]) + const bytes: number[] = [] + let v = n + while (v > 0) { + bytes.unshift(v & 0xff) + v = Math.floor(v / 256) + } + return Buffer.from([0x80 | bytes.length, ...bytes]) +} +function tlv(tag: number, content: Buffer): Buffer { + return Buffer.concat([Buffer.from([tag]), derLength(content.length), content]) +} +const seq = (...parts: Buffer[]): Buffer => tlv(0x30, Buffer.concat(parts)) +const set = (...parts: Buffer[]): Buffer => tlv(0x31, Buffer.concat(parts)) +function int(n: number): Buffer { + const bytes: number[] = [] + let v = n + if (v === 0) bytes.push(0) + while (v > 0) { + bytes.unshift(v & 0xff) + v = Math.floor(v / 256) + } + if ((bytes[0] ?? 0) & 0x80) bytes.unshift(0) + return tlv(0x02, Buffer.from(bytes)) +} +function oid(dotted: string): Buffer { + const parts = dotted.split(".").map(Number) + const bytes = [(parts[0] ?? 0) * 40 + (parts[1] ?? 0)] + for (const p of parts.slice(2)) { + if (p < 128) { + bytes.push(p) + continue + } + const chunk = [p & 0x7f] + let v = p >> 7 + while (v > 0) { + chunk.unshift((v & 0x7f) | 0x80) + v >>= 7 + } + bytes.push(...chunk) + } + return tlv(0x06, Buffer.from(bytes)) +} +const utf8String = (s: string): Buffer => tlv(0x0c, Buffer.from(s, "utf8")) +function utcTime(date: Date): Buffer { + const p2 = (n: number): string => String(n).padStart(2, "0") + const text = `${p2(date.getUTCFullYear() % 100)}${p2(date.getUTCMonth() + 1)}${p2(date.getUTCDate())}${p2(date.getUTCHours())}${p2(date.getUTCMinutes())}${p2(date.getUTCSeconds())}Z` + return tlv(0x17, Buffer.from(text, "ascii")) +} +const bitString = (content: Buffer): Buffer => tlv(0x03, Buffer.concat([Buffer.from([0]), content])) +const explicit = (n: number, content: Buffer): Buffer => tlv(0xa0 | n, content) +const octetString = (buf: Buffer): Buffer => tlv(0x04, buf) +const ipAddress = (ip: string): Buffer => tlv(0x87, Buffer.from(ip.split(".").map(Number))) +const dnsName = (value: string): Buffer => tlv(0x82, Buffer.from(value, "ascii")) + +const CN_OID = oid("2.5.4.3") +const name = (cn: string): Buffer => seq(set(seq(CN_OID, utf8String(cn)))) + +const ECDSA_SHA256_OID = oid("1.2.840.10045.4.3.2") +const algEcdsaSha256 = (): Buffer => seq(ECDSA_SHA256_OID) + +const SAN_OID = oid("2.5.29.17") +const extensionSAN = (altNames: Buffer[]): Buffer => seq(SAN_OID, octetString(seq(...altNames))) + +function toPem(der: Buffer, label: string): string { + const lines = der.toString("base64").match(/.{1,64}/g) ?? [] + return `-----BEGIN ${label}-----\n${lines.join("\n")}\n-----END ${label}-----\n` +} + +/** + * A fresh self-signed certificate for `127.0.0.1`/`localhost`, valid for the next day, + * built from an EC key pair signed with its own private key: subject and issuer are + * the same name, which is what makes a certificate self-signed rather than merely + * unsigned-by-anyone-trusted, and the only way `openssl verify -CAfile` (or Node's own + * chain check, handed this certificate as its `ca`) accepts it as its own root. + */ +export function createSelfSignedCert(): { cert: string; key: string } { + const { publicKey, privateKey } = generateKeyPairSync("ec", { namedCurve: "prime256v1" }) + const publicKeyDer = publicKey.export({ type: "spki", format: "der" }) + + const notBefore = new Date(Date.now() - 60_000) + const notAfter = new Date(Date.now() + 24 * 60 * 60 * 1000) + const serial = Math.floor(Math.random() * 1e9) + 1 + const subjectAndIssuer = name("127.0.0.1") + + const tbsCertificate = seq( + explicit(0, int(2)), + int(serial), + algEcdsaSha256(), + subjectAndIssuer, + seq(utcTime(notBefore), utcTime(notAfter)), + subjectAndIssuer, + publicKeyDer, + explicit(3, seq(extensionSAN([ipAddress("127.0.0.1"), dnsName("localhost")]))) + ) + const signature = cryptoSign("sha256", tbsCertificate, { key: privateKey }) + const certificate = seq(tbsCertificate, algEcdsaSha256(), bitString(signature)) + + return { + cert: toPem(certificate, "CERTIFICATE"), + key: privateKey.export({ type: "pkcs8", format: "pem" }) as string + } +} diff --git a/tests/ipc/loginFailureReason.test.ts b/tests/ipc/loginFailureReason.test.ts index d30a8980..a605e80b 100644 --- a/tests/ipc/loginFailureReason.test.ts +++ b/tests/ipc/loginFailureReason.test.ts @@ -27,7 +27,9 @@ describe("loginFailureReason names what went wrong", () => { for (const [message, expected] of [ ["Network request timed out", "timeout"], ["Network response is too large", "response-too-large"], - ["Network response was aborted", "response-aborted"] + ["Network response was aborted", "response-aborted"], + ["Login proxy requires authentication", "proxy-auth-required"], + ["Login proxy is not supported", "proxy-unsupported"] ] as const) { it(`reads "${message}" as ${expected}`, () => { assert.equal(loginFailureReason(new Error(message)), expected) @@ -228,6 +230,10 @@ describe("loginFailureFamily places every token loginFailureReason can emit", () "network-EPROTO": "network-unreachable", "network-ERR_SOCKET_CONNECTION_TIMEOUT": "network-unreachable", "network-ERR_STREAM_PREMATURE_CLOSE": "network-unreachable", + // A proxy-shaped cause the login transport cannot resolve on its own (#481): same family, + // since the request never reached the service either way. + "proxy-auth-required": "network-unreachable", + "proxy-unsupported": "network-unreachable", // A certificate this machine would not accept. "network-CERT_HAS_EXPIRED": "certificate-error", "network-CERT_NOT_YET_VALID": "certificate-error", diff --git a/tests/ipc/networkProxy.test.ts b/tests/ipc/networkProxy.test.ts new file mode 100644 index 00000000..2552974b --- /dev/null +++ b/tests/ipc/networkProxy.test.ts @@ -0,0 +1,344 @@ +import assert from "node:assert/strict" +import { createServer as createHttpServer } from "node:http" +import type { IncomingMessage, Server } from "node:http" +import { createServer as createHttpsServer } from "node:https" +import type { Server as HttpsServer } from "node:https" +import { connect as netConnect } from "node:net" +import type { Socket } from "node:net" +import { afterEach, describe, it } from "vitest" + +import "./helpers/electronMock" +import { setElectronProxyResolution } from "./helpers/electronMock" +import "./helpers/tlsFixtures" +import { createSelfSignedCert, setTrustedCa } from "./helpers/tlsFixtures" + +import { requestBoundedTextViaNode } from "@src/ipc/network" + +/** + * `requestBoundedTextViaNode`'s system-proxy support (issue #481): the login + * transport now asks Electron's session for the proxy before opening a + * socket, tunnels an HTTP proxy answer with a plain CONNECT, and refuses a + * proxy shape it cannot route through before ever touching the network. + * + * Most servers here (origin and proxy alike) are plain, unencrypted + * `node:http`/`node:net` servers on 127.0.0.1: the tunnel is exercised with an + * `http:` target URL, the one case `requestBoundedTextViaNode` itself already + * skips the TLS wrap for (see its `url.protocol === "http:"` check), which + * proves the CONNECT-and-relay mechanics without a certificate anywhere in + * most of the tests. A handful reach for a self-signed certificate from + * `./helpers/tlsFixtures` instead, where the point under test is a real TLS + * connection: an `https:` proxy URL, or an `https:` target reached through + * the tunnel, both real logins hit and neither of which a plain server can + * stand in for. + */ + +const openSockets = new Set() +let origin: Server | undefined +let proxy: Server | undefined +let secureProxy: HttpsServer | undefined +let secureOrigin: HttpsServer | undefined + +afterEach(async () => { + for (const socket of openSockets) socket.destroy() + openSockets.clear() + setElectronProxyResolution("DIRECT") + setTrustedCa(undefined) + delete process.env.HTTPS_PROXY + delete process.env.HTTP_PROXY + delete process.env.NO_PROXY + + for (const server of [origin, proxy, secureProxy, secureOrigin]) { + if (!server) continue + await new Promise((resolve) => server.close(() => resolve())) + } + origin = undefined + proxy = undefined + secureProxy = undefined + secureOrigin = undefined +}) + +function trackSockets(server: Server | HttpsServer): void { + server.on("connection", (socket) => { + openSockets.add(socket) + socket.on("close", () => openSockets.delete(socket)) + }) +} + +function startOrigin(handler: (req: IncomingMessage, res: import("node:http").ServerResponse) => void): Promise { + return new Promise((resolve) => { + origin = createHttpServer(handler) + trackSockets(origin) + origin.listen(0, "127.0.0.1", () => { + const address = origin?.address() + if (address === null || typeof address !== "object") throw new Error("Origin server failed to bind") + resolve(new URL(`http://127.0.0.1:${address.port}/`)) + }) + }) +} + +/** Like {@link startOrigin}, but the origin itself is reached over TLS with `cert`/`key`: an `https:` target, the shape a real login always is. */ +function startSecureOrigin(cert: string, key: string, handler: (req: IncomingMessage, res: import("node:http").ServerResponse) => void): Promise { + return new Promise((resolve) => { + secureOrigin = createHttpsServer({ cert, key }, handler) + trackSockets(secureOrigin) + secureOrigin.listen(0, "127.0.0.1", () => { + const address = secureOrigin?.address() + if (address === null || typeof address !== "object") throw new Error("Secure origin server failed to bind") + resolve(new URL(`https://127.0.0.1:${address.port}/`)) + }) + }) +} + +/** A CONNECT-accepting proxy that relays the tunnel to the real target, and records every CONNECT target it saw. */ +function startRelayProxy(): Promise<{ port: number; connectTargets: string[] }> { + return new Promise((resolve) => { + const connectTargets: string[] = [] + proxy = createHttpServer() + trackSockets(proxy) + proxy.on("connect", (req, clientSocket, head) => { + connectTargets.push(req.url ?? "") + const [host, portText] = (req.url ?? "").split(":") + const upstream = netConnect(Number(portText), host) + openSockets.add(upstream) + upstream.on("close", () => openSockets.delete(upstream)) + upstream.on("connect", () => { + clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n") + if (head.length > 0) upstream.write(head) + upstream.pipe(clientSocket) + clientSocket.pipe(upstream) + }) + upstream.on("error", () => clientSocket.destroy()) + clientSocket.on("error", () => upstream.destroy()) + }) + proxy.listen(0, "127.0.0.1", () => { + const address = proxy?.address() + if (address === null || typeof address !== "object") throw new Error("Proxy server failed to bind") + resolve({ port: address.port, connectTargets }) + }) + }) +} + +/** Like {@link startRelayProxy}, but the proxy itself is reached over TLS with `cert`/`key`: an `https:` proxy URL. */ +function startSecureRelayProxy(cert: string, key: string): Promise<{ port: number; connectTargets: string[] }> { + return new Promise((resolve) => { + const connectTargets: string[] = [] + secureProxy = createHttpsServer({ cert, key }) + trackSockets(secureProxy) + secureProxy.on("connect", (req, clientSocket, head) => { + connectTargets.push(req.url ?? "") + const [host, portText] = (req.url ?? "").split(":") + const upstream = netConnect(Number(portText), host) + openSockets.add(upstream) + upstream.on("close", () => openSockets.delete(upstream)) + upstream.on("connect", () => { + clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n") + if (head.length > 0) upstream.write(head) + upstream.pipe(clientSocket) + clientSocket.pipe(upstream) + }) + upstream.on("error", () => clientSocket.destroy()) + clientSocket.on("error", () => upstream.destroy()) + }) + secureProxy.listen(0, "127.0.0.1", () => { + const address = secureProxy?.address() + if (address === null || typeof address !== "object") throw new Error("Secure proxy server failed to bind") + resolve({ port: address.port, connectTargets }) + }) + }) +} + +/** A CONNECT-accepting proxy that never tunnels: it always answers with `status`, e.g. a 407. */ +function startRefusingProxy(status: number): Promise<{ port: number; connectTargets: string[] }> { + return new Promise((resolve) => { + const connectTargets: string[] = [] + proxy = createHttpServer() + trackSockets(proxy) + proxy.on("connect", (req, clientSocket) => { + connectTargets.push(req.url ?? "") + clientSocket.end(`HTTP/1.1 ${status} Proxy Refused\r\n\r\n`) + }) + proxy.listen(0, "127.0.0.1", () => { + const address = proxy?.address() + if (address === null || typeof address !== "object") throw new Error("Proxy server failed to bind") + resolve({ port: address.port, connectTargets }) + }) + }) +} + +describe("requestBoundedTextViaNode reaches the login host through an HTTP proxy (#481)", () => { + it("tunnels the request through the proxy and gets the real response back", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200, { "Content-Type": "text/plain" }) + res.end("ok") + }) + const relay = await startRelayProxy() + setElectronProxyResolution(`PROXY 127.0.0.1:${relay.port}`) + + const result = await requestBoundedTextViaNode(url) + + assert.equal(result, "ok") + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) + + it("posts the same body and headers through the tunnel as it would send directly", async () => { + let receivedBody = "" + let receivedHeaders: IncomingMessage["headers"] = {} + const url = await startOrigin((req, res) => { + receivedHeaders = req.headers + const chunks: Buffer[] = [] + req.on("data", (chunk: Buffer) => chunks.push(chunk)) + req.on("end", () => { + receivedBody = Buffer.concat(chunks).toString("utf8") + res.writeHead(200) + res.end("ok") + }) + }) + const relay = await startRelayProxy() + setElectronProxyResolution(`PROXY 127.0.0.1:${relay.port}`) + + await requestBoundedTextViaNode(url, { method: "POST", body: "a=b" }) + + assert.equal(receivedBody, "a=b") + assert.equal(receivedHeaders["content-type"], "application/x-www-form-urlencoded") + assert.equal(receivedHeaders["accept"], "application/json, text/plain;q=0.9") + }) +}) + +describe("requestBoundedTextViaNode maps a proxy CONNECT refusal to a fixed reason (#481)", () => { + it("maps a 407 to the proxy-auth-required reason, never reaching the origin", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("should never be reached") + }) + const refusing = await startRefusingProxy(407) + setElectronProxyResolution(`PROXY 127.0.0.1:${refusing.port}`) + + await assert.rejects(requestBoundedTextViaNode(url), /Login proxy requires authentication/) + assert.deepEqual(refusing.connectTargets, [`127.0.0.1:${url.port}`]) + }) +}) + +describe("requestBoundedTextViaNode refuses a SOCKS proxy up front (#481)", () => { + it("rejects with the proxy-unsupported reason before opening any socket", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("should never be reached") + }) + setElectronProxyResolution("SOCKS5 127.0.0.1:1080") + + await assert.rejects(requestBoundedTextViaNode(url), /Login proxy is not supported/) + }) +}) + +describe("requestBoundedTextViaNode keeps the direct path when the session answers DIRECT (#481)", () => { + it("bypasses the tunnel entirely, unchanged from before this issue", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + setElectronProxyResolution("DIRECT") + + assert.equal(await requestBoundedTextViaNode(url), "ok") + }) +}) + +describe("requestBoundedTextViaNode falls back to HTTPS_PROXY only when the session found nothing (#481)", () => { + it("uses HTTPS_PROXY when the session answers DIRECT", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + const relay = await startRelayProxy() + setElectronProxyResolution("DIRECT") + process.env.HTTPS_PROXY = `http://127.0.0.1:${relay.port}` + + const result = await requestBoundedTextViaNode(url) + + assert.equal(result, "ok") + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) + + it("still bypasses HTTPS_PROXY for a host NO_PROXY names", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + setElectronProxyResolution("DIRECT") + // Nothing is listening on this port: if NO_PROXY failed to bypass it, the request would + // fail to connect instead of succeeding. + process.env.HTTPS_PROXY = "http://127.0.0.1:1" + process.env.NO_PROXY = "127.0.0.1" + + assert.equal(await requestBoundedTextViaNode(url), "ok") + }) + + it("stays on the direct path when HTTPS_PROXY is not a valid URL", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + setElectronProxyResolution("DIRECT") + process.env.HTTPS_PROXY = "not a url" + + assert.equal(await requestBoundedTextViaNode(url), "ok") + }) +}) + +describe("requestBoundedTextViaNode keeps HTTPS_PROXY's own scheme (#481)", () => { + it("reaches an https HTTPS_PROXY over its own TLS connection instead of a plaintext CONNECT", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + const { cert, key } = createSelfSignedCert() + const relay = await startSecureRelayProxy(cert, key) + setElectronProxyResolution("DIRECT") + setTrustedCa(cert) + process.env.HTTPS_PROXY = `https://127.0.0.1:${relay.port}` + + const result = await requestBoundedTextViaNode(url) + + assert.equal(result, "ok") + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) + + it("rejects a socks5 HTTPS_PROXY as unsupported rather than treating it as an HTTP proxy", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("should never be reached") + }) + setElectronProxyResolution("DIRECT") + // Nothing is listening on this port: the old string-surgery fallback treated any + // HTTPS_PROXY as an HTTP proxy and would have tried to CONNECT through it, failing to + // connect instead of being refused up front. + process.env.HTTPS_PROXY = "socks5://127.0.0.1:1" + + await assert.rejects(requestBoundedTextViaNode(url), /Login proxy is not supported/) + }) +}) + +describe("requestBoundedTextViaNode reaches an https target through an HTTP proxy's CONNECT tunnel (#481)", () => { + it("wraps the tunnelled socket in TLS and posts the login body through it", async () => { + const { cert, key } = createSelfSignedCert() + let receivedBody = "" + const url = await startSecureOrigin(cert, key, (req, res) => { + const chunks: Buffer[] = [] + req.on("data", (chunk: Buffer) => chunks.push(chunk)) + req.on("end", () => { + receivedBody = Buffer.concat(chunks).toString("utf8") + res.writeHead(200, { "Content-Type": "text/plain" }) + res.end("ok") + }) + }) + const relay = await startRelayProxy() + setElectronProxyResolution(`PROXY 127.0.0.1:${relay.port}`) + setTrustedCa(cert) + + const body = "email=someone%40example.com&password=hunter2" + const result = await requestBoundedTextViaNode(url, { method: "POST", body }) + + assert.equal(result, "ok") + assert.equal(receivedBody, body) + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) +}) diff --git a/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx b/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx index 17f09628..11496351 100644 --- a/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx +++ b/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx @@ -50,6 +50,32 @@ describe("SessionButton names the family a login request failure belongs to", () }) } + /** + * The proxy half of #481: a login that failed because of a proxy the main process has no + * client for (an authenticated proxy answering 407, or a SOCKS answer) is classified as + * `proxy-auth-required`/`proxy-unsupported` in the main process, which join the + * `network-unreachable` family the renderer already had a sentence for (issue #482). + * Both reason tokens collapse into the same wire status before they ever cross the IPC + * boundary, so what a renderer test can prove is this: the sentence the family already + * shows names a proxy, not just a connection or a firewall. en-US and fr-FR both carry the + * wording already, so no new string was needed for either. + */ + it("mentions a proxy in the network-unreachable sentence, the family a proxy failure joins", async () => { + const login = vi.fn(async () => ({ status: "network-unreachable" }) as AccountLoginResult) + installMockWindowApi({ accountManager: { login } }) + + renderWithProviders( + <> + + + + ) + + await submitLogin() + + expect(await screen.findByText(/proxy/i)).toBeTruthy() + }) + it("still shows the generic message for a request failure loginFailureFamily could not place", async () => { const login = vi.fn(async () => { throw new Error("Login failed")