From 2487e9b98deb32b10a43bd93cc839d4556223ad4 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Mon, 12 May 2025 11:13:49 -0400 Subject: [PATCH] [PM-20637] Trigger password reprompt when updating a reprompt cipher via notification (#14575) * reprompt user on cipher update when enabled Co-authored-by: Daniel Riera * update tests * add test --------- Co-authored-by: Daniel Riera --- .../notification.background.spec.ts | 57 ++++++++++++++++++- .../background/notification.background.ts | 42 ++++++++++++-- .../overlay-notifications-content.service.ts | 1 + .../overlay-notifications-content.service.ts | 6 +- .../services/abstractions/autofill.service.ts | 6 +- .../src/autofill/services/autofill.service.ts | 9 ++- .../vault-v2/view-v2/view-v2.component.ts | 27 +++++++-- libs/common/src/autofill/constants/index.ts | 3 +- 8 files changed, 134 insertions(+), 17 deletions(-) diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index 63ae1193737..009efd7ff36 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -17,6 +17,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag import { SelfHostedEnvironment } from "@bitwarden/common/platform/services/default-environment.service"; import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service"; import { UserId } from "@bitwarden/common/types/guid"; +import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { CipherService } from "@bitwarden/common/vault/services/cipher.service"; @@ -828,6 +829,7 @@ describe("NotificationBackground", () => { id: "testId", name: "testItemName", login: { username: "testUser" }, + reprompt: CipherRepromptType.None, }); getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView); @@ -842,6 +844,7 @@ describe("NotificationBackground", () => { message.edit, sender.tab, "testId", + false, ); expect(updateWithServerSpy).toHaveBeenCalled(); expect(tabSendMessageDataSpy).toHaveBeenCalledWith( @@ -855,6 +858,55 @@ describe("NotificationBackground", () => { ); }); + it("prompts the user for master password entry if the notification message type is for ChangePassword and the cipher reprompt is enabled", async () => { + const tab = createChromeTabMock({ id: 1, url: "https://example.com" }); + const sender = mock({ tab }); + const message: NotificationBackgroundExtensionMessage = { + command: "bgSaveCipher", + edit: false, + folder: "folder-id", + }; + const queueMessage = mock({ + type: NotificationQueueMessageType.ChangePassword, + tab, + domain: "example.com", + newPassword: "newPassword", + }); + notificationBackground["notificationQueue"] = [queueMessage]; + const cipherView = mock({ + id: "testId", + name: "testItemName", + login: { username: "testUser" }, + reprompt: CipherRepromptType.Password, + }); + getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView); + + sendMockExtensionMessage(message, sender); + await flushPromises(); + + expect(editItemSpy).not.toHaveBeenCalled(); + expect(autofillService.isPasswordRepromptRequired).toHaveBeenCalled(); + expect(createWithServerSpy).not.toHaveBeenCalled(); + expect(updatePasswordSpy).toHaveBeenCalledWith( + cipherView, + queueMessage.newPassword, + message.edit, + sender.tab, + "testId", + false, + ); + expect(updateWithServerSpy).not.toHaveBeenCalled(); + expect(tabSendMessageDataSpy).not.toHaveBeenCalledWith( + sender.tab, + "saveCipherAttemptCompleted", + { + itemName: "testItemName", + cipherId: cipherView.id, + task: undefined, + }, + ); + }); + it("completes password update notification with a security task notice if any are present for the cipher, and dismisses tasks for the updated cipher", async () => { const mockCipherId = "testId"; const mockOrgId = "testOrgId"; @@ -905,6 +957,7 @@ describe("NotificationBackground", () => { id: mockCipherId, organizationId: mockOrgId, name: "Test Item", + reprompt: CipherRepromptType.None, }); getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView); @@ -919,6 +972,7 @@ describe("NotificationBackground", () => { message.edit, sender.tab, mockCipherId, + false, ); expect(updateWithServerSpy).toHaveBeenCalled(); expect(tabSendMessageDataSpy).toHaveBeenCalledWith( @@ -1000,6 +1054,7 @@ describe("NotificationBackground", () => { message.edit, sender.tab, "testId", + false, ); expect(editItemSpy).toHaveBeenCalled(); expect(updateWithServerSpy).not.toHaveBeenCalled(); @@ -1170,7 +1225,7 @@ describe("NotificationBackground", () => { newPassword: "newPassword", }); notificationBackground["notificationQueue"] = [queueMessage]; - const cipherView = mock(); + const cipherView = mock({ reprompt: CipherRepromptType.None }); getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView); const errorMessage = "fetch error"; updateWithServerSpy.mockImplementation(() => { diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 5dd15274677..52920ec67a8 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -14,6 +14,7 @@ import { ExtensionCommand, ExtensionCommandType, NOTIFICATION_BAR_LIFESPAN_MS, + UPDATE_PASSWORD, } from "@bitwarden/common/autofill/constants"; import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; import { UserNotificationSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/user-notification-settings.service"; @@ -104,6 +105,8 @@ export default class NotificationBackground { this.removeTabFromNotificationQueue(sender.tab), bgReopenUnlockPopout: ({ sender }) => this.openUnlockPopout(sender.tab), bgSaveCipher: ({ message, sender }) => this.handleSaveCipherMessage(message, sender), + bgHandleReprompt: ({ message, sender }: any) => + this.handleCipherUpdateRepromptResponse(message), bgUnlockPopoutOpened: ({ message, sender }) => this.unlockVault(message, sender.tab), checkNotificationQueue: ({ sender }) => this.checkNotificationQueue(sender.tab), collectPageDetailsResponse: ({ message }) => @@ -631,6 +634,17 @@ export default class NotificationBackground { await this.saveOrUpdateCredentials(sender.tab, message.edit, message.folder); } + async handleCipherUpdateRepromptResponse(message: NotificationBackgroundExtensionMessage) { + if (message.success) { + await this.saveOrUpdateCredentials(message.tab, false, undefined, true); + } else { + await BrowserApi.tabSendMessageData(message.tab, "saveCipherAttemptCompleted", { + error: "Password reprompt failed", + }); + return; + } + } + /** * Saves or updates credentials based on the message within the * notification queue that is associated with the specified tab. @@ -639,7 +653,12 @@ export default class NotificationBackground { * @param edit - Identifies if the credentials should be edited or simply added * @param folderId - The folder to add the cipher to */ - private async saveOrUpdateCredentials(tab: chrome.tabs.Tab, edit: boolean, folderId?: string) { + private async saveOrUpdateCredentials( + tab: chrome.tabs.Tab, + edit: boolean, + folderId?: string, + skipReprompt: boolean = false, + ) { for (let i = this.notificationQueue.length - 1; i >= 0; i--) { const queueMessage = this.notificationQueue[i]; if ( @@ -654,18 +673,26 @@ export default class NotificationBackground { continue; } - this.notificationQueue.splice(i, 1); - const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getOptionalUserId), ); if (queueMessage.type === NotificationQueueMessageType.ChangePassword) { const cipherView = await this.getDecryptedCipherById(queueMessage.cipherId, activeUserId); - await this.updatePassword(cipherView, queueMessage.newPassword, edit, tab, activeUserId); + + await this.updatePassword( + cipherView, + queueMessage.newPassword, + edit, + tab, + activeUserId, + skipReprompt, + ); return; } + this.notificationQueue.splice(i, 1); + // If the vault was locked, check if a cipher needs updating instead of creating a new one if (queueMessage.wasVaultLocked) { const allCiphers = await this.cipherService.getAllDecryptedForUrl( @@ -725,6 +752,7 @@ export default class NotificationBackground { edit: boolean, tab: chrome.tabs.Tab, userId: UserId, + skipReprompt: boolean = false, ) { cipherView.login.password = newPassword; @@ -758,6 +786,12 @@ export default class NotificationBackground { } : undefined; + if (cipherView.reprompt && !skipReprompt) { + await this.autofillService.isPasswordRepromptRequired(cipherView, tab, UPDATE_PASSWORD); + + return; + } + await this.cipherService.updateWithServer(cipher); await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { diff --git a/apps/browser/src/autofill/overlay/notifications/abstractions/overlay-notifications-content.service.ts b/apps/browser/src/autofill/overlay/notifications/abstractions/overlay-notifications-content.service.ts index 82c03cacadf..42d7666e8a7 100644 --- a/apps/browser/src/autofill/overlay/notifications/abstractions/overlay-notifications-content.service.ts +++ b/apps/browser/src/autofill/overlay/notifications/abstractions/overlay-notifications-content.service.ts @@ -15,6 +15,7 @@ export type NotificationsExtensionMessage = { typeData?: NotificationTypeData; height?: number; error?: string; + closedByUser?: boolean; fadeOutNotification?: boolean; }; }; diff --git a/apps/browser/src/autofill/overlay/notifications/content/overlay-notifications-content.service.ts b/apps/browser/src/autofill/overlay/notifications/content/overlay-notifications-content.service.ts index 519521feaa9..c21aaa37dd4 100644 --- a/apps/browser/src/autofill/overlay/notifications/content/overlay-notifications-content.service.ts +++ b/apps/browser/src/autofill/overlay/notifications/content/overlay-notifications-content.service.ts @@ -106,13 +106,15 @@ export class OverlayNotificationsContentService * @param message - The message containing the data for closing the notification bar. */ private handleCloseNotificationBarMessage(message: NotificationsExtensionMessage) { + const closedByUser = + typeof message.data?.closedByUser === "boolean" ? message.data.closedByUser : true; if (message.data?.fadeOutNotification) { setElementStyles(this.notificationBarIframeElement, { opacity: "0" }, true); - globalThis.setTimeout(() => this.closeNotificationBar(true), 150); + globalThis.setTimeout(() => this.closeNotificationBar(closedByUser), 150); return; } - this.closeNotificationBar(true); + this.closeNotificationBar(closedByUser); } /** diff --git a/apps/browser/src/autofill/services/abstractions/autofill.service.ts b/apps/browser/src/autofill/services/abstractions/autofill.service.ts index 5b1b4b3b8bb..daafd871789 100644 --- a/apps/browser/src/autofill/services/abstractions/autofill.service.ts +++ b/apps/browser/src/autofill/services/abstractions/autofill.service.ts @@ -87,5 +87,9 @@ export abstract class AutofillService { cipherType?: CipherType, ) => Promise; setAutoFillOnPageLoadOrgPolicy: () => Promise; - isPasswordRepromptRequired: (cipher: CipherView, tab: chrome.tabs.Tab) => Promise; + isPasswordRepromptRequired: ( + cipher: CipherView, + tab: chrome.tabs.Tab, + action?: string, + ) => Promise; } diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index da46ceb0864..525010bacc1 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -593,15 +593,20 @@ export default class AutofillService implements AutofillServiceInterface { * * @param cipher - The cipher to autofill * @param tab - The tab to autofill + * @param action - override for default action once reprompt is completed successfully */ - async isPasswordRepromptRequired(cipher: CipherView, tab: chrome.tabs.Tab): Promise { + async isPasswordRepromptRequired( + cipher: CipherView, + tab: chrome.tabs.Tab, + action?: string, + ): Promise { const userHasMasterPasswordAndKeyHash = await this.userVerificationService.hasMasterPasswordAndMasterKeyHash(); if (cipher.reprompt === CipherRepromptType.Password && userHasMasterPasswordAndKeyHash) { if (!this.isDebouncingPasswordRepromptPopout()) { await this.openVaultItemPasswordRepromptPopout(tab, { cipherId: cipher.id, - action: "autofill", + action: action ?? "autofill", }); } diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts index 56db47619b0..a834314560b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts @@ -19,6 +19,7 @@ import { COPY_USERNAME_ID, COPY_VERIFICATION_CODE_ID, SHOW_AUTOFILL_BUTTON, + UPDATE_PASSWORD, } from "@bitwarden/common/autofill/constants"; import { EventType } from "@bitwarden/common/enums"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; @@ -49,6 +50,7 @@ import { PasswordRepromptService, } from "@bitwarden/vault"; +import { sendExtensionMessage } from "../../../../../autofill/utils/index"; import { BrowserApi } from "../../../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils"; import { PopOutComponent } from "../../../../../platform/popup/components/pop-out.component"; @@ -72,7 +74,8 @@ type LoadAction = | typeof SHOW_AUTOFILL_BUTTON | typeof COPY_USERNAME_ID | typeof COPY_PASSWORD_ID - | typeof COPY_VERIFICATION_CODE_ID; + | typeof COPY_VERIFICATION_CODE_ID + | typeof UPDATE_PASSWORD; @Component({ selector: "app-view-v2", @@ -294,7 +297,7 @@ export class ViewV2Component { // Both vaultPopupAutofillService and copyCipherFieldService will perform password re-prompting internally. switch (loadAction) { - case "show-autofill-button": + case SHOW_AUTOFILL_BUTTON: // This action simply shows the cipher view, no need to do anything. if ( this.cipher.reprompt !== CipherRepromptType.None && @@ -303,30 +306,42 @@ export class ViewV2Component { await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${this.cipher.id}`); } return; - case "autofill": + case AUTOFILL_ID: actionSuccess = await this.vaultPopupAutofillService.doAutofill(this.cipher, false); break; - case "copy-username": + case COPY_USERNAME_ID: actionSuccess = await this.copyCipherFieldService.copy( this.cipher.login.username, "username", this.cipher, ); break; - case "copy-password": + case COPY_PASSWORD_ID: actionSuccess = await this.copyCipherFieldService.copy( this.cipher.login.password, "password", this.cipher, ); break; - case "copy-totp": + case COPY_VERIFICATION_CODE_ID: actionSuccess = await this.copyCipherFieldService.copy( this.cipher.login.totp, "totp", this.cipher, ); break; + case UPDATE_PASSWORD: { + const repromptSuccess = await this.passwordRepromptService.showPasswordPrompt(); + + await sendExtensionMessage("bgHandleReprompt", { + tab: await chrome.tabs.get(senderTabId), + success: repromptSuccess, + }); + + await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${this.cipher.id}`); + + break; + } } if (BrowserPopupUtils.inPopout(window)) { diff --git a/libs/common/src/autofill/constants/index.ts b/libs/common/src/autofill/constants/index.ts index 62ad10e9a90..dc79e27b6aa 100644 --- a/libs/common/src/autofill/constants/index.ts +++ b/libs/common/src/autofill/constants/index.ts @@ -38,7 +38,7 @@ export const ClearClipboardDelay = { FiveMinutes: 300, } as const; -/* Context Menu item Ids */ +/* Ids for context menu items and messaging events */ export const AUTOFILL_CARD_ID = "autofill-card"; export const AUTOFILL_ID = "autofill"; export const SHOW_AUTOFILL_BUTTON = "show-autofill-button"; @@ -54,6 +54,7 @@ export const GENERATE_PASSWORD_ID = "generate-password"; export const NOOP_COMMAND_SUFFIX = "noop"; export const ROOT_ID = "root"; export const SEPARATOR_ID = "separator"; +export const UPDATE_PASSWORD = "update-password"; export const NOTIFICATION_BAR_LIFESPAN_MS = 150000; // 150 seconds