fix(video-call): open conference http(s) popups as sandboxed child windows (SUP-1108)

docs/video-call-window-management.md

@@ -355,6 +355,32 @@ webview.setAttribute('partition', 'persist:video-call-session'); // Generic part
 webview.src = url; // Set last - triggers loading
 ```
 
+### Conference Webview Popup Policy
+
+`allowpopups` alone is not enough to control what a conference page can spawn
+via `window.open` / `target="_blank"` — `src/videoCallWindow/ipc.ts` installs
+a `setWindowOpenHandler` on the attached conference webview (separate from the
+one used by the video call window's own host page) with the following policy:
+
+- **Tab-target links** (`disposition` `foreground-tab` / `background-tab`,
+  e.g. middle-click or ctrl-click) are sent to the system browser and the
+  Electron popup is denied.
+- **Plain `http(s)` popups** (default disposition) are allowed as sandboxed
+  Electron child windows (`nodeIntegration: false`, `contextIsolation: true`,
+  `sandbox: true`, no preload, hidden until `ready-to-show`). This is required
+  for identity-provider SSO login flows (e.g. Pexip's IdP popup), which rely
+  on `window.opener` to hand the result back to the conference page — routing
+  these to the system browser breaks that relationship.
+- **In-app popup schemes** (`about:`, `blob:`) are allowed as before, for
+  device pickers and transient blob/PDF exports.
+- **Everything else** (`javascript:`, `data:`, `file:`, `smb:`, etc.) is
+  denied.
+
+Any popup spawned this way gets the same policy applied recursively via
+`did-create-window`, so a nested popup or an in-popup navigation can't escape
+the sandbox. The video call window's own host page is unaffected by this
+change and keeps routing all `http(s)` popups to the system browser.
+
 ### Deferred React Import
 ```typescript
 let screenPickerModule: typeof import('./screenSharePickerMount') | null = null;

src/videoCallWindow/ipc.ts

@@ -322,6 +322,83 @@ const handleVideoCallWindowOpen = ({
   return { action: 'deny' };
 };
 
+// Window-open policy for the conference (guest) webview only. Unlike the host
+// page above, an http(s) popup here isn't always a plain external link — SSO
+// providers (e.g. Pexip's IdP login flow) rely on `window.open` handing
+// control back to the opener via `window.opener`, which only works if the
+// popup is a real (sandboxed) Electron child window. Tab-target links
+// (middle-click / ctrl-click) still go to the system browser; dangerous
+// schemes are still denied.
+type WindowOpenDetails = {
+  url: string;
+  disposition: string;
+};
+const handleConferenceWebviewWindowOpen = ({
+  url,
+  disposition,
+}: WindowOpenDetails):
+  | { action: 'deny' }
+  | {
+      action: 'allow';
+      overrideBrowserWindowOptions?: Electron.BrowserWindowConstructorOptions;
+    } => {
+  if (disposition === 'foreground-tab' || disposition === 'background-tab') {
+    isProtocolAllowed(url).then((allowed) => {
+      if (allowed) {
+        openExternal(url);
+      }
+    });
+    return { action: 'deny' };
+  }
+
+  if (url.startsWith('http://') || url.startsWith('https://')) {
+    return {
+      action: 'allow',
+      overrideBrowserWindowOptions: {
+        show: false,
+        autoHideMenuBar: true,
+        webPreferences: {
+          nodeIntegration: false,
+          contextIsolation: true,
+          sandbox: true,
+          webviewTag: false,
+        },
+      },
+    };
+  }
+
+  const lower = url.toLowerCase();
+  if (ALLOWED_POPUP_SCHEMES.some((scheme) => lower.startsWith(scheme))) {
+    return { action: 'allow' };
+  }
+  return { action: 'deny' };
+};
+
+// Shared by the conference webview's own `will-navigate` and any popup child
+// window it spawns: send external-protocol target="_self" navigations
+// (mailto:, tel:, custom schemes) to the browser; http(s)/file/data/about/blob
+// self-navigations stay in place so auth redirects and the conference's own
+// flows keep working.
+const handleConferenceWillNavigate = (event: Event, navUrl: string): void => {
+  try {
+    const { protocol } = new URL(navUrl);
+    if (
+      !['http:', 'https:', 'file:', 'data:', 'about:', 'blob:'].includes(
+        protocol
+      )
+    ) {
+      event.preventDefault();
+      isProtocolAllowed(navUrl).then((allowed) => {
+        if (allowed) {
+          openExternal(navUrl);
+        }
+      });
+    }
+  } catch {
+    // Ignore unparseable URLs.
+  }
+};
+
 const setupWebviewHandlers = (webContents: WebContents) => {
   // Track attached webviews that need handler setup
   const pendingWebviews: WebContents[] = [];
@@ -400,31 +477,29 @@ const setupWebviewHandlers = (webContents: WebContents) => {
     webviewWebContents: WebContents
   ): void => {
     // Route external links opened from the conference (target="_blank" /
-    // window.open) to the system browser instead of spawning a new Electron
-    // window, mirroring the main app window.
-    webviewWebContents.setWindowOpenHandler(handleVideoCallWindowOpen);
+    // window.open) to a sandboxed Electron popup window for plain http(s)
+    // (needed for SSO/IdP login flows such as Pexip, which rely on
+    // window.opener), and to the system browser for tab-target links and
+    // in-app popup schemes otherwise. See handleConferenceWebviewWindowOpen.
+    webviewWebContents.setWindowOpenHandler(handleConferenceWebviewWindowOpen);
 
     // Send external-protocol target="_self" navigations (mailto:, tel:, custom
     // schemes) to the browser too; http(s) self-navigations stay in the webview
     // so the conference's own flows (auth redirects, etc.) keep working.
-    webviewWebContents.on('will-navigate', (event: Event, navUrl: string) => {
-      try {
-        const { protocol } = new URL(navUrl);
-        if (
-          !['http:', 'https:', 'file:', 'data:', 'about:', 'blob:'].includes(
-            protocol
-          )
-        ) {
-          event.preventDefault();
-          isProtocolAllowed(navUrl).then((allowed) => {
-            if (allowed) {
-              openExternal(navUrl);
-            }
-          });
-        }
-      } catch {
-        // Ignore unparseable URLs.
-      }
+    webviewWebContents.on('will-navigate', handleConferenceWillNavigate);
+
+    // A guest-webview popup (e.g. an SSO/IdP login window) is created hidden
+    // to avoid a white flash, and its own popups/navigations must follow the
+    // same guest policy so a nested popup (or a redirect inside the popup)
+    // can't escape the sandbox.
+    webviewWebContents.on('did-create-window', (childWindow) => {
+      childWindow.once('ready-to-show', () => {
+        childWindow.show();
+      });
+      childWindow.webContents.setWindowOpenHandler(
+        handleConferenceWebviewWindowOpen
+      );
+      childWindow.webContents.on('will-navigate', handleConferenceWillNavigate);
     });
 
     // Media (mic/cam) permission requests from the conference originate in the

src/videoCallWindow/main/ipc.main.spec.ts

@@ -955,26 +955,60 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => {
       return guest;
     };
 
-    it('routes http(s) popups to the system browser and denies the Electron window', async () => {
+    it('allows http(s) popups as sandboxed Electron child windows (SSO/IdP flow)', async () => {
       const guest = await attachGuest();
       expect(guest.setWindowOpenHandler).toHaveBeenCalledTimes(1);
       const handler = guest.setWindowOpenHandler.mock.calls[0][0];
 
-      expect(handler({ url: 'https://example.com/page' })).toEqual({
+      const result = handler({
+        url: 'https://example.com/page',
+        disposition: 'new-window',
+      });
+      expect(result.action).toBe('allow');
+      expect(result.overrideBrowserWindowOptions.webPreferences).toMatchObject({
+        nodeIntegration: false,
+        contextIsolation: true,
+        sandbox: true,
+      });
+
+      const { openExternal } = (await import(
+        '../../utils/browserLauncher'
+      )) as any;
+      expect(openExternal).not.toHaveBeenCalled();
+    });
+
+    it('routes tab-disposition http(s) popups to the system browser and denies the Electron window', async () => {
+      const guest = await attachGuest();
+      const handler = guest.setWindowOpenHandler.mock.calls[0][0];
+
+      expect(
+        handler({
+          url: 'https://example.com/page',
+          disposition: 'foreground-tab',
+        })
+      ).toEqual({
         action: 'deny',
       });
       const { openExternal } = (await import(
         '../../utils/browserLauncher'
       )) as any;
+      await flushPromises();
       expect(openExternal).toHaveBeenCalledWith('https://example.com/page');
     });
 
     it('allows in-app (non-external) popups', async () => {
       const guest = await attachGuest();
       const handler = guest.setWindowOpenHandler.mock.calls[0][0];
 
-      expect(handler({ url: 'about:blank' })).toEqual({ action: 'allow' });
-      expect(handler({ url: 'blob:https://meet.example/abc' })).toEqual({
+      expect(
+        handler({ url: 'about:blank', disposition: 'new-window' })
+      ).toEqual({ action: 'allow' });
+      expect(
+        handler({
+          url: 'blob:https://meet.example/abc',
+          disposition: 'new-window',
+        })
+      ).toEqual({
         action: 'allow',
       });
     });
@@ -983,16 +1017,27 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => {
       const guest = await attachGuest();
       const handler = guest.setWindowOpenHandler.mock.calls[0][0];
 
-      expect(handler({ url: 'javascript:alert(1)' })).toEqual({
+      expect(
+        handler({ url: 'javascript:alert(1)', disposition: 'new-window' })
+      ).toEqual({
         action: 'deny',
       });
-      expect(handler({ url: 'file:///etc/passwd' })).toEqual({
+      expect(
+        handler({ url: 'file:///etc/passwd', disposition: 'new-window' })
+      ).toEqual({
         action: 'deny',
       });
-      expect(handler({ url: 'data:text/html,<script>1</script>' })).toEqual({
+      expect(
+        handler({
+          url: 'data:text/html,<script>1</script>',
+          disposition: 'new-window',
+        })
+      ).toEqual({
         action: 'deny',
       });
-      expect(handler({ url: 'smb://share/path' })).toEqual({ action: 'deny' });
+      expect(
+        handler({ url: 'smb://share/path', disposition: 'new-window' })
+      ).toEqual({ action: 'deny' });
     });
 
     it('registers a will-navigate handler on the guest webview', async () => {
@@ -1003,6 +1048,73 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => {
       );
     });
 
+    it('registers a did-create-window handler that wires the popup child window with the guest policy', async () => {
+      const guest = await attachGuest();
+      expect(guest.on).toHaveBeenCalledWith(
+        'did-create-window',
+        expect.any(Function)
+      );
+      const didCreateWindowHandler = guest.on.mock.calls.find(
+        ([event]: [string]) => event === 'did-create-window'
+      )[1];
+
+      const childWindow = {
+        once: jest.fn(),
+        show: jest.fn(),
+        webContents: {
+          setWindowOpenHandler: jest.fn(),
+          on: jest.fn(),
+        },
+      };
+
+      didCreateWindowHandler(childWindow, { url: 'https://idp.example/sso' });
+
+      expect(
+        childWindow.webContents.setWindowOpenHandler
+      ).toHaveBeenCalledTimes(1);
+      expect(childWindow.webContents.on).toHaveBeenCalledWith(
+        'will-navigate',
+        expect.any(Function)
+      );
+
+      expect(childWindow.once).toHaveBeenCalledWith(
+        'ready-to-show',
+        expect.any(Function)
+      );
+      const readyToShowCallback = childWindow.once.mock.calls.find(
+        ([event]: [string]) => event === 'ready-to-show'
+      )[1];
+      expect(childWindow.show).not.toHaveBeenCalled();
+      readyToShowCallback();
+      expect(childWindow.show).toHaveBeenCalledTimes(1);
+    });
+
+    it('leaves the host window popup policy unchanged: http(s) still denied + sent to the system browser', async () => {
+      await attachGuest();
+
+      // The host window's webContents (createdWindows[0].webContents) has its
+      // window-open handler registered twice (an smb-only guard, then the
+      // shared handleVideoCallWindowOpen wrapper) — the last registration is
+      // the one Electron actually uses.
+      const hostSetWindowOpenHandler =
+        createdWindows[0].webContents.setWindowOpenHandler;
+      expect(hostSetWindowOpenHandler.mock.calls.length).toBeGreaterThanOrEqual(
+        2
+      );
+      const hostHandler =
+        hostSetWindowOpenHandler.mock.calls[
+          hostSetWindowOpenHandler.mock.calls.length - 1
+        ][0];
+
+      expect(hostHandler({ url: 'https://example.com/page' })).toEqual({
+        action: 'deny',
+      });
+      const { openExternal } = (await import(
+        '../../utils/browserLauncher'
+      )) as any;
+      expect(openExternal).toHaveBeenCalledWith('https://example.com/page');
+    });
+
     it('installs a media permission handler on the fallback (isolated) webview session', async () => {
       const { handleMediaPermissionRequest } = (await import(
         '../../ui/main/mediaPermissions'