mirror of
https://github.com/bitwarden/browser
synced 2026-02-24 16:43:27 +00:00
do not show passkey dialog and notifications at the same time (#18878)
This commit is contained in:
@@ -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<ConfigService>();
|
||||
const accountService = mock<AccountService>();
|
||||
const organizationService = mock<OrganizationService>();
|
||||
const fido2Background = mock<Fido2Background>();
|
||||
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",
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -45,6 +45,8 @@ type Fido2BackgroundExtensionMessageHandlers = {
|
||||
|
||||
interface Fido2Background {
|
||||
init(): void;
|
||||
isCredentialRequestInProgress(tabId: number): boolean;
|
||||
isPasskeySettingEnabled(): Promise<boolean>;
|
||||
}
|
||||
|
||||
export {
|
||||
|
||||
@@ -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<Fido2ExtensionMessage>({
|
||||
command: "fido2RegisterCredentialRequest",
|
||||
requestId: "123",
|
||||
data: mock<CreateCredentialParams>(),
|
||||
});
|
||||
|
||||
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<Fido2ExtensionMessage>({
|
||||
command: "fido2GetCredentialRequest",
|
||||
requestId: "123",
|
||||
data: mock<AssertCredentialParams>(),
|
||||
});
|
||||
|
||||
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<Fido2ExtensionMessage>({
|
||||
command: "fido2RegisterCredentialRequest",
|
||||
requestId: "123",
|
||||
data: mock<CreateCredentialParams>(),
|
||||
});
|
||||
|
||||
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<Fido2ExtensionMessage>({
|
||||
command: "fido2RegisterCredentialRequest",
|
||||
requestId: "123",
|
||||
data: mock<CreateCredentialParams>(),
|
||||
});
|
||||
|
||||
sendMockExtensionMessage(message, senderMock);
|
||||
await flushPromises();
|
||||
|
||||
expect(fido2Background.isCredentialRequestInProgress(tabMock.id)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("extension message handlers", () => {
|
||||
beforeEach(() => {
|
||||
fido2Background.init();
|
||||
|
||||
@@ -35,6 +35,7 @@ export class Fido2Background implements Fido2BackgroundInterface {
|
||||
private currentAuthStatus$: Subscription;
|
||||
private abortManager = new AbortManager();
|
||||
private fido2ContentScriptPortsSet = new Set<chrome.runtime.Port>();
|
||||
private activeCredentialRequests = new Set<number>();
|
||||
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<T>,
|
||||
) => {
|
||||
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$);
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -1409,6 +1409,7 @@ export default class MainBackground {
|
||||
this.userNotificationSettingsService,
|
||||
this.taskService,
|
||||
this.messagingService,
|
||||
this.fido2Background,
|
||||
);
|
||||
|
||||
this.overlayNotificationsBackground = new OverlayNotificationsBackground(
|
||||
|
||||
Reference in New Issue
Block a user