From f12456bd3e8af71cfed5c9e1dccf1a70994ee9bd Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Fri, 28 Feb 2025 13:14:15 -0500 Subject: [PATCH] Pm 18493 pass relevant cipher name into confirmation UI (#13570) * PM-18276-wip * update typing * dynamically retrieve messages, resolve theme in function * five second timeout after save or update * adjust timeout to five seconds * negligible performance gain-revert * sacrifice contorl for to remove event listeners-revert * PM-18493 initial wip commit * fix types and story * edit tests to account for sendmessagewithdata * add tests and return id on new add/save * function name --- apps/browser/src/_locales/en/messages.json | 18 +++++++++++ .../abstractions/notification.background.ts | 1 + .../notification.background.spec.ts | 32 +++++++++++++++---- .../background/notification.background.ts | 25 ++++++++++++--- .../notification/confirmation.lit-stories.ts | 2 +- .../notification/confirmation-container.ts | 18 +++++++++-- .../components/notification/confirmation.ts | 8 +++-- .../src/autofill/content/notification-bar.ts | 2 ++ .../abstractions/notification-bar.ts | 3 +- apps/browser/src/autofill/notification/bar.ts | 16 ++++++++-- 10 files changed, 102 insertions(+), 23 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 4f19b4d880f..b6c44405ffa 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -1070,6 +1070,24 @@ "notificationAddSave": { "message": "Save" }, + "loginSaveSuccessDetails": { + "message": "$USERNAME$ saved to Bitwarden.", + "placeholders": { + "username": { + "content": "$1" + } + }, + "description": "Shown to user after login is saved." + }, + "loginUpdatedSuccessDetails": { + "message": "$USERNAME$ updated in Bitwarden.", + "placeholders": { + "username": { + "content": "$1" + } + }, + "description": "Shown to user after login is updated." + }, "enableChangedPasswordNotification": { "message": "Ask to update existing login" }, diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index 1b989283112..851f07576dd 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -100,6 +100,7 @@ type NotificationBackgroundExtensionMessageHandlers = { bgChangedPassword: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgRemoveTabFromNotificationQueue: ({ sender }: BackgroundSenderParam) => void; bgSaveCipher: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; + bgOpenVault: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgNeverSave: ({ sender }: BackgroundSenderParam) => Promise; bgUnlockPopoutOpened: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgReopenUnlockPopout: ({ sender }: BackgroundSenderParam) => Promise; diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index 40c4d07cadf..e02e3d8d951 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -812,7 +812,10 @@ describe("NotificationBackground", () => { newPassword: "newPassword", }); notificationBackground["notificationQueue"] = [queueMessage]; - const cipherView = mock(); + const cipherView = mock({ + id: "testId", + login: { username: "testUser" }, + }); getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView); sendMockExtensionMessage(message, sender); @@ -828,9 +831,14 @@ describe("NotificationBackground", () => { "testId", ); expect(updateWithServerSpy).toHaveBeenCalled(); - expect(tabSendMessageSpy).toHaveBeenCalledWith(sender.tab, { - command: "saveCipherAttemptCompleted", - }); + expect(tabSendMessageDataSpy).toHaveBeenCalledWith( + sender.tab, + "saveCipherAttemptCompleted", + { + username: cipherView.login.username, + cipherId: cipherView.id, + }, + ); }); it("updates the cipher password if the queue message was locked and an existing cipher has the same username as the message", async () => { @@ -976,11 +984,16 @@ describe("NotificationBackground", () => { }); notificationBackground["notificationQueue"] = [queueMessage]; const cipherView = mock({ + id: "testId", login: { username: "test", password: "password" }, }); folderExistsSpy.mockResolvedValueOnce(false); convertAddLoginQueueMessageToCipherViewSpy.mockReturnValueOnce(cipherView); editItemSpy.mockResolvedValueOnce(undefined); + cipherEncryptSpy.mockResolvedValueOnce({ + ...cipherView, + id: "testId", + }); sendMockExtensionMessage(message, sender); await flushPromises(); @@ -991,9 +1004,14 @@ describe("NotificationBackground", () => { ); expect(cipherEncryptSpy).toHaveBeenCalledWith(cipherView, "testId"); expect(createWithServerSpy).toHaveBeenCalled(); - expect(tabSendMessageSpy).toHaveBeenCalledWith(sender.tab, { - command: "saveCipherAttemptCompleted", - }); + expect(tabSendMessageDataSpy).toHaveBeenCalledWith( + sender.tab, + "saveCipherAttemptCompleted", + { + username: cipherView.login.username, + cipherId: cipherView.id, + }, + ); expect(tabSendMessageSpy).toHaveBeenCalledWith(sender.tab, { command: "addedCipher" }); }); diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 1a99425b7de..11037e7e261 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -87,6 +87,7 @@ export default class NotificationBackground { getWebVaultUrlForNotification: () => this.getWebVaultUrl(), notificationRefreshFlagValue: () => this.getNotificationFlag(), bgGetDecryptedCiphers: () => this.getNotificationCipherData(), + bgOpenVault: ({ message, sender }) => this.openVault(message, sender.tab), }; constructor( @@ -594,7 +595,10 @@ export default class NotificationBackground { const cipher = await this.cipherService.encrypt(newCipher, activeUserId); try { await this.cipherService.createWithServer(cipher); - await BrowserApi.tabSendMessage(tab, { command: "saveCipherAttemptCompleted" }); + await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { + username: String(queueMessage?.username), + cipherId: String(cipher?.id), + }); await BrowserApi.tabSendMessage(tab, { command: "addedCipher" }); } catch (error) { await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { @@ -630,15 +634,16 @@ export default class NotificationBackground { await BrowserApi.tabSendMessage(tab, { command: "editedCipher" }); return; } - const cipher = await this.cipherService.encrypt(cipherView, userId); try { - // We've only updated the password, no need to broadcast editedCipher message await this.cipherService.updateWithServer(cipher); - await BrowserApi.tabSendMessage(tab, { command: "saveCipherAttemptCompleted" }); + await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { + username: String(cipherView?.login?.username), + cipherId: String(cipherView?.id), + }); } catch (error) { await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { - error: String(error.message), + error: String(error?.message), }); } } @@ -663,6 +668,16 @@ export default class NotificationBackground { await this.openAddEditVaultItemPopout(senderTab, { cipherId: cipherView.id }); } + private async openVault( + message: NotificationBackgroundExtensionMessage, + senderTab: chrome.tabs.Tab, + ) { + if (!message.cipherId) { + await this.openAddEditVaultItemPopout(senderTab); + } + await this.openAddEditVaultItemPopout(senderTab, { cipherId: message.cipherId }); + } + private async folderExists(folderId: string, userId: UserId) { if (Utils.isNullOrWhitespace(folderId) || folderId === "null") { return false; diff --git a/apps/browser/src/autofill/content/components/lit-stories/notification/confirmation.lit-stories.ts b/apps/browser/src/autofill/content/components/lit-stories/notification/confirmation.lit-stories.ts index 94dbaace9aa..b3dee95efd0 100644 --- a/apps/browser/src/autofill/content/components/lit-stories/notification/confirmation.lit-stories.ts +++ b/apps/browser/src/autofill/content/components/lit-stories/notification/confirmation.lit-stories.ts @@ -7,7 +7,7 @@ import { NotificationConfirmationBody } from "../../notification/confirmation"; type Args = { buttonText: string; confirmationMessage: string; - handleClick: () => void; + handleOpenVault: () => void; theme: Theme; error: string; }; diff --git a/apps/browser/src/autofill/content/components/notification/confirmation-container.ts b/apps/browser/src/autofill/content/components/notification/confirmation-container.ts index 4fda95986db..8fdc5474486 100644 --- a/apps/browser/src/autofill/content/components/notification/confirmation-container.ts +++ b/apps/browser/src/autofill/content/components/notification/confirmation-container.ts @@ -19,18 +19,22 @@ import { export function NotificationConfirmationContainer({ error, handleCloseNotification, + handleOpenVault, i18n, theme = ThemeTypes.Light, type, + username, }: NotificationBarIframeInitData & { handleCloseNotification: (e: Event) => void; + handleOpenVault: (e: Event) => void; } & { error: string; i18n: { [key: string]: string }; type: NotificationType; + username: string; }) { const headerMessage = getHeaderMessage(i18n, type, error); - const confirmationMessage = getConfirmationMessage(i18n, type, error); + const confirmationMessage = getConfirmationMessage(i18n, username, type, error); const buttonText = error ? i18n.newItem : i18n.view; return html` @@ -41,9 +45,10 @@ export function NotificationConfirmationContainer({ theme, })} ${NotificationConfirmationBody({ - error: error, buttonText, confirmationMessage, + error: error, + handleOpenVault, theme, })} @@ -68,14 +73,21 @@ const notificationContainerStyles = (theme: Theme) => css` function getConfirmationMessage( i18n: { [key: string]: string }, + username: string, type?: NotificationType, error?: string, ) { + const loginSaveSuccessDetails = chrome.i18n.getMessage("loginSaveSuccessDetails", [username]); + const loginUpdatedSuccessDetails = chrome.i18n.getMessage("loginUpdatedSuccessDetails", [ + username, + ]); + if (error) { return i18n.saveFailureDetails; } - return type === "add" ? i18n.loginSaveSuccessDetails : i18n.loginUpdateSuccessDetails; + return type === "add" ? loginSaveSuccessDetails : loginUpdatedSuccessDetails; } + function getHeaderMessage( i18n: { [key: string]: string }, type?: NotificationType, diff --git a/apps/browser/src/autofill/content/components/notification/confirmation.ts b/apps/browser/src/autofill/content/components/notification/confirmation.ts index 0c389f75eb6..13d6c970731 100644 --- a/apps/browser/src/autofill/content/components/notification/confirmation.ts +++ b/apps/browser/src/autofill/content/components/notification/confirmation.ts @@ -1,7 +1,7 @@ import createEmotion from "@emotion/css/create-instance"; import { html } from "lit"; -import { Theme, ThemeTypes } from "@bitwarden/common/platform/enums"; +import { Theme } from "@bitwarden/common/platform/enums"; import { themes } from "../constants/styles"; import { PartyHorn, Warning } from "../icons"; @@ -18,12 +18,14 @@ export function NotificationConfirmationBody({ buttonText, error, confirmationMessage, - theme = ThemeTypes.Light, + theme, + handleOpenVault, }: { error?: string; buttonText: string; confirmationMessage: string; theme: Theme; + handleOpenVault: (e: Event) => void; }) { const IconComponent = !error ? PartyHorn : Warning; return html` @@ -31,7 +33,7 @@ export function NotificationConfirmationBody({
${IconComponent({ theme })}
${confirmationMessage && buttonText ? NotificationConfirmationMessage({ - handleClick: () => {}, + handleClick: handleOpenVault, confirmationMessage, theme, buttonText, diff --git a/apps/browser/src/autofill/content/notification-bar.ts b/apps/browser/src/autofill/content/notification-bar.ts index 8c6251b7030..58480cd6d83 100644 --- a/apps/browser/src/autofill/content/notification-bar.ts +++ b/apps/browser/src/autofill/content/notification-bar.ts @@ -193,6 +193,8 @@ async function loadNotificationBar() { { command: "saveCipherAttemptCompleted", error: msg.data?.error, + username: msg.data?.username, + cipherId: msg.data?.cipherId, }, "*", ); diff --git a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts index 53948a26a2e..cb14a86dffa 100644 --- a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts +++ b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts @@ -19,10 +19,11 @@ type NotificationBarIframeInitData = { }; type NotificationBarWindowMessage = { - [key: string]: any; command: string; error?: string; initData?: NotificationBarIframeInitData; + username?: string; + cipherId?: string; }; type NotificationBarWindowMessageHandlers = { diff --git a/apps/browser/src/autofill/notification/bar.ts b/apps/browser/src/autofill/notification/bar.ts index 8c4c8a3e229..a31f1b4fdbc 100644 --- a/apps/browser/src/autofill/notification/bar.ts +++ b/apps/browser/src/autofill/notification/bar.ts @@ -55,6 +55,8 @@ function getI18n() { close: chrome.i18n.getMessage("close"), never: chrome.i18n.getMessage("never"), folder: chrome.i18n.getMessage("folder"), + loginSaveSuccessDetails: chrome.i18n.getMessage("loginSaveSuccessDetails"), + loginUpdateSuccessDetails: chrome.i18n.getMessage("loginUpdatedSuccessDetails"), notificationAddSave: chrome.i18n.getMessage("notificationAddSave"), notificationAddDesc: chrome.i18n.getMessage("notificationAddDesc"), notificationEdit: chrome.i18n.getMessage("edit"), @@ -70,9 +72,7 @@ function getI18n() { saveLoginPrompt: "Save login?", updateLoginPrompt: "Update existing login?", loginSaveSuccess: "Login saved", - loginSaveSuccessDetails: "Login saved to Bitwarden.", loginUpdateSuccess: "Login updated", - loginUpdateSuccessDetails: "Login updated in Bitwarden.", saveFailure: "Error saving", saveFailureDetails: "Oh no! We couldn't save this. Try entering the details as a New item", newItem: "New item", @@ -289,9 +289,17 @@ function handleSaveCipherAttemptCompletedMessage(message: NotificationBarWindowM ); } +function openViewVaultItemPopout(e: Event, cipherId: string) { + e.preventDefault(); + sendPlatformMessage({ + command: "bgOpenVault", + cipherId, + }); +} + function handleSaveCipherConfirmation(message: NotificationBarWindowMessage) { const { theme, type } = notificationBarIframeInitData; - const { error } = message; + const { error, username, cipherId } = message; const i18n = getI18n(); const resolvedTheme = getResolvedTheme(theme); @@ -305,6 +313,8 @@ function handleSaveCipherConfirmation(message: NotificationBarWindowMessage) { handleCloseNotification, i18n, error, + username, + handleOpenVault: (e) => openViewVaultItemPopout(e, cipherId), }), document.body, );