Fix sign-in popups in the desktop browser (#2137)

* fix(desktop): keep sign-in popups connected

Turning every window-open request into a workspace tab severed the opener relationship required by popup authentication flows. Keep script-created and POST-backed opens as secured child windows while ordinary links continue to use workspace tabs.

* fix(desktop): keep Shift-click links in workspace tabs

Electron reports both popup windows and Shift-clicked links as new-window. Use popup features or a named target to preserve script popups without bypassing Paseo tab state for ordinary links.

* fix(desktop): tighten popup profile handling

* fix(desktop): preserve popup feature intent

* fix(desktop): match browser popup heuristics

packages/desktop/src/features/browser-profile.test.ts

@@ -66,12 +66,12 @@ class FakeWebContents extends FakeLiveGuest {
 }
 
 describe("listPaseoBrowserProfileGuests", () => {
-  test("returns every live webview in the shared profile without deduplicating tabs", () => {
+  test("returns every live webview and popup in the shared profile", () => {
     const profileSession = {};
     const firstWindowGuest = new FakeWebContents(1, profileSession, "webview");
     const secondWindowGuest = new FakeWebContents(2, profileSession, "webview");
     const foreignProfileGuest = new FakeWebContents(3, {}, "webview");
-    const mainRenderer = new FakeWebContents(4, profileSession, "window");
+    const popupWindow = new FakeWebContents(4, profileSession, "window");
     const destroyedGuest = new FakeWebContents(5, profileSession, "webview", true);
 
     const guests = listPaseoBrowserProfileGuests({
@@ -80,12 +80,12 @@ describe("listPaseoBrowserProfileGuests", () => {
         firstWindowGuest,
         secondWindowGuest,
         foreignProfileGuest,
-        mainRenderer,
+        popupWindow,
         destroyedGuest,
       ],
     });
 
-    expect(guests).toEqual([firstWindowGuest, secondWindowGuest]);
+    expect(guests).toEqual([firstWindowGuest, secondWindowGuest, popupWindow]);
   });
 });
 

packages/desktop/src/features/browser-profile.ts

@@ -96,7 +96,7 @@ export function listPaseoBrowserProfileGuests(
   return input.webContents.filter(
     (contents) =>
       !contents.isDestroyed() &&
-      contents.getType() === "webview" &&
+      (contents.getType() === "webview" || contents.getType() === "window") &&
       contents.session === input.profileSession,
   );
 }

packages/desktop/src/features/browser-webviews/index.ts

@@ -2,7 +2,7 @@ import { webContents as allWebContents, type WebContents } from "electron";
 import { PASEO_BROWSER_PROFILE_PARTITION } from "../browser-profile.js";
 import {
   BROWSER_NEW_TAB_REQUEST_EVENT,
-  handleBrowserWindowOpenRequest,
+  decideBrowserWindowOpenRequest,
   isAllowedBrowserWebviewUrl,
   PendingBrowserWindowOpenRequests,
 } from "./window-open.js";
@@ -10,7 +10,7 @@ import { PaseoBrowserWebviewRegistry } from "./registry.js";
 
 export {
   BROWSER_NEW_TAB_REQUEST_EVENT,
-  handleBrowserWindowOpenRequest,
+  decideBrowserWindowOpenRequest,
   PendingBrowserWindowOpenRequests,
 };
 

packages/desktop/src/features/browser-webviews/window-open.test.ts

@@ -1,40 +1,162 @@
-import { describe, expect, it, vi } from "vitest";
+import { describe, expect, it } from "vitest";
 
-import { handleBrowserWindowOpenRequest, PendingBrowserWindowOpenRequests } from ".";
+import { decideBrowserWindowOpenRequest, PendingBrowserWindowOpenRequests } from ".";
 
 describe("browser webview window-open requests", () => {
-  it("denies Electron window creation and requests a Paseo browser tab", () => {
-    const requestNewTab = vi.fn();
-
-    const result = handleBrowserWindowOpenRequest({
+  it("routes foreground tabs to a Paseo workspace tab", () => {
+    const result = decideBrowserWindowOpenRequest({
       url: "https://example.com/target",
-      sourceBrowserId: "browser-1",
-      requestNewTab,
+      disposition: "foreground-tab",
+      frameName: "_blank",
+      features: "",
+      hasPostBody: false,
     });
 
-    expect(result).toEqual({ action: "deny" });
-    expect(requestNewTab).toHaveBeenCalledWith({
-      sourceBrowserId: "browser-1",
-      url: "https://example.com/target",
-    });
+    expect(result).toEqual({ kind: "workspace-tab", url: "https://example.com/target" });
   });
 
-  it("denies unsupported window-open requests before asking for a Paseo browser tab", () => {
-    const requestNewTab = vi.fn();
-
-    const result = handleBrowserWindowOpenRequest({
-      url: "file:///etc/passwd",
-      sourceBrowserId: "browser-1",
-      requestNewTab,
+  it("keeps script-opened windows as real popups", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://login.example.com/signin",
+      disposition: "new-window",
+      frameName: "oauth",
+      features: "width=500,height=600",
+      hasPostBody: false,
     });
 
-    expect(result).toEqual({ action: "deny" });
-    expect(requestNewTab).not.toHaveBeenCalled();
+    expect(result).toEqual({ kind: "popup" });
+  });
+
+  it("keeps named windows as real popups without a feature string", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://login.example.com/signin",
+      disposition: "new-window",
+      frameName: "oauth",
+      features: "",
+      hasPostBody: false,
+    });
+
+    expect(result).toEqual({ kind: "popup" });
+  });
+
+  it.each(["noopener", "noreferrer"])(
+    "routes a named target with %s to a Paseo workspace tab",
+    (features) => {
+      const result = decideBrowserWindowOpenRequest({
+        url: "https://example.com/target",
+        disposition: "new-window",
+        frameName: "secure-target",
+        features,
+        hasPostBody: false,
+      });
+
+      expect(result).toEqual({ kind: "workspace-tab", url: "https://example.com/target" });
+    },
+  );
+
+  it("routes Shift-clicked links to a Paseo workspace tab", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://example.com/target",
+      disposition: "new-window",
+      frameName: "",
+      features: "",
+      hasPostBody: false,
+    });
+
+    expect(result).toEqual({ kind: "workspace-tab", url: "https://example.com/target" });
+  });
+
+  it.each(["noopener", "noreferrer", "attributionsrc=https://example.com/register", "popup=false"])(
+    "routes non-popup feature %s to a Paseo workspace tab",
+    (features) => {
+      const result = decideBrowserWindowOpenRequest({
+        url: "https://example.com/target",
+        disposition: "new-window",
+        frameName: "_blank",
+        features,
+        hasPostBody: false,
+      });
+
+      expect(result).toEqual({ kind: "workspace-tab", url: "https://example.com/target" });
+    },
+  );
+
+  it("keeps an explicitly requested popup as a real popup", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://login.example.com/signin",
+      disposition: "new-window",
+      frameName: "_blank",
+      features: "noopener,popup=yes",
+      hasPostBody: false,
+    });
+
+    expect(result).toEqual({ kind: "popup" });
+  });
+
+  it("keeps legacy browser-chrome features as a real popup", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://login.example.com/signin",
+      disposition: "new-window",
+      frameName: "_blank",
+      features: "menubar=no,toolbar=no,status=no,scrollbars=no",
+      hasPostBody: false,
+    });
+
+    expect(result).toEqual({ kind: "popup" });
+  });
+
+  it("keeps unknown window features as a real popup", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://login.example.com/signin",
+      disposition: "new-window",
+      frameName: "_blank",
+      features: "dialog=yes",
+      hasPostBody: false,
+    });
+
+    expect(result).toEqual({ kind: "popup" });
+  });
+
+  it("routes an all-enabled browser-chrome request to a Paseo workspace tab", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://example.com/target",
+      disposition: "new-window",
+      frameName: "_blank",
+      features:
+        "toolbar=yes,location=yes,menubar=yes,status=yes,scrollbars=yes,resizable=yes,noopener",
+      hasPostBody: false,
+    });
+
+    expect(result).toEqual({ kind: "workspace-tab", url: "https://example.com/target" });
+  });
+
+  it("keeps POST-backed foreground tabs as real popups", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "https://example.com/submit",
+      disposition: "foreground-tab",
+      frameName: "_blank",
+      features: "",
+      hasPostBody: true,
+    });
+
+    expect(result).toEqual({ kind: "popup" });
+  });
+
+  it("denies unsupported window-open requests", () => {
+    const result = decideBrowserWindowOpenRequest({
+      url: "file:///etc/passwd",
+      disposition: "new-window",
+      frameName: "oauth",
+      features: "width=500,height=600",
+      hasPostBody: false,
+    });
+
+    expect(result).toEqual({ kind: "deny" });
   });
 });
 
 describe("pending browser window-open requests", () => {
-  it("holds early allowed popups until browser identity registration", () => {
+  it("holds early workspace-tab requests until browser identity registration", () => {
     const pending = new PendingBrowserWindowOpenRequests();
     pending.add(101, "https://example.com/first");
     pending.add(101, "file:///etc/passwd");
@@ -44,7 +166,7 @@ describe("pending browser window-open requests", () => {
     expect(pending.take(101)).toEqual([]);
   });
 
-  it("drops pending popups when an unregistered guest is destroyed", () => {
+  it("drops pending workspace-tab requests when an unregistered guest is destroyed", () => {
     const pending = new PendingBrowserWindowOpenRequests();
     pending.add(202, "https://example.com/target");
     pending.delete(202);

packages/desktop/src/features/browser-webviews/window-open.ts

@@ -1,11 +1,46 @@
 export const BROWSER_NEW_TAB_REQUEST_EVENT = "paseo:event:browser-new-tab-request";
 
-export interface BrowserNewTabRequestPayload {
-  sourceBrowserId: string;
-  url: string;
-}
+export type BrowserWindowOpenDisposition =
+  | "default"
+  | "foreground-tab"
+  | "background-tab"
+  | "new-window"
+  | "other";
+
+export type BrowserWindowOpenDecision =
+  | { kind: "deny" }
+  | { kind: "popup" }
+  | { kind: "workspace-tab"; url: string };
 
 const MAX_PENDING_WINDOW_OPEN_REQUESTS_PER_GUEST = 20;
+const POPUP_WINDOW_GEOMETRY_FEATURE_NAMES = new Set([
+  "height",
+  "innerheight",
+  "innerwidth",
+  "left",
+  "outerheight",
+  "outerwidth",
+  "screenx",
+  "screeny",
+  "top",
+  "width",
+  "x",
+  "y",
+]);
+const POPUP_WINDOW_UI_FEATURE_NAMES = new Set([
+  "location",
+  "menubar",
+  "resizable",
+  "scrollbars",
+  "status",
+  "toolbar",
+]);
+const NON_POPUP_WINDOW_FEATURE_NAMES = new Set([
+  "attributionsrc",
+  "noopener",
+  "noreferrer",
+  "popup",
+]);
 
 export class PendingBrowserWindowOpenRequests {
   private readonly urlsByWebContentsId = new Map<number, string[]>();
@@ -47,18 +82,87 @@ export function isAllowedBrowserWebviewUrl(value: string | undefined): boolean {
   }
 }
 
-export function handleBrowserWindowOpenRequest(input: {
+export function decideBrowserWindowOpenRequest(input: {
   url: string;
-  sourceBrowserId: string | null;
-  requestNewTab: (payload: BrowserNewTabRequestPayload) => void;
-}): { action: "deny" } {
-  if (!isAllowedBrowserWebviewUrl(input.url) || !input.sourceBrowserId) {
-    return { action: "deny" };
+  disposition: BrowserWindowOpenDisposition;
+  frameName: string;
+  features: string;
+  hasPostBody: boolean;
+}): BrowserWindowOpenDecision {
+  if (!isAllowedBrowserWebviewUrl(input.url)) {
+    return { kind: "deny" };
   }
 
-  input.requestNewTab({
-    sourceBrowserId: input.sourceBrowserId,
-    url: input.url,
-  });
-  return { action: "deny" };
+  const featureIntent = getBrowserWindowFeatureIntent(input.features);
+  const hasNamedWindowTarget = input.frameName.length > 0 && input.frameName !== "_blank";
+  const isScriptPopup =
+    input.disposition === "new-window" &&
+    (featureIntent.requestsPopup || (hasNamedWindowTarget && !featureIntent.disownsOpener));
+
+  // A real popup preserves window.opener, postMessage, named-window reuse, and
+  // window.close(). OAuth and payment flows depend on those browser contracts.
+  // POST-backed opens must also remain real windows because a workspace tab can
+  // only carry the URL and would silently turn the request into a GET.
+  if (isScriptPopup || input.hasPostBody) {
+    return { kind: "popup" };
+  }
+
+  return { kind: "workspace-tab", url: input.url };
+}
+
+function getBrowserWindowFeatureIntent(features: string): {
+  requestsPopup: boolean;
+  disownsOpener: boolean;
+} {
+  let requestsPopup = false;
+  let disownsOpener = false;
+  let hasPopupRelevantFeature = false;
+  const enabledUiFeatures = new Map<string, boolean>();
+
+  for (const rawFeature of features.split(",")) {
+    const separatorIndex = rawFeature.indexOf("=");
+    const name = rawFeature
+      .slice(0, separatorIndex === -1 ? undefined : separatorIndex)
+      .trim()
+      .toLowerCase();
+    const value =
+      separatorIndex === -1
+        ? ""
+        : rawFeature
+            .slice(separatorIndex + 1)
+            .trim()
+            .toLowerCase();
+
+    if (POPUP_WINDOW_GEOMETRY_FEATURE_NAMES.has(name)) {
+      requestsPopup = true;
+    }
+    if (POPUP_WINDOW_UI_FEATURE_NAMES.has(name)) {
+      hasPopupRelevantFeature = true;
+      enabledUiFeatures.set(name, isEnabledWindowFeature(value));
+    } else if (name.length > 0 && !NON_POPUP_WINDOW_FEATURE_NAMES.has(name)) {
+      hasPopupRelevantFeature = true;
+    }
+    if (name === "popup" && isEnabledWindowFeature(value)) {
+      requestsPopup = true;
+    }
+    if ((name === "noopener" || name === "noreferrer") && isEnabledWindowFeature(value)) {
+      disownsOpener = true;
+    }
+  }
+
+  if (!requestsPopup && hasPopupRelevantFeature) {
+    const isUiFeatureEnabled = (name: string): boolean => enabledUiFeatures.get(name) ?? false;
+    requestsPopup =
+      (!isUiFeatureEnabled("location") && !isUiFeatureEnabled("toolbar")) ||
+      !isUiFeatureEnabled("menubar") ||
+      !isUiFeatureEnabled("resizable") ||
+      !isUiFeatureEnabled("scrollbars") ||
+      !isUiFeatureEnabled("status");
+  }
+
+  return { requestsPopup, disownsOpener };
+}
+
+function isEnabledWindowFeature(value: string): boolean {
+  return value !== "0" && value !== "false" && value !== "no";
 }