From 5ff2c5feba5261104c03d0a724cadf0d550fd058 Mon Sep 17 00:00:00 2001
From: joel <13933101+lurkshark@users.noreply.github.com>
Date: Tue, 10 Nov 2026 18:28:54 -0800
Subject: [PATCH] CUS-221: Explain port conflicts when two logins run at once

Login uses a fixed local redirect port (52700 for Google, 52701 for Azure). If a second `p0 login`, or a command that starts a login, runs while another is waiting for browser approval, binding that port failed with a raw `listen EADDRINUSE` error.

`withRedirectServer` now detects `EADDRINUSE` and fails with a short message naming the port and telling the user to finish or cancel the other login and rerun. The message is thrown as a plain string so no stack trace prints. Other `listen` errors are passed through unchanged, and the ports and the login flow are the same when the port is free. The listen error handler also no longer rejects the redirect promise, which nothing awaits at that point and which would have surfaced as an unhandled rejection that hid the new message.
---
 src/common/auth/__tests__/server.test.ts | 50 ++++++++++++++++++++++++
 src/common/auth/server.ts                | 18 ++++++---
 2 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100644 src/common/auth/__tests__/server.test.ts

diff --git a/src/common/auth/__tests__/server.test.ts b/src/common/auth/__tests__/server.test.ts
new file mode 100644
index 00000000..b89aaaa6
--- /dev/null
+++ b/src/common/auth/__tests__/server.test.ts
@@ -0,0 +1,50 @@
+/** Copyright © 2024-present P0 Security
+
+This file is part of @p0security/cli
+
+@p0security/cli is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 3 of the License.
+
+@p0security/cli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License along with @p0security/cli. If not, see <https://www.gnu.org/licenses/>.
+**/
+import { withRedirectServer } from "../server";
+import http from "node:http";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+// Under vitest the landing-page assets resolve to src/public, which does not exist
+vi.mock("node:fs/promises", () => ({
+  readFile: vi.fn().mockResolvedValue(Buffer.from("")),
+}));
+
+vi.mock("../../../util", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../../../util")>()),
+  getAppName: () => "p0",
+}));
+
+describe("withRedirectServer", () => {
+  let blocker: http.Server;
+  let port: number;
+
+  beforeEach(async () => {
+    blocker = http.createServer();
+    await new Promise<void>((resolve) => blocker.listen(0, resolve));
+    const address = blocker.address();
+    if (!address || typeof address === "string")
+      throw new Error("blocker is not listening on a TCP port");
+    port = address.port;
+  });
+
+  afterEach(async () => {
+    await new Promise<void>((resolve) => blocker.close(() => resolve()));
+  });
+
+  it("explains that another login holds the port when the port is in use", async () => {
+    const beginAuth = vi.fn();
+
+    await expect(withRedirectServer(beginAuth, vi.fn(), { port })).rejects.toBe(
+      `Login failed: port ${port} is already in use, probably by another \`p0 login\` waiting for browser approval.\nFinish or cancel that login (Ctrl+C in its terminal), then run this command again.`
+    );
+    expect(beginAuth).not.toHaveBeenCalled();
+  });
+});
diff --git a/src/common/auth/server.ts b/src/common/auth/server.ts
index 42de5be6..e3a614f7 100644
--- a/src/common/auth/server.ts
+++ b/src/common/auth/server.ts
@@ -10,7 +10,7 @@ You should have received a copy of the GNU General Public License along with @p0
 **/
 
 /** Implements a local auth server, which can receive auth tokens from an OIDC app */
-import { sleep } from "../../util";
+import { getAppName, sleep } from "../../util";
 import express from "express";
 import { noop } from "lodash";
 import { readFile } from "node:fs/promises";
@@ -89,7 +89,8 @@ export const withRedirectServer = async <S, T, U>(
 
   app.use(redirectRouter);
 
-  const server = app.listen(options?.port ?? 0);
+  const port = options?.port ?? 0;
+  const server = app.listen(port);
 
   // Set up cleanup handler for process interruption
   const cleanup = async () => {
@@ -109,11 +110,18 @@ export const withRedirectServer = async <S, T, U>(
   process.once("SIGTERM", signalHandler);
 
   // Wait for server to start listening or fail
+  // Rejects only this promise: redirectPromise is never awaited when listen fails,
+  // so rejecting it would surface as an unhandled rejection
   await new Promise<void>((resolve, reject) => {
     server.once("listening", () => resolve());
-    server.once("error", (error) => {
-      redirectReject(error);
-      reject(error);
+    server.once("error", (error: NodeJS.ErrnoException) => {
+      if (error.code === "EADDRINUSE") {
+        reject(
+          `Login failed: port ${port} is already in use, probably by another \`${getAppName()} login\` waiting for browser approval.\nFinish or cancel that login (Ctrl+C in its terminal), then run this command again.`
+        );
+      } else {
+        reject(error);
+      }
     });
   });