[PM-14909] Add data/state for security task completion notification (#14279)

* include tasks with notification cipher data * send security task information with update success message for notification * mark completed cipher updates with tasks as complete * refactor notification confirmation components and add stories * add keyhole icon * add conditional footer button to notification confirmation component * add external link icon * add external link icon to action button * add notification confirmation footer story * use keyhole icon if there are no additional security tasks to complete * add new message catalog entries to chrome.i18n * reimplement sending security task information with update success message for notification * open tasks in extension from confirmation notification button * update vault message key and dismiss all security tasks for a given cipher upon password update * resolve changes against updated main branch basis * put task fetching behind feature flag and update tests * cleanup * more cleanup
2025-12-06 00:13:28 +00:00 · 2025-04-15 14:37:12 -04:00
parent f74d7e5fd5
commit e3d1ef456e
8 changed files with 247 additions and 41 deletions
--- a/apps/browser/src/autofill/background/abstractions/notification.background.ts
+++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts
@@ -95,6 +95,7 @@ type NotificationBackgroundExtensionMessageHandlers = {
  unlockCompleted: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
  bgGetFolderData: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<FolderView[]>;
  bgCloseNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
+  bgOpenAtRisksPasswords: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
  bgAdjustNotificationBar: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
  bgAddLogin: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
  bgChangedPassword: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise<void>;
--- a/apps/browser/src/autofill/background/notification.background.spec.ts
+++ b/apps/browser/src/autofill/background/notification.background.spec.ts
@@ -1,5 +1,5 @@
 import { mock, MockProxy } from "jest-mock-extended";
-import { BehaviorSubject, firstValueFrom } from "rxjs";
+import { BehaviorSubject, firstValueFrom, of } from "rxjs";

 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { DefaultPolicyService } from "@bitwarden/common/admin-console/services/policy/default-policy.service";
@@ -12,6 +12,7 @@ import { UserNotificationSettingsService } from "@bitwarden/common/autofill/serv
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 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";
@@ -19,6 +20,7 @@ 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";
 import { FolderService } from "@bitwarden/common/vault/services/folder/folder.service";
+import { TaskService, SecurityTask } from "@bitwarden/common/vault/tasks";

 import { BrowserApi } from "../../platform/browser/browser-api";
 import { NotificationQueueMessageType } from "../enums/notification-queue-message-type.enum";
@@ -46,6 +48,8 @@ jest.mock("rxjs", () => {
 });

 describe("NotificationBackground", () => {
+  const messagingService = mock<MessagingService>();
+  const taskService = mock<TaskService>();
  let notificationBackground: NotificationBackground;
  const autofillService = mock<AutofillService>();
  const cipherService = mock<CipherService>();
@@ -88,6 +92,8 @@ describe("NotificationBackground", () => {
      policyService,
      themeStateService,
      userNotificationSettingsService,
+      taskService,
+      messagingService,
    );
  });

@@ -201,8 +207,8 @@ describe("NotificationBackground", () => {
        await flushPromises();

        expect(notificationBackground["handleSaveCipherMessage"]).toHaveBeenCalledWith(
-          message.data.commandToRetry.message,
-          message.data.commandToRetry.sender,
+          message.data?.commandToRetry?.message,
+          message.data?.commandToRetry?.sender,
        );
      });
    });
@@ -498,7 +504,7 @@ describe("NotificationBackground", () => {
        expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith(
          null,
          "example.com",
-          message.data.newPassword,
+          message.data?.newPassword,
          sender.tab,
          true,
        );
@@ -570,7 +576,7 @@ describe("NotificationBackground", () => {
        expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith(
          "cipher-id",
          "example.com",
-          message.data.newPassword,
+          message.data?.newPassword,
          sender.tab,
        );
      });
@@ -618,7 +624,7 @@ describe("NotificationBackground", () => {
        expect(pushChangePasswordToQueueSpy).toHaveBeenCalledWith(
          "cipher-id",
          "example.com",
-          message.data.newPassword,
+          message.data?.newPassword,
          sender.tab,
        );
      });
@@ -844,6 +850,86 @@ describe("NotificationBackground", () => {
          );
        });

+        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";
+          const mockSecurityTask = {
+            id: "testTaskId",
+            organizationId: mockOrgId,
+            cipherId: mockCipherId,
+            type: 0,
+            status: 0,
+            creationDate: new Date(),
+            revisionDate: new Date(),
+          } as SecurityTask;
+          const mockSecurityTask2 = {
+            ...mockSecurityTask,
+            id: "testTaskId2",
+            cipherId: "testId2",
+          } as SecurityTask;
+          taskService.tasksEnabled$.mockImplementation(() => of(true));
+          taskService.pendingTasks$.mockImplementation(() =>
+            of([mockSecurityTask, mockSecurityTask2]),
+          );
+          jest
+            .spyOn(notificationBackground as any, "getNotificationFlag")
+            .mockResolvedValueOnce(true);
+          jest.spyOn(notificationBackground as any, "getOrgData").mockResolvedValueOnce([
+            {
+              id: mockOrgId,
+              name: "Org Name, LLC",
+              productTierType: 3,
+            },
+          ]);
+
+          const tab = createChromeTabMock({ id: 1, url: "https://example.com" });
+          const sender = mock<chrome.runtime.MessageSender>({ tab });
+          const message: NotificationBackgroundExtensionMessage = {
+            command: "bgSaveCipher",
+            edit: false,
+            folder: "folder-id",
+          };
+          const queueMessage = mock<AddChangePasswordQueueMessage>({
+            type: NotificationQueueMessageType.ChangePassword,
+            tab,
+            domain: "example.com",
+            newPassword: "newPassword",
+          });
+          notificationBackground["notificationQueue"] = [queueMessage];
+          const cipherView = mock<CipherView>({
+            id: mockCipherId,
+            organizationId: mockOrgId,
+            login: { username: "testUser" },
+          });
+          getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView);
+
+          sendMockExtensionMessage(message, sender);
+          await flushPromises();
+
+          expect(editItemSpy).not.toHaveBeenCalled();
+          expect(createWithServerSpy).not.toHaveBeenCalled();
+          expect(updatePasswordSpy).toHaveBeenCalledWith(
+            cipherView,
+            queueMessage.newPassword,
+            message.edit,
+            sender.tab,
+            mockCipherId,
+          );
+          expect(updateWithServerSpy).toHaveBeenCalled();
+          expect(tabSendMessageDataSpy).toHaveBeenCalledWith(
+            sender.tab,
+            "saveCipherAttemptCompleted",
+            {
+              cipherId: "testId",
+              task: {
+                orgName: "Org Name, LLC",
+                remainingTasksCount: 1,
+              },
+              username: "testUser",
+            },
+          );
+        });
+
        it("updates the cipher password if the queue message was locked and an existing cipher has the same username as the message", async () => {
          const tab = createChromeTabMock({ id: 1, url: "https://example.com" });
          const sender = mock<chrome.runtime.MessageSender>({ tab });
--- a/apps/browser/src/autofill/background/notification.background.ts
+++ b/apps/browser/src/autofill/background/notification.background.ts
@@ -1,6 +1,6 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
-import { firstValueFrom, switchMap } from "rxjs";
+import { firstValueFrom, switchMap, map } from "rxjs";

 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
@@ -22,16 +22,21 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co
 import { ServerConfig } from "@bitwarden/common/platform/abstractions/config/server-config";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service";
 import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherType } from "@bitwarden/common/vault/enums";
+import { VaultMessages } from "@bitwarden/common/vault/enums/vault-messages.enum";
 import { buildCipherIcon } from "@bitwarden/common/vault/icon/build-cipher-icon";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view";
 import { LoginView } from "@bitwarden/common/vault/models/view/login.view";
+import { TaskService } from "@bitwarden/common/vault/tasks";
+import { SecurityTaskType } from "@bitwarden/common/vault/tasks/enums";
+import { SecurityTask } from "@bitwarden/common/vault/tasks/models/security-task";

 import { openUnlockPopout } from "../../auth/popup/utils/auth-popout-window";
 import { BrowserApi } from "../../platform/browser/browser-api";
@@ -70,6 +75,8 @@ export default class NotificationBackground {
    bgChangedPassword: ({ message, sender }) => this.changedPassword(message, sender),
    bgCloseNotificationBar: ({ message, sender }) =>
      this.handleCloseNotificationBarMessage(message, sender),
+    bgOpenAtRisksPasswords: ({ message, sender }) =>
+      this.handleOpenAtRisksPasswordsMessage(message, sender),
    bgGetActiveUserServerConfig: () => this.getActiveUserServerConfig(),
    bgGetDecryptedCiphers: () => this.getNotificationCipherData(),
    bgGetEnableChangedPasswordPrompt: () => this.getEnableChangedPasswordPrompt(),
@@ -106,6 +113,8 @@ export default class NotificationBackground {
    private policyService: PolicyService,
    private themeStateService: ThemeStateService,
    private userNotificationSettingsService: UserNotificationSettingsServiceAbstraction,
+    private taskService: TaskService,
+    protected messagingService: MessagingService,
  ) {}

  init() {
@@ -154,17 +163,20 @@ export default class NotificationBackground {
      firstValueFrom(this.domainSettingsService.showFavicons$),
      firstValueFrom(this.environmentService.environment$),
    ]);
+
    const iconsServerUrl = env.getIconsUrl();
    const activeUserId = await firstValueFrom(
      this.accountService.activeAccount$.pipe(getOptionalUserId),
    );
+
    const decryptedCiphers = await this.cipherService.getAllDecryptedForUrl(
-      currentTab.url,
+      currentTab?.url,
      activeUserId,
    );

    return decryptedCiphers.map((view) => {
      const { id, name, reprompt, favorite, login } = view;
+
      return {
        id,
        name,
@@ -599,13 +611,13 @@ export default class NotificationBackground {
      try {
        await this.cipherService.createWithServer(cipher);
        await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", {
-          username: String(queueMessage?.username),
-          cipherId: String(cipher?.id),
+          username: queueMessage?.username && String(queueMessage.username),
+          cipherId: cipher?.id && String(cipher.id),
        });
        await BrowserApi.tabSendMessage(tab, { command: "addedCipher" });
      } catch (error) {
        await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", {
-          error: String(error.message),
+          error: error?.message && String(error.message),
        });
      }
    }
@@ -638,15 +650,49 @@ export default class NotificationBackground {
      return;
    }
    const cipher = await this.cipherService.encrypt(cipherView, userId);
+
+    const shouldGetTasks = await this.getNotificationFlag();
+
    try {
+      const tasks = shouldGetTasks ? await this.getSecurityTasks(userId) : [];
+      const updatedCipherTask = tasks.find((task) => task.cipherId === cipherView?.id);
+      const cipherHasTask = !!updatedCipherTask?.id;
+
+      let taskOrgName: string;
+      if (cipherHasTask && updatedCipherTask?.organizationId) {
+        const userOrgs = await this.getOrgData();
+        taskOrgName = userOrgs.find(({ id }) => id === updatedCipherTask.organizationId)?.name;
+      }
+
+      const taskData = cipherHasTask
+        ? {
+            remainingTasksCount: tasks.length - 1,
+            orgName: taskOrgName,
+          }
+        : undefined;
+
      await this.cipherService.updateWithServer(cipher);
+
      await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", {
-        username: String(cipherView?.login?.username),
-        cipherId: String(cipherView?.id),
+        username: cipherView?.login?.username && String(cipherView.login.username),
+        cipherId: cipherView?.id && String(cipherView.id),
+        task: taskData,
      });
+
+      // If the cipher had a security task, mark it as complete
+      if (cipherHasTask) {
+        // guard against multiple (redundant) security tasks per cipher
+        await Promise.all(
+          tasks.map((task) => {
+            if (task.cipherId === cipherView?.id) {
+              return this.taskService.markAsComplete(task.id, userId);
+            }
+          }),
+        );
+      }
    } catch (error) {
      await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", {
-        error: String(error?.message),
+        error: error?.message && String(error.message),
      });
    }
  }
@@ -699,6 +745,32 @@ export default class NotificationBackground {
    return null;
  }

+  private async getSecurityTasks(userId: UserId) {
+    let tasks: SecurityTask[] = [];
+
+    if (userId) {
+      tasks = await firstValueFrom(
+        this.taskService.tasksEnabled$(userId).pipe(
+          switchMap((tasksEnabled) => {
+            if (!tasksEnabled) {
+              return [];
+            }
+
+            return this.taskService
+              .pendingTasks$(userId)
+              .pipe(
+                map((tasks) =>
+                  tasks.filter(({ type }) => type === SecurityTaskType.UpdateAtRiskCredential),
+                ),
+              );
+          }),
+        ),
+      );
+    }
+
+    return tasks;
+  }
+
  /**
   * Saves the current tab's domain to the never save list.
   *
@@ -819,6 +891,41 @@ export default class NotificationBackground {
    });
  }

+  /**
+   * Sends a message to the background to open the
+   * at-risk passwords extension view. Triggers
+   * notification closure as a side-effect.
+   *
+   * @param message - The extension message
+   * @param sender - The contextual sender of the message
+   */
+  private async handleOpenAtRisksPasswordsMessage(
+    message: NotificationBackgroundExtensionMessage,
+    sender: chrome.runtime.MessageSender,
+  ) {
+    const browserAction = BrowserApi.getBrowserAction();
+
+    try {
+      // Set route of the popup before attempting to open it.
+      // If the vault is locked, this won't have an effect as the auth guards will
+      // redirect the user to the login page.
+      await browserAction.setPopup({ popup: "popup/index.html#/at-risk-passwords" });
+
+      await Promise.all([
+        this.messagingService.send(VaultMessages.OpenAtRiskPasswords),
+        BrowserApi.tabSendMessageData(sender.tab, "closeNotificationBar", {
+          fadeOutNotification: !!message.fadeOutNotification,
+        }),
+      ]);
+    } finally {
+      // Reset the popup route to the default route so any subsequent
+      // popup openings will not open to the at-risk-passwords page.
+      await browserAction.setPopup({
+        popup: "popup/index.html#/",
+      });
+    }
+  }
+
  /**
   * Sends a message back to the sender tab which triggers
   * an CSS adjustment of the notification bar.
--- a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts
+++ b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts
@@ -29,11 +29,14 @@ type NotificationBarIframeInitData = {
 };

 type NotificationBarWindowMessage = {
-  cipherId?: string;
  command: string;
+  data?: {
+    cipherId?: string;
+    task?: NotificationTaskInfo;
+    username?: string;
+  };
  error?: string;
  initData?: NotificationBarIframeInitData;
-  username?: string;
 };

 type NotificationBarWindowMessageHandlers = {
--- a/apps/browser/src/autofill/notification/bar.ts
+++ b/apps/browser/src/autofill/notification/bar.ts
@@ -356,7 +356,8 @@ function openViewVaultItemPopout(e: Event, cipherId: string) {

 function handleSaveCipherConfirmation(message: NotificationBarWindowMessage) {
  const { theme, type } = notificationBarIframeInitData;
-  const { error, username, cipherId } = message;
+  const { error, data } = message;
+  const { username, cipherId, task } = data || {};
  const i18n = getI18n();
  const resolvedTheme = getResolvedTheme(theme ?? ThemeTypes.Light);

@@ -371,8 +372,9 @@ function handleSaveCipherConfirmation(message: NotificationBarWindowMessage) {
      i18n,
      error,
      username: username ?? i18n.typeLogin,
+      task,
      handleOpenVault: (e) => cipherId && openViewVaultItemPopout(e, cipherId),
-      handleOpenTasks: () => {},
+      handleOpenTasks: () => sendPlatformMessage({ command: "bgOpenAtRisksPasswords" }),
    }),
    document.body,
  );
--- a/apps/browser/src/autofill/overlay/notifications/content/overlay-notifications-content.service.spec.ts
+++ b/apps/browser/src/autofill/overlay/notifications/content/overlay-notifications-content.service.spec.ts
@@ -23,8 +23,8 @@ describe("OverlayNotificationsContentService", () => {
    autofillInit = new AutofillInit(
      domQueryService,
      domElementVisibilityService,
-      null,
-      null,
+      undefined,
+      undefined,
      overlayNotificationsContentService,
    );
    autofillInit.init();
@@ -89,7 +89,7 @@ describe("OverlayNotificationsContentService", () => {
      await flushPromises();

      expect(
-        overlayNotificationsContentService["notificationBarIframeElement"].style.transform,
+        overlayNotificationsContentService["notificationBarIframeElement"]?.style.transform,
      ).toBe("translateX(100%)");
    });

@@ -103,12 +103,12 @@ describe("OverlayNotificationsContentService", () => {
      });
      await flushPromises();

-      overlayNotificationsContentService["notificationBarIframeElement"].dispatchEvent(
+      overlayNotificationsContentService["notificationBarIframeElement"]?.dispatchEvent(
        new Event("load"),
      );

      expect(
-        overlayNotificationsContentService["notificationBarIframeElement"].style.transform,
+        overlayNotificationsContentService["notificationBarIframeElement"]?.style.transform,
      ).toBe("translateX(0)");
    });

@@ -134,7 +134,7 @@ describe("OverlayNotificationsContentService", () => {
      globalThis.dispatchEvent(
        new MessageEvent("message", {
          data: { command: "initNotificationBar" },
-          source: overlayNotificationsContentService["notificationBarIframeElement"].contentWindow,
+          source: overlayNotificationsContentService["notificationBarIframeElement"]?.contentWindow,
        }),
      );
      await flushPromises();
@@ -168,9 +168,9 @@ describe("OverlayNotificationsContentService", () => {
        data: { fadeOutNotification: true },
      });

-      expect(overlayNotificationsContentService["notificationBarIframeElement"].style.opacity).toBe(
-        "0",
-      );
+      expect(
+        overlayNotificationsContentService["notificationBarIframeElement"]?.style.opacity,
+      ).toBe("0");

      jest.advanceTimersByTime(150);

@@ -210,7 +210,7 @@ describe("OverlayNotificationsContentService", () => {
        data: { height: 1000 },
      });

-      expect(overlayNotificationsContentService["notificationBarElement"].style.height).toBe(
+      expect(overlayNotificationsContentService["notificationBarElement"]?.style.height).toBe(
        "1000px",
      );
    });
@@ -236,13 +236,13 @@ describe("OverlayNotificationsContentService", () => {

      sendMockExtensionMessage({
        command: "saveCipherAttemptCompleted",
-        data: { error: "" },
+        data: { error: undefined },
      });

      expect(
        overlayNotificationsContentService["notificationBarIframeElement"].contentWindow
          .postMessage,
-      ).toHaveBeenCalledWith({ command: "saveCipherAttemptCompleted", error: "" }, "*");
+      ).toHaveBeenCalledWith({ command: "saveCipherAttemptCompleted", error: undefined }, "*");
    });
  });

--- 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
@@ -135,9 +135,13 @@ export class OverlayNotificationsContentService
   * @private
   */
  private handleSaveCipherAttemptCompletedMessage(message: NotificationsExtensionMessage) {
+    // destructure error out of data
+    const { error, ...otherData } = message?.data || {};
+
    this.sendMessageToNotificationBarIframe({
      command: "saveCipherAttemptCompleted",
-      error: message.data?.error,
+      data: Object.keys(otherData).length ? otherData : undefined,
+      error,
    });
  }

--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@@ -1186,6 +1186,17 @@ export default class MainBackground {
      this.authService,
      () => this.generatePasswordToClipboard(),
    );
+
+    this.taskService = new DefaultTaskService(
+      this.stateProvider,
+      this.apiService,
+      this.organizationService,
+      this.configService,
+      this.authService,
+      this.notificationsService,
+      messageListener,
+    );
+
    this.notificationBackground = new NotificationBackground(
      this.accountService,
      this.authService,
@@ -1200,6 +1211,8 @@ export default class MainBackground {
      this.policyService,
      this.themeStateService,
      this.userNotificationSettingsService,
+      this.taskService,
+      this.messagingService,
    );

    this.overlayNotificationsBackground = new OverlayNotificationsBackground(
@@ -1304,16 +1317,6 @@ export default class MainBackground {
      this.configService,
    );

-    this.taskService = new DefaultTaskService(
-      this.stateProvider,
-      this.apiService,
-      this.organizationService,
-      this.configService,
-      this.authService,
-      this.notificationsService,
-      messageListener,
-    );
-
    this.inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService();

    this.ipcContentScriptManagerService = new IpcContentScriptManagerService(this.configService);