diff --git a/src/main/browser/browser-webauthn-account-picker.test.ts b/src/main/browser/browser-webauthn-account-picker.test.ts index 3e9e370b807..f930438ccec 100644 --- a/src/main/browser/browser-webauthn-account-picker.test.ts +++ b/src/main/browser/browser-webauthn-account-picker.test.ts @@ -163,4 +163,84 @@ describe('browser WebAuthn account picker', () => { renderer.emit('render-process-gone') await expect(windowCrashed).resolves.toBeNull() }) + + it.each(['guest', 'renderer'] as const)( + 'retires the request when the %s commits a navigation', + async (target) => { + const owners = { guest: mockWebContents(91), renderer: mockWebContents(92) } + fromFrameMock.mockReturnValue(owners.guest) + getRendererContextForGuestMock.mockReturnValue({ + browserPageId: 'page-6', + renderer: owners.renderer + }) + + const selection = requestBrowserWebAuthnAccount(accountDetails(), mockSession()) + const requestId = vi.mocked(owners.renderer.send).mock.calls[0][1].requestId + owners[target].emit('did-navigate') + + await expect(selection).resolves.toBeNull() + expect(owners.renderer.send).toHaveBeenLastCalledWith( + 'browser:webauthn-account-request-closed', + { requestId } + ) + expect( + respondToBrowserWebAuthnAccountRequest(owners.renderer, { + requestId, + credentialId: 'credential-1' + }) + ).toBe(false) + for (const contents of Object.values(owners)) { + for (const event of ['did-navigate', 'render-process-gone', 'destroyed']) { + expect(contents.listenerCount(event)).toBe(0) + } + } + expect(vi.getTimerCount()).toBe(0) + } + ) + + it('keeps sibling and successor requests independent of a retired document', async () => { + const retiredGuest = mockWebContents(101) + const siblingGuest = mockWebContents(102) + const renderer = mockWebContents(103) + getRendererContextForGuestMock.mockImplementation((guestId: number) => ({ + browserPageId: `page-${guestId}`, + renderer + })) + const browserSession = mockSession() + const requestIdAt = (index: number): string => + vi.mocked(renderer.send).mock.calls[index][1].requestId + + fromFrameMock.mockReturnValueOnce(retiredGuest) + const retired = requestBrowserWebAuthnAccount(accountDetails(), browserSession) + fromFrameMock.mockReturnValueOnce(siblingGuest) + const sibling = requestBrowserWebAuthnAccount(accountDetails(), browserSession) + const [retiredId, siblingId] = [requestIdAt(0), requestIdAt(1)] + + retiredGuest.emit('did-navigate') + await expect(retired).resolves.toBeNull() + fromFrameMock.mockReturnValueOnce(retiredGuest) + const successor = requestBrowserWebAuthnAccount(accountDetails(), browserSession) + const successorId = requestIdAt(3) + + expect( + respondToBrowserWebAuthnAccountRequest(renderer, { + requestId: retiredId, + credentialId: 'credential-1' + }) + ).toBe(false) + expect( + respondToBrowserWebAuthnAccountRequest(renderer, { + requestId: successorId, + credentialId: 'credential-2' + }) + ).toBe(true) + expect( + respondToBrowserWebAuthnAccountRequest(renderer, { + requestId: siblingId, + credentialId: 'credential-1' + }) + ).toBe(true) + await expect(successor).resolves.toBe('credential-2') + await expect(sibling).resolves.toBe('credential-1') + }) }) diff --git a/src/main/browser/browser-webauthn-account-picker.ts b/src/main/browser/browser-webauthn-account-picker.ts index 522630e6851..b0807f7a305 100644 --- a/src/main/browser/browser-webauthn-account-picker.ts +++ b/src/main/browser/browser-webauthn-account-picker.ts @@ -1,5 +1,6 @@ import { randomUUID } from 'node:crypto' import { webContents } from 'electron' +import { abortWhenRendererGone } from '../ipc/renderer-lifetime-abort' import type { BrowserWebAuthnAccountRequest, @@ -12,11 +13,10 @@ export const BROWSER_WEBAUTHN_ACCOUNT_PICKER_TIMEOUT_MS = 60_000 type PendingAccountRequest = { browserPageId: string browserSession: Electron.Session - guest: Electron.WebContents renderer: Electron.WebContents credentialIds: Set timer: ReturnType - onUnavailable: () => void + dispose: () => void settle: (credentialId: string | null) => void } @@ -34,10 +34,7 @@ function closeRendererPrompt(requestId: string, request: PendingAccountRequest): function cleanUpRequest(requestId: string, request: PendingAccountRequest): void { clearTimeout(request.timer) - request.guest.removeListener('destroyed', request.onUnavailable) - request.guest.removeListener('render-process-gone', request.onUnavailable) - request.renderer.removeListener('destroyed', request.onUnavailable) - request.renderer.removeListener('render-process-gone', request.onUnavailable) + request.dispose() pendingAccountRequests.delete(requestId) } @@ -80,21 +77,30 @@ export function requestBrowserWebAuthnAccount( } resolve(credentialId) } + const lifetimes = [abortWhenRendererGone(guest), abortWhenRendererGone(context.renderer)] + const onUnavailable = (): void => settle(null) const request: PendingAccountRequest = { browserPageId: context.browserPageId, browserSession, - guest, renderer: context.renderer, credentialIds: new Set(details.accounts.map((account) => account.credentialId)), timer: setTimeout(settle, BROWSER_WEBAUTHN_ACCOUNT_PICKER_TIMEOUT_MS), - onUnavailable: () => settle(null), + dispose: () => { + for (const lifetime of lifetimes) { + lifetime.signal.removeEventListener('abort', onUnavailable) + lifetime.dispose() + } + }, settle } pendingAccountRequests.set(requestId, request) - guest.once('destroyed', request.onUnavailable) - guest.once('render-process-gone', request.onUnavailable) - context.renderer.once('destroyed', request.onUnavailable) - context.renderer.once('render-process-gone', request.onUnavailable) + for (const lifetime of lifetimes) { + lifetime.signal.addEventListener('abort', onUnavailable, { once: true }) + } + if (lifetimes.some(({ signal }) => signal.aborted)) { + settle(null) + return + } try { context.renderer.send('browser:webauthn-account-requested', requestPayload) } catch {