Skip to content

Commit

Permalink
Pm 18493 pass relevant cipher name into confirmation UI (#13570)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dan-livefront authored Feb 28, 2025
1 parent 40f7a0d commit f12456b
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 23 deletions.
18 changes: 18 additions & 0 deletions apps/browser/src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ type NotificationBackgroundExtensionMessageHandlers = {
bgChangedPassword: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
bgRemoveTabFromNotificationQueue: ({ sender }: BackgroundSenderParam) => void;
bgSaveCipher: ({ message, sender }: BackgroundOnMessageHandlerParams) => void;
bgOpenVault: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
bgNeverSave: ({ sender }: BackgroundSenderParam) => Promise<void>;
bgUnlockPopoutOpened: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
bgReopenUnlockPopout: ({ sender }: BackgroundSenderParam) => Promise<void>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,10 @@ describe("NotificationBackground", () => {
newPassword: "newPassword",
});
notificationBackground["notificationQueue"] = [queueMessage];
const cipherView = mock<CipherView>();
const cipherView = mock<CipherView>({
id: "testId",
login: { username: "testUser" },
});
getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView);

sendMockExtensionMessage(message, sender);
Expand All @@ -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 () => {
Expand Down Expand Up @@ -976,11 +984,16 @@ describe("NotificationBackground", () => {
});
notificationBackground["notificationQueue"] = [queueMessage];
const cipherView = mock<CipherView>({
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();
Expand All @@ -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" });
});

Expand Down
25 changes: 20 additions & 5 deletions apps/browser/src/autofill/background/notification.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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", {
Expand Down Expand Up @@ -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),
});
}
}
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { NotificationConfirmationBody } from "../../notification/confirmation";
type Args = {
buttonText: string;
confirmationMessage: string;
handleClick: () => void;
handleOpenVault: () => void;
theme: Theme;
error: string;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -41,9 +45,10 @@ export function NotificationConfirmationContainer({
theme,
})}
${NotificationConfirmationBody({
error: error,
buttonText,
confirmationMessage,
error: error,
handleOpenVault,
theme,
})}
</div>
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -18,20 +18,22 @@ 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`
<div class=${notificationConfirmationBodyStyles({ theme })}>
<div class=${iconContainerStyles(error)}>${IconComponent({ theme })}</div>
${confirmationMessage && buttonText
? NotificationConfirmationMessage({
handleClick: () => {},
handleClick: handleOpenVault,
confirmationMessage,
theme,
buttonText,
Expand Down
2 changes: 2 additions & 0 deletions apps/browser/src/autofill/content/notification-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ async function loadNotificationBar() {
{
command: "saveCipherAttemptCompleted",
error: msg.data?.error,
username: msg.data?.username,
cipherId: msg.data?.cipherId,
},
"*",
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ type NotificationBarIframeInitData = {
};

type NotificationBarWindowMessage = {
[key: string]: any;
command: string;
error?: string;
initData?: NotificationBarIframeInitData;
username?: string;
cipherId?: string;
};

type NotificationBarWindowMessageHandlers = {
Expand Down
16 changes: 13 additions & 3 deletions apps/browser/src/autofill/notification/bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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",
Expand Down Expand Up @@ -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);

Expand All @@ -305,6 +313,8 @@ function handleSaveCipherConfirmation(message: NotificationBarWindowMessage) {
handleCloseNotification,
i18n,
error,
username,
handleOpenVault: (e) => openViewVaultItemPopout(e, cipherId),
}),
document.body,
);
Expand Down

0 comments on commit f12456b

Please sign in to comment.