From aa4eac7d409cd86fd74e680932d2f7e49f85057e Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Fri, 20 Feb 2026 10:01:04 -0500 Subject: [PATCH] do not show passkey dialog and notifications at the same time (#18878) --- .../notification.background.spec.ts | 21 ++++- .../background/notification.background.ts | 7 ++ .../abstractions/fido2.background.ts | 2 + .../fido2/background/fido2.background.spec.ts | 78 +++++++++++++++++++ .../fido2/background/fido2.background.ts | 34 +++++--- .../browser-fido2-user-interface.service.ts | 3 + .../browser/src/background/main.background.ts | 1 + 7 files changed, 136 insertions(+), 10 deletions(-) diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index 95d4111987b..1bd1ae5513b 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -28,6 +28,7 @@ import { TaskService, SecurityTask } from "@bitwarden/common/vault/tasks"; import { BrowserApi } from "../../platform/browser/browser-api"; import { NotificationType } from "../enums/notification-type.enum"; +import { Fido2Background } from "../fido2/background/abstractions/fido2.background"; import { FormData } from "../services/abstractions/autofill.service"; import AutofillService from "../services/autofill.service"; import { createAutofillPageDetailsMock, createChromeTabMock } from "../spec/autofill-mocks"; @@ -81,6 +82,8 @@ describe("NotificationBackground", () => { const configService = mock(); const accountService = mock(); const organizationService = mock(); + const fido2Background = mock(); + fido2Background.isCredentialRequestInProgress.mockReturnValue(false); const userId = "testId" as UserId; const activeAccountSubject = new BehaviorSubject({ @@ -115,6 +118,7 @@ describe("NotificationBackground", () => { userNotificationSettingsService, taskService, messagingService, + fido2Background, ); }); @@ -759,7 +763,6 @@ describe("NotificationBackground", () => { notificationBackground as any, "getEnableChangedPasswordPrompt", ); - pushChangePasswordToQueueSpy = jest.spyOn( notificationBackground as any, "pushChangePasswordToQueue", @@ -822,6 +825,22 @@ describe("NotificationBackground", () => { expectSkippedCheckingNotification(); }); + it("skips checking if a notification should trigger if a fido2 credential request is in progress for the tab", async () => { + const formEntryData: ModifyLoginCipherFormData = { + newPassword: "", + password: "", + uri: mockFormURI, + username: "ADent", + }; + + activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); + fido2Background.isCredentialRequestInProgress.mockReturnValueOnce(true); + + await notificationBackground.triggerCipherNotification(formEntryData, tab); + + expectSkippedCheckingNotification(); + }); + it("skips checking if a notification should trigger if the user has disabled both the new login and update password notification", async () => { const formEntryData: ModifyLoginCipherFormData = { newPassword: "Bab3lPhs5h", diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 3713cd7c4c2..64c52701e21 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -61,6 +61,7 @@ import { } from "../content/components/cipher/types"; import { CollectionView } from "../content/components/common-types"; import { NotificationType } from "../enums/notification-type.enum"; +import { Fido2Background } from "../fido2/background/abstractions/fido2.background"; import { AutofillService } from "../services/abstractions/autofill.service"; import { TemporaryNotificationChangeLoginService } from "../services/notification-change-login-password.service"; @@ -165,6 +166,7 @@ export default class NotificationBackground { private userNotificationSettingsService: UserNotificationSettingsServiceAbstraction, private taskService: TaskService, protected messagingService: MessagingService, + private fido2Background: Fido2Background, ) {} init() { @@ -665,6 +667,11 @@ export default class NotificationBackground { return false; } + // If there is an active passkey prompt, exit early + if (this.fido2Background.isCredentialRequestInProgress(tab.id)) { + return false; + } + // If no cipher add/update notifications are enabled, we can exit early const changePasswordNotificationIsEnabled = await this.getEnableChangedPasswordPrompt(); const newLoginNotificationIsEnabled = await this.getEnableAddedLoginPrompt(); diff --git a/apps/browser/src/autofill/fido2/background/abstractions/fido2.background.ts b/apps/browser/src/autofill/fido2/background/abstractions/fido2.background.ts index 6ad069ad56e..c5346d61566 100644 --- a/apps/browser/src/autofill/fido2/background/abstractions/fido2.background.ts +++ b/apps/browser/src/autofill/fido2/background/abstractions/fido2.background.ts @@ -45,6 +45,8 @@ type Fido2BackgroundExtensionMessageHandlers = { interface Fido2Background { init(): void; + isCredentialRequestInProgress(tabId: number): boolean; + isPasskeySettingEnabled(): Promise; } export { diff --git a/apps/browser/src/autofill/fido2/background/fido2.background.spec.ts b/apps/browser/src/autofill/fido2/background/fido2.background.spec.ts index 752851b3d37..6ead7416b96 100644 --- a/apps/browser/src/autofill/fido2/background/fido2.background.spec.ts +++ b/apps/browser/src/autofill/fido2/background/fido2.background.spec.ts @@ -256,6 +256,84 @@ describe("Fido2Background", () => { }); }); + describe("isCredentialRequestInProgress", () => { + beforeEach(() => { + fido2Background.init(); + }); + + it("returns false when no credential request is active", () => { + expect(fido2Background.isCredentialRequestInProgress(tabMock.id)).toBe(false); + }); + + it("returns true while a register credential request is in progress", async () => { + let duringRequestResult: boolean; + fido2ClientService.createCredential.mockImplementation(async () => { + duringRequestResult = fido2Background.isCredentialRequestInProgress(tabMock.id); + return mock(); + }); + + const message = mock({ + command: "fido2RegisterCredentialRequest", + requestId: "123", + data: mock(), + }); + + sendMockExtensionMessage(message, senderMock); + await flushPromises(); + + expect(duringRequestResult).toBe(true); + }); + + it("returns true while a get credential request is in progress", async () => { + let duringRequestResult: boolean; + fido2ClientService.assertCredential.mockImplementation(async () => { + duringRequestResult = fido2Background.isCredentialRequestInProgress(tabMock.id); + return mock(); + }); + + const message = mock({ + command: "fido2GetCredentialRequest", + requestId: "123", + data: mock(), + }); + + sendMockExtensionMessage(message, senderMock); + await flushPromises(); + + expect(duringRequestResult).toBe(true); + }); + + it("returns false after a credential request completes", async () => { + fido2ClientService.createCredential.mockResolvedValue(mock()); + + const message = mock({ + command: "fido2RegisterCredentialRequest", + requestId: "123", + data: mock(), + }); + + sendMockExtensionMessage(message, senderMock); + await flushPromises(); + + expect(fido2Background.isCredentialRequestInProgress(tabMock.id)).toBe(false); + }); + + it("returns false after a credential request errors", async () => { + fido2ClientService.createCredential.mockRejectedValue(new Error("error")); + + const message = mock({ + command: "fido2RegisterCredentialRequest", + requestId: "123", + data: mock(), + }); + + sendMockExtensionMessage(message, senderMock); + await flushPromises(); + + expect(fido2Background.isCredentialRequestInProgress(tabMock.id)).toBe(false); + }); + }); + describe("extension message handlers", () => { beforeEach(() => { fido2Background.init(); diff --git a/apps/browser/src/autofill/fido2/background/fido2.background.ts b/apps/browser/src/autofill/fido2/background/fido2.background.ts index 22ee4a1822d..495b0d85f0b 100644 --- a/apps/browser/src/autofill/fido2/background/fido2.background.ts +++ b/apps/browser/src/autofill/fido2/background/fido2.background.ts @@ -35,6 +35,7 @@ export class Fido2Background implements Fido2BackgroundInterface { private currentAuthStatus$: Subscription; private abortManager = new AbortManager(); private fido2ContentScriptPortsSet = new Set(); + private activeCredentialRequests = new Set(); private registeredContentScripts: browser.contentScripts.RegisteredContentScript; private readonly sharedInjectionDetails: SharedFido2ScriptInjectionDetails = { runAt: "document_start", @@ -61,6 +62,16 @@ export class Fido2Background implements Fido2BackgroundInterface { private authService: AuthService, ) {} + /** + * Checks if a FIDO2 credential request (registration or assertion) + * is currently in progress for the given tab. + * + * @param tabId - The tab id to check. + */ + isCredentialRequestInProgress(tabId: number): boolean { + return this.activeCredentialRequests.has(tabId); + } + /** * Initializes the FIDO2 background service. Sets up the extension message * and port listeners. Subscribes to the enablePasskeys$ observable to @@ -307,20 +318,25 @@ export class Fido2Background implements Fido2BackgroundInterface { abortController: AbortController, ) => Promise, ) => { - return await this.abortManager.runWithAbortController(requestId, async (abortController) => { - try { - return await callback(data, tab, abortController); - } finally { - await BrowserApi.focusTab(tab.id); - await BrowserApi.focusWindow(tab.windowId); - } - }); + this.activeCredentialRequests.add(tab.id); + try { + return await this.abortManager.runWithAbortController(requestId, async (abortController) => { + try { + return await callback(data, tab, abortController); + } finally { + await BrowserApi.focusTab(tab.id); + await BrowserApi.focusWindow(tab.windowId); + } + }); + } finally { + this.activeCredentialRequests.delete(tab.id); + } }; /** * Checks if the enablePasskeys setting is enabled. */ - private async isPasskeySettingEnabled() { + async isPasskeySettingEnabled() { return await firstValueFrom(this.vaultSettingsService.enablePasskeys$); } diff --git a/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts b/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts index 19c1dbc8790..11dc170db16 100644 --- a/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts +++ b/apps/browser/src/autofill/fido2/services/browser-fido2-user-interface.service.ts @@ -363,6 +363,9 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi ), ); + // Defensive measure in case an existing notification appeared before the passkey popout + await BrowserApi.tabSendMessageData(this.tab, "closeNotificationBar"); + const popoutId = await openFido2Popout(this.tab, { sessionId: this.sessionId, fallbackSupported: this.fallbackSupported, diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 25c7b344982..95ec6e5ad20 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1409,6 +1409,7 @@ export default class MainBackground { this.userNotificationSettingsService, this.taskService, this.messagingService, + this.fido2Background, ); this.overlayNotificationsBackground = new OverlayNotificationsBackground(