CUS-221: Explain port conflicts when two logins run at once

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();
+  });
+});

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);
+      }
     });
   });