From a83f68833525e8e97e7092503d8424cce46622e2 Mon Sep 17 00:00:00 2001 From: Jean Brito Date: Fri, 4 Sep 2026 09:21:28 -0300 Subject: [PATCH 1/4] fix(video-call): open conference http(s) popups as sandboxed child windows Since 4.16.0 (#3370) the conference webview's window-open handler denied every http(s) popup and sent it to the system browser. Identity-provider SSO flows such as Pexip's log in through a popup and hand the result back via window.opener, so login never completed inside the video call window. The conference webview now has its own policy: plain http(s) popups open as sandboxed Electron child windows (nodeIntegration off, contextIsolation on, sandbox on, no preload, shown on ready-to-show), tab dispositions still go to the system browser, about:/blob: stay allowed, and javascript:/data:/ file: stay denied. Popups re-apply the same policy to their own popups and navigations. The host page handler is unchanged. Fixes SUP-1108. --- docs/video-call-window-management.md | 26 +++++ src/videoCallWindow/ipc.ts | 117 ++++++++++++++++---- src/videoCallWindow/main/ipc.main.spec.ts | 128 ++++++++++++++++++++-- 3 files changed, 242 insertions(+), 29 deletions(-) diff --git a/docs/video-call-window-management.md b/docs/video-call-window-management.md index 1bafecfc93..e3114e9733 100644 --- a/docs/video-call-window-management.md +++ b/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; diff --git a/src/videoCallWindow/ipc.ts b/src/videoCallWindow/ipc.ts index 85ce3883d0..d966ed6120 100644 --- a/src/videoCallWindow/ipc.ts +++ b/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 diff --git a/src/videoCallWindow/main/ipc.main.spec.ts b/src/videoCallWindow/main/ipc.main.spec.ts index d05e56a112..25bf160d75 100644 --- a/src/videoCallWindow/main/ipc.main.spec.ts +++ b/src/videoCallWindow/main/ipc.main.spec.ts @@ -955,17 +955,44 @@ 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'); }); @@ -973,8 +1000,15 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => { 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,' })).toEqual({ + expect( + handler({ + url: 'data:text/html,', + 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' From 8c1154842a33262e99acd75bb084ebd867111efa Mon Sep 17 00:00:00 2001 From: Jean Brito Date: Fri, 4 Sep 2026 09:58:43 -0300 Subject: [PATCH 2/4] docs(agents): keep internal references out of PR descriptions PR descriptions and commit bodies are public. Document that internal tracker links and ticket text, customer details, and local developer tooling notes stay out of them, while a bare ticket key in the subject remains the accepted convention. --- AGENTS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 54a9ef1dbf..01451cde86 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -340,6 +340,12 @@ git worktree add ../Rocket.Chat.Electron-worktrees/feature-name -b new-branch de Only include numbers from actual logs, error messages, or documented sources. - PR descriptions: straightforward language, focus on what changed and why. +- PR descriptions and commit bodies are public. Do not include internal + references (Jira or Zoho Desk links and ticket text, customer names, + support conversations) or local developer tooling notes (GitNexus, + agent workflow, ledgers). A bare Jira key in the title or subject is + the accepted convention; everything else about the ticket stays + internal. # GitNexus — Code Intelligence From a3c3662d0471403ca3410812fd737839a3ef3b3a Mon Sep 17 00:00:00 2001 From: Jean Brito Date: Fri, 4 Sep 2026 10:27:39 -0300 Subject: [PATCH 3/4] fix(video-call): apply the popup protocol policy to server-side redirects will-navigate only covers page-initiated navigations. A 302 from an http(s) popup to a custom scheme fires will-redirect instead, so register the same protocol check for will-redirect on the conference webview and on popup child windows. --- docs/video-call-window-management.md | 8 +++--- src/videoCallWindow/ipc.ts | 13 ++++++---- src/videoCallWindow/main/ipc.main.spec.ts | 31 ++++++++++++++++++++++- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/docs/video-call-window-management.md b/docs/video-call-window-management.md index e3114e9733..70f1a9a9bf 100644 --- a/docs/video-call-window-management.md +++ b/docs/video-call-window-management.md @@ -377,9 +377,11 @@ one used by the video call window's own host page) with the following policy: 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. +`did-create-window`. Both page-initiated navigations (`will-navigate`) and +server-side redirects (`will-redirect`) go through the same protocol check, +so a nested popup or an in-popup navigation/redirect 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 diff --git a/src/videoCallWindow/ipc.ts b/src/videoCallWindow/ipc.ts index d966ed6120..7ed761fe96 100644 --- a/src/videoCallWindow/ipc.ts +++ b/src/videoCallWindow/ipc.ts @@ -374,11 +374,12 @@ const handleConferenceWebviewWindowOpen = ({ 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. +// Shared by the conference webview's own `will-navigate`/`will-redirect` and +// any popup child window it spawns: send external-protocol target="_self" +// navigations (mailto:, tel:, custom schemes) — whether page-initiated +// (`will-navigate`) or a server-side redirect (`will-redirect`) — to the +// browser; http(s)/file/data/about/blob 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); @@ -487,6 +488,7 @@ const setupWebviewHandlers = (webContents: WebContents) => { // 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', handleConferenceWillNavigate); + webviewWebContents.on('will-redirect', 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 @@ -500,6 +502,7 @@ const setupWebviewHandlers = (webContents: WebContents) => { handleConferenceWebviewWindowOpen ); childWindow.webContents.on('will-navigate', handleConferenceWillNavigate); + childWindow.webContents.on('will-redirect', handleConferenceWillNavigate); }); // Media (mic/cam) permission requests from the conference originate in the diff --git a/src/videoCallWindow/main/ipc.main.spec.ts b/src/videoCallWindow/main/ipc.main.spec.ts index 25bf160d75..bedba7f7ea 100644 --- a/src/videoCallWindow/main/ipc.main.spec.ts +++ b/src/videoCallWindow/main/ipc.main.spec.ts @@ -1040,12 +1040,37 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => { ).toEqual({ action: 'deny' }); }); - it('registers a will-navigate handler on the guest webview', async () => { + it('registers will-navigate and will-redirect handlers on the guest webview', async () => { const guest = await attachGuest(); expect(guest.on).toHaveBeenCalledWith( 'will-navigate', expect.any(Function) ); + expect(guest.on).toHaveBeenCalledWith( + 'will-redirect', + expect.any(Function) + ); + }); + + it('will-redirect: denies a redirect to a dangerous scheme and sends it to the system browser, but leaves http(s) alone', async () => { + const guest = await attachGuest(); + const willRedirectHandler = guest.on.mock.calls.find( + ([event]: [string]) => event === 'will-redirect' + )[1]; + + const { openExternal } = (await import( + '../../utils/browserLauncher' + )) as any; + + const dangerousEvent = { preventDefault: jest.fn() }; + willRedirectHandler(dangerousEvent, 'smb://share/path'); + expect(dangerousEvent.preventDefault).toHaveBeenCalledTimes(1); + await flushPromises(); + expect(openExternal).toHaveBeenCalledWith('smb://share/path'); + + const httpEvent = { preventDefault: jest.fn() }; + willRedirectHandler(httpEvent, 'https://example.com/page'); + expect(httpEvent.preventDefault).not.toHaveBeenCalled(); }); it('registers a did-create-window handler that wires the popup child window with the guest policy', async () => { @@ -1076,6 +1101,10 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => { 'will-navigate', expect.any(Function) ); + expect(childWindow.webContents.on).toHaveBeenCalledWith( + 'will-redirect', + expect.any(Function) + ); expect(childWindow.once).toHaveBeenCalledWith( 'ready-to-show', From 01cec2046b62e1f94aca03203923115a3066cfdf Mon Sep 17 00:00:00 2001 From: Jean Brito Date: Fri, 4 Sep 2026 11:20:04 -0300 Subject: [PATCH 4/4] fix(video-call): police descendant popups and document the navigation allowlist The did-create-window listener only wired the popup policy onto the immediate child. Extract it into attachConferencePopupPolicy, which also registers itself on each child's did-create-window so every descendant popup gets the window-open handler and navigation guards. State the exact scheme allowlist in the popup policy doc instead of claiming nested navigation cannot escape. --- docs/video-call-window-management.md | 15 ++++--- src/videoCallWindow/ipc.ts | 34 +++++++++------ src/videoCallWindow/main/ipc.main.spec.ts | 53 +++++++++++++++++++++-- 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/docs/video-call-window-management.md b/docs/video-call-window-management.md index 70f1a9a9bf..b5c58c765a 100644 --- a/docs/video-call-window-management.md +++ b/docs/video-call-window-management.md @@ -376,12 +376,15 @@ one used by the video call window's own host page) with the following policy: - **Everything else** (`javascript:`, `data:`, `file:`, `smb:`, etc.) is denied. -Any popup spawned this way gets the same policy applied recursively via -`did-create-window`. Both page-initiated navigations (`will-navigate`) and -server-side redirects (`will-redirect`) go through the same protocol check, -so a nested popup or an in-popup navigation/redirect 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. +Any popup spawned this way gets the same policy re-applied to every +descendant popup (a popup opened from a popup, and so on) via each window's +own `did-create-window` registration. Both page-initiated navigations +(`will-navigate`) and server-side redirects (`will-redirect`) go through the +same protocol check: navigations/redirects to `http:`, `https:`, `file:`, +`data:`, `about:`, or `blob:` proceed in place, and any other scheme is +cancelled and, if the app's external-protocol check allows it, handed to the +system browser. 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 diff --git a/src/videoCallWindow/ipc.ts b/src/videoCallWindow/ipc.ts index 7ed761fe96..d36dd17407 100644 --- a/src/videoCallWindow/ipc.ts +++ b/src/videoCallWindow/ipc.ts @@ -400,6 +400,23 @@ const handleConferenceWillNavigate = (event: Event, navUrl: string): void => { } }; +// Applies the conference guest's popup policy (window-open handler, +// will-navigate/will-redirect guards) to a popup window, and recurses via +// `did-create-window` so every descendant popup (a popup opened from a +// popup, and so on) gets the same wiring — a grandchild can't slip through +// unpoliced just because it wasn't opened directly from the webview. +const attachConferencePopupPolicy = (childWindow: BrowserWindow): void => { + childWindow.once('ready-to-show', () => { + childWindow.show(); + }); + childWindow.webContents.setWindowOpenHandler( + handleConferenceWebviewWindowOpen + ); + childWindow.webContents.on('will-navigate', handleConferenceWillNavigate); + childWindow.webContents.on('will-redirect', handleConferenceWillNavigate); + childWindow.webContents.on('did-create-window', attachConferencePopupPolicy); +}; + const setupWebviewHandlers = (webContents: WebContents) => { // Track attached webviews that need handler setup const pendingWebviews: WebContents[] = []; @@ -491,19 +508,10 @@ const setupWebviewHandlers = (webContents: WebContents) => { webviewWebContents.on('will-redirect', 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); - childWindow.webContents.on('will-redirect', handleConferenceWillNavigate); - }); + // to avoid a white flash, and the same policy is applied to every + // descendant popup (a popup opened from this popup, and so on) via + // attachConferencePopupPolicy's own did-create-window registration. + webviewWebContents.on('did-create-window', attachConferencePopupPolicy); // Media (mic/cam) permission requests from the conference originate in the // webview's session, NOT the host window's, so the handler must live on the diff --git a/src/videoCallWindow/main/ipc.main.spec.ts b/src/videoCallWindow/main/ipc.main.spec.ts index bedba7f7ea..2738dcff67 100644 --- a/src/videoCallWindow/main/ipc.main.spec.ts +++ b/src/videoCallWindow/main/ipc.main.spec.ts @@ -1073,7 +1073,7 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => { expect(httpEvent.preventDefault).not.toHaveBeenCalled(); }); - it('registers a did-create-window handler that wires the popup child window with the guest policy', async () => { + it('registers a did-create-window handler that wires the popup child window with the guest policy, recursively for descendant popups', async () => { const guest = await attachGuest(); expect(guest.on).toHaveBeenCalledWith( 'did-create-window', @@ -1083,14 +1083,16 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => { ([event]: [string]) => event === 'did-create-window' )[1]; - const childWindow = { + const makeFakeChild = () => ({ once: jest.fn(), show: jest.fn(), webContents: { setWindowOpenHandler: jest.fn(), on: jest.fn(), }, - }; + }); + + const childWindow = makeFakeChild(); didCreateWindowHandler(childWindow, { url: 'https://idp.example/sso' }); @@ -1105,6 +1107,10 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => { 'will-redirect', expect.any(Function) ); + expect(childWindow.webContents.on).toHaveBeenCalledWith( + 'did-create-window', + expect.any(Function) + ); expect(childWindow.once).toHaveBeenCalledWith( 'ready-to-show', @@ -1116,6 +1122,47 @@ describe('videoCallWindow/ipc — PR #3359 hardening', () => { expect(childWindow.show).not.toHaveBeenCalled(); readyToShowCallback(); expect(childWindow.show).toHaveBeenCalledTimes(1); + + // A popup opened FROM the child (a grandchild) must get the same + // wiring — the recursive did-create-window registration above is what + // makes that happen. + const childDidCreateWindowHandler = + childWindow.webContents.on.mock.calls.find( + ([event]: [string]) => event === 'did-create-window' + )[1]; + + const grandchildWindow = makeFakeChild(); + childDidCreateWindowHandler(grandchildWindow, { + url: 'https://idp.example/sso/nested', + }); + + expect( + grandchildWindow.webContents.setWindowOpenHandler + ).toHaveBeenCalledTimes(1); + expect(grandchildWindow.webContents.on).toHaveBeenCalledWith( + 'will-navigate', + expect.any(Function) + ); + expect(grandchildWindow.webContents.on).toHaveBeenCalledWith( + 'will-redirect', + expect.any(Function) + ); + expect(grandchildWindow.webContents.on).toHaveBeenCalledWith( + 'did-create-window', + expect.any(Function) + ); + + expect(grandchildWindow.once).toHaveBeenCalledWith( + 'ready-to-show', + expect.any(Function) + ); + const grandchildReadyToShowCallback = + grandchildWindow.once.mock.calls.find( + ([event]: [string]) => event === 'ready-to-show' + )[1]; + expect(grandchildWindow.show).not.toHaveBeenCalled(); + grandchildReadyToShowCallback(); + expect(grandchildWindow.show).toHaveBeenCalledTimes(1); }); it('leaves the host window popup policy unchanged: http(s) still denied + sent to the system browser', async () => {