From 93354291d18daaacbc05a751fe90fc7d644c6a18 Mon Sep 17 00:00:00 2001 From: Gideon Date: Thu, 20 Aug 2026 05:06:54 +0900 Subject: [PATCH] fix(browser): stop the auth-host UA write from cancelling redirects (#15221) * fix(browser): stop the auth-host UA write from cancelling redirects WebContents.setUserAgent() from will-redirect makes Chromium abort the in-flight navigation (ERR_ABORTED) and replay the original request, so any redirect crossing the Google auth-host boundary dies on a blank tab. Route that write through the CDP override, which retargets navigator.userAgent without touching the navigation. Fixes #15216 * fix(browser): recover auth UA override state * fix(browser): keep the viewport UA override on the session identity The auth-host switch writes the Firefox UA through WebContents.setUserAgent on a direct navigation and then moves to the CDP override, and nothing restores that WebContents UA. sendViewportUserAgentOverride read it back as its base identity, so a tab with a viewport preset republished the Firefox UA on every ordinary host afterwards, with sec-ch-ua still saying Chrome. Read the profile's session UA instead, which is the stable base identity at both call sites. --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> (cherry picked from commit 0b8107a41f0d38f62835ea1a4fd5c0794768c12e) --- .../browser-manager-auth-user-agent.test.ts | 321 +++++++++++++++++- .../browser-manager-viewport-override.test.ts | 139 ++++++-- src/main/browser/browser-manager.ts | 159 ++++++++- 3 files changed, 569 insertions(+), 50 deletions(-) diff --git a/src/main/browser/browser-manager-auth-user-agent.test.ts b/src/main/browser/browser-manager-auth-user-agent.test.ts index 79f24c83de8..405b689e850 100644 --- a/src/main/browser/browser-manager-auth-user-agent.test.ts +++ b/src/main/browser/browser-manager-auth-user-agent.test.ts @@ -48,6 +48,11 @@ import { resetBrowserManagerMocks, resetBrowserManagerState } from './browser-manager-test-harness' +import { + createViewportGuestFactory, + flushViewportOps, + GUEST_CLEAN_UA +} from './browser-manager-viewport-test-fixtures' const { guestOffMock, @@ -57,6 +62,7 @@ const { guestOpenDevToolsMock, webContentsFromIdMock } = browserMocks +const makeViewportGuest = createViewportGuestFactory(browserMocks) describe('browserManager', () => { beforeEach(() => { @@ -68,8 +74,9 @@ describe('browserManager', () => { vi.useRealTimers() }) - it('presents the Firefox UA on Google auth hosts and restores the base UA off them', () => { + it('presents the Firefox UA on Google auth hosts and restores the base UA off them', async () => { let currentUa = guestBaseUserAgent + const sendCommand = vi.fn().mockResolvedValue(undefined) const setUserAgent = vi.fn((ua: string) => { currentUa = ua }) @@ -85,6 +92,7 @@ describe('browserManager', () => { getURL: vi.fn(() => 'https://accounts.google.com/'), getUserAgent: vi.fn(() => currentUa), setUserAgent, + debugger: { isAttached: vi.fn(() => true), sendCommand }, session: { getUserAgent: vi.fn(() => guestBaseUserAgent) } } webContentsFromIdMock.mockReturnValue(guest) @@ -98,14 +106,26 @@ describe('browserManager', () => { const didStartNavigation = guestOnMock.mock.calls.find( ([event]) => event === 'did-start-navigation' )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + const willRedirect = guestOnMock.mock.calls.find( + ([event]) => event === 'will-redirect' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void setUserAgent.mockClear() didStartNavigation(null, 'https://accounts.google.com/v3/signin/identifier', false, true) expect(setUserAgent).toHaveBeenLastCalledWith(googleAuthUserAgent()) - // Off the auth host, the guest's base identity is restored. - didStartNavigation(null, 'https://myaccount.google.com/', false, true) - expect(setUserAgent).toHaveBeenLastCalledWith(guestBaseUserAgent) + // A redirect off the auth host must derive the CDP write from the session UA, not the stale + // Firefox WebContents UA installed by the direct navigation. + sendCommand.mockClear() + willRedirect(null, 'https://myaccount.google.com/', false, true) + const uaOverrideIndex = sendCommand.mock.calls.findIndex( + ([method]) => method === 'Emulation.setUserAgentOverride' + ) + await expect(sendCommand.mock.results[uaOverrideIndex]?.value).resolves.toBeUndefined() + expect(sendCommand.mock.calls[uaOverrideIndex]).toEqual([ + 'Emulation.setUserAgentOverride', + { userAgent: guestBaseUserAgent } + ]) // A navigation that doesn't change the required UA must not thrash setUserAgent. setUserAgent.mockClear() @@ -233,4 +253,297 @@ describe('browserManager', () => { popupDidStartNavigation(null, 'https://accounts.google.com/v3/signin/identifier', false, true) expect(popupSetUserAgent).not.toHaveBeenCalled() }) + + // Why: WebContents.setUserAgent() from will-redirect makes Chromium cancel the in-flight navigation + // (ERR_ABORTED) and replay the original request. A "Sign in with Google" button POSTs to the + // provider and lands on accounts.google.com only by redirect, so the replay never reproduces it and + // the tab is left blank. Verified against Electron 43.1.0: the server sees the original request twice. + it('retargets the auth-host UA over CDP during a redirect instead of cancelling it', async () => { + const debuggerHandlers = new Map void>() + let holdNextUserAgentOverride = false + let releaseHeldUserAgentOverride = (): void => {} + let rejectNextUserAgentOverride = false + const sendCommand = vi.fn((method: string) => { + if (method === 'Emulation.setUserAgentOverride' && holdNextUserAgentOverride) { + holdNextUserAgentOverride = false + return new Promise((resolve) => { + releaseHeldUserAgentOverride = resolve + }) + } + if (method === 'Emulation.setUserAgentOverride' && rejectNextUserAgentOverride) { + rejectNextUserAgentOverride = false + return Promise.reject(new Error('debugger detached')) + } + return Promise.resolve(undefined) + }) + const guest = { + id: 418, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: guestSetBackgroundThrottlingMock, + setWindowOpenHandler: guestSetWindowOpenHandlerMock, + on: guestOnMock, + off: guestOffMock, + openDevTools: guestOpenDevToolsMock, + getURL: vi.fn(() => 'https://mail.google.com/'), + getUserAgent: vi.fn(() => guestBaseUserAgent), + setUserAgent: vi.fn(), + debugger: { + isAttached: vi.fn(() => true), + sendCommand, + on: vi.fn((event: string, handler: () => void) => debuggerHandlers.set(event, handler)), + off: vi.fn((event: string) => debuggerHandlers.delete(event)) + }, + session: { getUserAgent: vi.fn(() => guestBaseUserAgent) } + } + webContentsFromIdMock.mockReturnValue(guest) + + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'browser-redirect-ua', + webContentsId: guest.id, + rendererWebContentsId + }) + const willRedirect = guestOnMock.mock.calls.find( + ([event]) => event === 'will-redirect' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + sendCommand.mockClear() + + rejectNextUserAgentOverride = true + willRedirect( + { preventDefault: vi.fn() }, + 'https://accounts.google.com/v3/signin/identifier', + false, + true + ) + await expect(sendCommand.mock.results.at(-1)?.value).rejects.toThrow('debugger detached') + + // A rejected write must not be recorded as installed; the next navigation retries it. + willRedirect( + { preventDefault: vi.fn() }, + 'https://accounts.google.com/v3/signin/identifier', + false, + true + ) + await expect(sendCommand.mock.results.at(-1)?.value).resolves.toBeUndefined() + + expect(guest.setUserAgent).not.toHaveBeenCalled() + expect(sendCommand).toHaveBeenCalledWith('Emulation.setUserAgentOverride', { + userAgent: googleAuthUserAgent() + }) + + // Detaching clears Chromium's CDP overrides, so the confirmed identity must be invalidated too. + sendCommand.mockClear() + debuggerHandlers.get('detach')?.() + willRedirect( + { preventDefault: vi.fn() }, + 'https://accounts.google.com/v3/signin/identifier', + false, + true + ) + expect(sendCommand).toHaveBeenCalledWith('Emulation.setUserAgentOverride', { + userAgent: googleAuthUserAgent() + }) + + // Why: the CDP override outranks the WebContents UA, so getUserAgent() still reports the base + // identity. Leaving the auth host must read the override, not that stale value, or the guest + // keeps presenting Firefox on every later host. + sendCommand.mockClear() + const didStartNavigation = guestOnMock.mock.calls.find( + ([event]) => event === 'did-start-navigation' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + rejectNextUserAgentOverride = true + didStartNavigation(null, 'https://example.com/', false, true) + await expect(sendCommand.mock.results.at(-1)?.value).rejects.toThrow('debugger detached') + didStartNavigation(null, 'https://example.com/', false, true) + + expect(guest.setUserAgent).not.toHaveBeenCalled() + expect(sendCommand.mock.calls).toEqual([ + ['Emulation.setUserAgentOverride', { userAgent: guestBaseUserAgent }], + ['Emulation.setUserAgentOverride', { userAgent: guestBaseUserAgent }] + ]) + + // A newer confirmed write outranks an older write that remains in flight. + debuggerHandlers.get('detach')?.() + holdNextUserAgentOverride = true + willRedirect( + { preventDefault: vi.fn() }, + 'https://accounts.google.com/v3/signin/identifier', + false, + true + ) + const olderHeldWrite = sendCommand.mock.results.at(-1)?.value as Promise + didStartNavigation(null, 'https://example.com/', false, true) + await expect(sendCommand.mock.results.at(-1)?.value).resolves.toBeUndefined() + + sendCommand.mockClear() + willRedirect( + { preventDefault: vi.fn() }, + 'https://accounts.google.com/v3/signin/identifier', + false, + true + ) + expect(sendCommand).toHaveBeenCalledWith('Emulation.setUserAgentOverride', { + userAgent: googleAuthUserAgent() + }) + releaseHeldUserAgentOverride() + await olderHeldWrite + + // A newer failure must not discard an older write that is still in flight. + debuggerHandlers.get('detach')?.() + holdNextUserAgentOverride = true + willRedirect( + { preventDefault: vi.fn() }, + 'https://accounts.google.com/v3/signin/identifier', + false, + true + ) + const heldWrite = sendCommand.mock.results.at(-1)?.value as Promise + rejectNextUserAgentOverride = true + didStartNavigation(null, 'https://example.com/', false, true) + await expect(sendCommand.mock.results.at(-1)?.value).rejects.toThrow('debugger detached') + releaseHeldUserAgentOverride() + await heldWrite + + sendCommand.mockClear() + didStartNavigation(null, 'https://example.com/', false, true) + expect(sendCommand).toHaveBeenCalledWith('Emulation.setUserAgentOverride', { + userAgent: guestBaseUserAgent + }) + browserManager.unregisterGuest('browser-redirect-ua') + }) + + // Why: a redirect must never be cancelled to keep the identity in sync. Without a debugger there is + // no cancel-free write available, and a stale navigator.userAgent is recoverable where a dead + // navigation is not. + it('leaves a redirect alone when no debugger is attached to write the UA', () => { + const guest = { + id: 419, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: guestSetBackgroundThrottlingMock, + setWindowOpenHandler: guestSetWindowOpenHandlerMock, + on: guestOnMock, + off: guestOffMock, + openDevTools: guestOpenDevToolsMock, + getURL: vi.fn(() => 'https://mail.google.com/'), + getUserAgent: vi.fn(() => guestBaseUserAgent), + setUserAgent: vi.fn(), + debugger: { isAttached: vi.fn(() => false), sendCommand: vi.fn() }, + session: { getUserAgent: vi.fn(() => guestBaseUserAgent) } + } + webContentsFromIdMock.mockReturnValue(guest) + + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'browser-redirect-no-debugger', + webContentsId: guest.id, + rendererWebContentsId + }) + const willRedirect = guestOnMock.mock.calls.find( + ([event]) => event === 'will-redirect' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + + willRedirect( + { preventDefault: vi.fn() }, + 'https://accounts.google.com/v3/signin/identifier', + false, + true + ) + + expect(guest.setUserAgent).not.toHaveBeenCalled() + expect(guest.debugger.sendCommand).not.toHaveBeenCalledWith( + 'Emulation.setUserAgentOverride', + expect.anything() + ) + }) + + // Why: a direct load reaches did-start-navigation before the request is dispatched, so the + // WebContents write is safe there and must stay — CDP is the redirect-path mechanism only. + it('still uses the WebContents UA write for navigations that are not redirects', () => { + const guest = { + id: 420, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: guestSetBackgroundThrottlingMock, + setWindowOpenHandler: guestSetWindowOpenHandlerMock, + on: guestOnMock, + off: guestOffMock, + openDevTools: guestOpenDevToolsMock, + getURL: vi.fn(() => 'https://example.com/'), + getUserAgent: vi.fn(() => guestBaseUserAgent), + setUserAgent: vi.fn(), + debugger: { isAttached: vi.fn(() => true), sendCommand: vi.fn(async () => undefined) }, + session: { getUserAgent: vi.fn(() => guestBaseUserAgent) } + } + webContentsFromIdMock.mockReturnValue(guest) + + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'browser-direct-ua', + webContentsId: guest.id, + rendererWebContentsId + }) + const didStartNavigation = guestOnMock.mock.calls.find( + ([event]) => event === 'did-start-navigation' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + + didStartNavigation(null, 'https://accounts.google.com/v3/signin/identifier', false, true) + + expect(guest.setUserAgent).toHaveBeenLastCalledWith(googleAuthUserAgent()) + expect(guest.debugger.sendCommand).not.toHaveBeenCalledWith( + 'Emulation.setUserAgentOverride', + expect.anything() + ) + }) + + // Why: the direct-navigation branch still writes the Firefox UA through WebContents.setUserAgent, + // and nothing ever restores it once the guest switches to the CDP override. A viewport preset that + // read getUserAgent() back as its base identity would therefore republish Firefox on every ordinary + // host — the wire UA saying Firefox while sec-ch-ua still says Chrome, the exact cross-layer tell + // this scope exists to remove. + it('keeps a viewport preset on the session identity after an auth-host visit', async () => { + const { guest, debuggerSendCommand } = makeViewportGuest(9001) + webContentsFromIdMock.mockReturnValue(guest) + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'tab-auth-then-preset', + webContentsId: guest.id as number, + rendererWebContentsId + }) + await browserManager.setViewportOverride('tab-auth-then-preset', { + width: 1280, + height: 800, + deviceScaleFactor: 1, + mobile: false + }) + await flushViewportOps() + + const didStartNavigation = guestOnMock.mock.calls.find( + ([event]) => event === 'did-start-navigation' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + const willRedirect = guestOnMock.mock.calls.find( + ([event]) => event === 'will-redirect' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + + didStartNavigation(null, 'https://accounts.google.com/v3/signin/identifier', false, true) + await flushViewportOps() + // The direct branch pins the WebContents UA to Firefox and never restores it. + expect((guest.getUserAgent as () => string)()).toBe(googleAuthUserAgent()) + + willRedirect({ preventDefault: vi.fn() }, 'https://myaccount.google.com/', false, true) + await flushViewportOps() + + debuggerSendCommand.mockClear() + didStartNavigation(null, 'https://github.com/', false, true) + await flushViewportOps() + + const uaWrites = debuggerSendCommand.mock.calls.filter( + ([method]) => method === 'Emulation.setUserAgentOverride' + ) + expect(uaWrites.length).toBeGreaterThan(0) + for (const [, params] of uaWrites) { + expect((params as { userAgent: string }).userAgent).toBe(GUEST_CLEAN_UA) + } + }) }) diff --git a/src/main/browser/browser-manager-viewport-override.test.ts b/src/main/browser/browser-manager-viewport-override.test.ts index f4d09a913a3..c72936cf6a3 100644 --- a/src/main/browser/browser-manager-viewport-override.test.ts +++ b/src/main/browser/browser-manager-viewport-override.test.ts @@ -55,6 +55,12 @@ import { const { guestOnMock, webContentsFromIdMock } = browserMocks const makeGuest = createViewportGuestFactory(browserMocks) +const MOBILE_VIEWPORT_OVERRIDE = { + width: 375, + height: 667, + deviceScaleFactor: 2, + mobile: true +} as const describe('browserManager', () => { beforeEach(() => { @@ -68,12 +74,7 @@ describe('browserManager', () => { describe('setViewportOverride', () => { it('returns false when the tab is not registered', async () => { - const result = await browserManager.setViewportOverride('missing', { - width: 375, - height: 667, - deviceScaleFactor: 2, - mobile: true - }) + const result = await browserManager.setViewportOverride('missing', MOBILE_VIEWPORT_OVERRIDE) expect(result).toBe(false) }) @@ -89,12 +90,7 @@ describe('browserManager', () => { webContentsFromIdMock.mockReset() webContentsFromIdMock.mockReturnValue(guest) - const ok = await browserManager.setViewportOverride('tab-mobile', { - width: 375, - height: 667, - deviceScaleFactor: 2, - mobile: true - }) + const ok = await browserManager.setViewportOverride('tab-mobile', MOBILE_VIEWPORT_OVERRIDE) expect(ok).toBe(true) expect(debuggerSendCommand).toHaveBeenCalledWith('Emulation.setDeviceMetricsOverride', { @@ -194,6 +190,14 @@ describe('browserManager', () => { const didStartNavigation = guestOnMock.mock.calls.find( ([event]) => event === 'did-start-navigation' )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + const willRedirect = guestOnMock.mock.calls.find( + ([event]) => event === 'will-redirect' + )?.[1] as ( + event: { preventDefault: () => void }, + url: string, + isInPlace: boolean, + isMainFrame: boolean + ) => void // Case A: the desktop preset lands first, while the tab is still off the auth host. await browserManager.setViewportOverride('tab-auth-nav', { @@ -216,11 +220,9 @@ describe('browserManager', () => { // Leaving the auth host restores the clean Chrome-shaped preset UA. debuggerSendCommand.mockClear() - didStartNavigation(null, 'https://example.com/', false, true) + willRedirect({ preventDefault: vi.fn() }, 'https://example.com/', false, true) await flushViewportOps() - expect(debuggerSendCommand).toHaveBeenCalledWith('Emulation.setUserAgentOverride', { - userAgent: GUEST_CLEAN_UA - }) + expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({ userAgent: GUEST_CLEAN_UA }) }) // Why: not an ordering race — debugger.sendCommand dispatches in call order over one channel, so @@ -263,12 +265,10 @@ describe('browserManager', () => { ([event]) => event === 'did-start-navigation' )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void - const presetDone = browserManager.setViewportOverride('tab-race-onto-auth', { - width: 375, - height: 667, - deviceScaleFactor: 2, - mobile: true - }) + const presetDone = browserManager.setViewportOverride( + 'tab-race-onto-auth', + MOBILE_VIEWPORT_OVERRIDE + ) await flushViewportOps() // The tab navigates to the auth host while the preset is still awaiting its first command. @@ -382,12 +382,7 @@ describe('browserManager', () => { // A later preset must also resolve the committed, non-auth URL. debuggerSendCommand.mockClear() - await browserManager.setViewportOverride('tab-pending-cleared', { - width: 375, - height: 667, - deviceScaleFactor: 2, - mobile: true - }) + await browserManager.setViewportOverride('tab-pending-cleared', MOBILE_VIEWPORT_OVERRIDE) await flushViewportOps() expect(lastUserAgentOverride(debuggerSendCommand)?.userAgent).toContain('iPhone') @@ -450,12 +445,16 @@ describe('browserManager', () => { willRedirect({ preventDefault: vi.fn() }, 'https://accounts.google.com/same', false, true) didStartNavigation(null, 'https://accounts.google.com/same', false, true) await flushViewportOps() + expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({ + userAgent: googleAuthUserAgent() + }) debuggerSendCommand.mockClear() didFailLoad(null, -3, 'Aborted', 'https://accounts.google.com/same', true) await flushViewportOps() - expect(guest.setUserAgent).toHaveBeenLastCalledWith(googleAuthUserAgent()) + // Why: the redirect path never writes the WebContents UA — that write cancels the navigation. + expect(guest.setUserAgent).not.toHaveBeenCalled() expect(debuggerSendCommand).not.toHaveBeenCalledWith( 'Emulation.setUserAgentOverride', expect.objectContaining({ userAgent: GUEST_CLEAN_UA }) @@ -507,14 +506,92 @@ describe('browserManager', () => { ) await flushViewportOps() - expect(guest.setUserAgent).toHaveBeenLastCalledWith(googleAuthUserAgent()) + // Why: the identity switch for a redirect goes over CDP only. WebContents.setUserAgent() here + // makes Chromium cancel the in-flight navigation and replay the original request, which is what + // left Google sign-in on a blank tab. + expect(guest.setUserAgent).not.toHaveBeenCalled() expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({ userAgent: googleAuthUserAgent() }) didFailLoad(null, -3, 'Aborted', 'https://accounts.google.com/redirected', true) await flushViewportOps() - expect(guest.setUserAgent).toHaveBeenLastCalledWith(GUEST_ELECTRON_UA) + expect(guest.setUserAgent).not.toHaveBeenCalled() + expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({ userAgent: GUEST_CLEAN_UA }) + }) + + it('preserves the auth identity when a viewport preset is cleared after a redirect', async () => { + const { guest, debuggerSendCommand } = makeGuest(4262, 'https://example.com/') + webContentsFromIdMock.mockReturnValue(guest) + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'tab-clear-on-auth', + webContentsId: guest.id as number, + rendererWebContentsId + }) + const didStartNavigation = guestOnMock.mock.calls.find( + ([event]) => event === 'did-start-navigation' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + const willRedirect = guestOnMock.mock.calls.find( + ([event]) => event === 'will-redirect' + )?.[1] as ( + event: { preventDefault: () => void }, + url: string, + isInPlace: boolean, + isMainFrame: boolean + ) => void + + await browserManager.setViewportOverride('tab-clear-on-auth', MOBILE_VIEWPORT_OVERRIDE) + didStartNavigation(null, 'https://example.com/start', false, true) + willRedirect({ preventDefault: vi.fn() }, 'https://accounts.google.com/', false, true) + await flushViewportOps() + + debuggerSendCommand.mockClear() + await expect(browserManager.setViewportOverride('tab-clear-on-auth', null)).resolves.toBe( + true + ) + expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({ + userAgent: googleAuthUserAgent() + }) + expect(debuggerSendCommand).not.toHaveBeenCalledWith('Emulation.setUserAgentOverride', { + userAgent: '' + }) + }) + + it('does not inherit a mobile owner UA in a desktop popup', async () => { + const { guest: ownerGuest } = makeGuest(4263) + webContentsFromIdMock.mockReturnValue(ownerGuest) + browserManager.attachGuestPolicies(ownerGuest as never) + browserManager.registerGuest({ + browserPageId: 'tab-mobile-popup-owner', + webContentsId: ownerGuest.id as number, + rendererWebContentsId + }) + await browserManager.setViewportOverride('tab-mobile-popup-owner', MOBILE_VIEWPORT_OVERRIDE) + + const { guest: popupGuest, debuggerSendCommand } = makeGuest(4264, 'https://mail.google.com/') + const popupOn = vi.fn() + popupGuest.on = popupOn + browserManager.attachGuestPolicies(popupGuest as never, { + browserTabId: 'tab-mobile-popup-owner', + rootGuestWebContentsId: ownerGuest.id as number + }) + const willRedirect = popupOn.mock.calls.find(([event]) => event === 'will-redirect')?.[1] as ( + event: { preventDefault: () => void }, + url: string, + isInPlace: boolean, + isMainFrame: boolean + ) => void + const didStartNavigation = popupOn.mock.calls.find( + ([event]) => event === 'did-start-navigation' + )?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void + + willRedirect({ preventDefault: vi.fn() }, 'https://accounts.google.com/', false, true) + await flushViewportOps() + debuggerSendCommand.mockClear() + didStartNavigation(null, 'https://example.com/', false, true) + await flushViewportOps() + expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({ userAgent: GUEST_CLEAN_UA }) }) diff --git a/src/main/browser/browser-manager.ts b/src/main/browser/browser-manager.ts index abbd79bcf4f..2a8360af946 100644 --- a/src/main/browser/browser-manager.ts +++ b/src/main/browser/browser-manager.ts @@ -153,6 +153,15 @@ type PendingMainFrameNavigation = { currentUrl: string supersededUrls: string[] } +type AuthUserAgentOverrideOperation = { + sequence: number + userAgent: string +} +type AuthUserAgentOverrideState = { + confirmed: AuthUserAgentOverrideOperation | null + nextSequence: number + pending: AuthUserAgentOverrideOperation[] +} const SAFE_POPUP_WINDOW_OPTIONS = { alwaysOnTop: false, closable: true, @@ -229,6 +238,12 @@ export class BrowserManager { // Why: presence means the preset requires a CDP UA override (installed or in flight), so navigation // can re-issue it against the target URL's identity. private readonly viewportUaOverrideMobileByTabId = new Map() + // Why: the confirmed CDP identity outranks getUserAgent; pending intent keeps rapid navigations + // ordered without claiming a failed write was installed. + private readonly authUserAgentOverrideStateByGuestId = new Map< + number, + AuthUserAgentOverrideState + >() // Why: the in-flight main-frame navigation target, held only until commit or failure — getURL() // still reports the outgoing page until then. See resolveTabNavigationUrl. private readonly pendingNavigationByGuestId = new Map() @@ -315,6 +330,7 @@ export class BrowserManager { // Why: proxy/bridge stop detaches the debugger and drops injections; re-attach (500ms delay to avoid racing a mid-restart) to keep overrides. const onDetach = (): void => { + this.authUserAgentOverrideStateByGuestId.delete(guest.id) if (!disposed && !guest.isDestroyed() && reattachTimer === null) { reattachTimer = setTimeout(() => { reattachTimer = null @@ -810,7 +826,7 @@ export class BrowserManager { return } this.updatePendingNavigationForRedirect(guest.id, url) - this.applyGoogleAuthUserAgent(guest, url) + this.applyGoogleAuthUserAgent(guest, url, { duringRedirect: true }) } const didFailLoadHandler = ( @@ -946,7 +962,11 @@ export class BrowserManager { // must be matched here per navigation or the two layers disagree — itself a bot tell. // Restores the session's base identity off the auth hosts. Native-UA profiles opt out // of the whole clean-UA path, so they keep their untouched identity everywhere. - private applyGoogleAuthUserAgent(guest: Electron.WebContents, url: string): void { + private applyGoogleAuthUserAgent( + guest: Electron.WebContents, + url: string, + options: { duringRedirect?: boolean } = {} + ): void { const browserPageId = this.tabIdByWebContentsId.get(guest.id) // Why: popup child windows get these policies but are never in tabIdByWebContentsId, so a direct // lookup misses the native-UA opt-out and would hand a native profile's popup the Firefox UA. @@ -961,23 +981,113 @@ export class BrowserManager { return } const firefoxUa = googleAuthUserAgent() - const currentUa = guest.getUserAgent() - if (isGoogleAuthUrl(url)) { - if (currentUa !== firefoxUa) { - guest.setUserAgent(firefoxUa) + const overrideState = this.authUserAgentOverrideStateByGuestId.get(guest.id) + const latestPendingOverride = overrideState?.pending.at(-1) + const confirmedOverride = overrideState?.confirmed + const currentOverride = + latestPendingOverride && latestPendingOverride.sequence > (confirmedOverride?.sequence ?? -1) + ? latestPendingOverride + : confirmedOverride + const currentUa = currentOverride?.userAgent ?? guest.getUserAgent() + const nextUa = isGoogleAuthUrl(url) + ? firefoxUa + : // Only restore when the auth-host override is actually in place, so normal + // navigation never touches the session UA. + currentUa === firefoxUa + ? guest.session.getUserAgent() + : null + let authOverrideIssuedOverCdp = false + if (nextUa !== null && nextUa !== currentUa) { + // Why: WebContents.setUserAgent() during a redirect makes Chromium cancel the in-flight + // navigation (ERR_ABORTED) and replay the original request, which a POST-started OAuth chain + // cannot survive — the sign-in lands on a blank tab. CDP retargets navigator.userAgent without + // touching the navigation, and it outranks the WebContents UA from then on, so a guest that + // switches to it stays on it. The wire UA never depended on this write: setupClientHintsOverride + // rewrites User-Agent per request for auth-host URLs on its own. + if (options.duringRedirect === true || overrideState !== undefined) { + if (this.canOverrideUserAgentOverCdp(guest)) { + authOverrideIssuedOverCdp = true + // Why: go through the viewport builder rather than writing nextUa raw, so both CDP writers + // resolve one identity for this URL — Firefox on auth hosts, the profile's clean base off + // them, any mobile preset preserved. Writing the session UA directly would put the + // unlaundered Electron token back on the wire. + void this.applyAuthUserAgentOverrideOverCdp( + guest, + (browserPageId ? this.viewportUaOverrideMobileByTabId.get(browserPageId) : undefined) ?? + false, + url, + nextUa + ) + } + // Why: with no debugger there is no way to retarget the identity without cancelling the + // redirect. A stale navigator.userAgent is recoverable; a dead navigation is not. + } else { + guest.setUserAgent(nextUa) } - } else if (currentUa === firefoxUa) { - // Only restore when the auth-host override is actually in place, so normal - // navigation never touches the session UA. - guest.setUserAgent(guest.session.getUserAgent()) } // Why: gate on the DIRECT page id, not ownerTabId — a popup has no device-metrics override of // its own, so inheriting the owner tab's preset UA would pair a mobile UA with a desktop viewport. - if (browserPageId) { + if (browserPageId && !authOverrideIssuedOverCdp) { this.reapplyViewportUserAgentOverride(guest, browserPageId, url) } } + private canOverrideUserAgentOverCdp(guest: Electron.WebContents): boolean { + try { + return !guest.isDestroyed() && guest.debugger.isAttached() + } catch { + return false + } + } + + private applyAuthUserAgentOverrideOverCdp( + guest: Electron.WebContents, + mobile: boolean, + url: string, + userAgent: string + ): Promise { + if (!this.canOverrideUserAgentOverCdp(guest)) { + return Promise.resolve(false) + } + const state = this.authUserAgentOverrideStateByGuestId.get(guest.id) ?? { + confirmed: null, + nextSequence: 0, + pending: [] + } + const operation = { sequence: ++state.nextSequence, userAgent } + state.pending.push(operation) + this.authUserAgentOverrideStateByGuestId.set(guest.id, state) + return this.sendViewportUserAgentOverride(guest, mobile, url, userAgent).then( + () => this.settleAuthUserAgentOverride(guest.id, state, operation, true), + () => { + this.settleAuthUserAgentOverride(guest.id, state, operation, false) + return false + } + ) + } + + private settleAuthUserAgentOverride( + guestId: number, + state: AuthUserAgentOverrideState, + operation: AuthUserAgentOverrideOperation, + succeeded: boolean + ): boolean { + if (this.authUserAgentOverrideStateByGuestId.get(guestId) !== state) { + return false + } + if (succeeded && (state.confirmed?.sequence ?? -1) < operation.sequence) { + state.confirmed = operation + } + const pendingIndex = state.pending.indexOf(operation) + if (pendingIndex !== -1) { + state.pending.splice(pendingIndex, 1) + } + if (state.confirmed === null && state.pending.length === 0) { + this.authUserAgentOverrideStateByGuestId.delete(guestId) + } + return true + } + private startPendingNavigation(guestId: number, url: string): void { const pending = this.pendingNavigationByGuestId.get(guestId) this.pendingNavigationByGuestId.set(guestId, { @@ -1044,7 +1154,8 @@ export class BrowserManager { private async sendViewportUserAgentOverride( guest: Electron.WebContents, mobile: boolean, - url?: string + url?: string, + baseUserAgent?: string ): Promise { if (guest.isDestroyed() || !guest.debugger.isAttached()) { return @@ -1054,7 +1165,10 @@ export class BrowserManager { buildViewportUserAgentOverride({ url: url ?? this.resolveTabNavigationUrl(guest), mobile, - baseUserAgent: cleanElectronUserAgent(guest.getUserAgent()) + // Why: the session UA is the profile's stable base identity. guest.getUserAgent() is not: + // applyGoogleAuthUserAgent leaves it pinned to the Firefox auth UA once a guest switches to + // the CDP override, so reading it back here would republish that identity on ordinary hosts. + baseUserAgent: cleanElectronUserAgent(baseUserAgent ?? guest.session.getUserAgent()) }) ) } @@ -1107,6 +1221,7 @@ export class BrowserManager { this.clickedLinkFrameNameByGuestId.delete(guestWebContentsId) this.offscreenGuestIds.delete(guestWebContentsId) this.popupOwnerContextByGuestId.delete(guestWebContentsId) + this.authUserAgentOverrideStateByGuestId.delete(guestWebContentsId) this.pendingNavigationByGuestId.delete(guestWebContentsId) // Why: a popup must stop inheriting authorization the moment its owner retires, before Chromium destroys the child. if (isPrimaryGuest) { @@ -1311,6 +1426,7 @@ export class BrowserManager { this.sessionProfileIdByPageId.clear() this.userAgentModeByPageId.clear() this.viewportUaOverrideMobileByTabId.clear() + this.authUserAgentOverrideStateByGuestId.clear() this.pendingNavigationByGuestId.clear() this.pendingLoadFailuresByGuestId.clear() this.loadErrorsByGuestId.clear() @@ -1730,9 +1846,22 @@ export class BrowserManager { const trackedMobile = this.viewportUaOverrideMobileByTabId.get(browserTabId) // A navigation after this point must not re-install the override behind the clear. this.viewportUaOverrideMobileByTabId.delete(browserTabId) - // Why: passing an empty string restores the session default UA. try { - await dbg.sendCommand('Emulation.setUserAgentOverride', { userAgent: '' }) + if (this.authUserAgentOverrideStateByGuestId.has(guest.id)) { + const url = this.resolveTabNavigationUrl(guest) + const restored = await this.applyAuthUserAgentOverrideOverCdp( + guest, + false, + url, + isGoogleAuthUrl(url) ? googleAuthUserAgent() : guest.session.getUserAgent() + ) + if (!restored) { + throw new Error('Failed to preserve auth user agent') + } + } else { + // Why: passing an empty string restores the session default UA. + await dbg.sendCommand('Emulation.setUserAgentOverride', { userAgent: '' }) + } } catch (error) { if (trackedMobile !== undefined) { this.viewportUaOverrideMobileByTabId.set(browserTabId, trackedMobile)