[PM-1345] Bugfix - Items with "Re-prompt Masterpassword" fail silently (#5621)

* upon action outside of the extenstion requiring password reprompt, open new tab with reprompt * allow popup view component to load with default action and send context menu actions on reprompt ciphers to password reprompt * open password reprompt in new window instead of new tab * update test and linting * Apply suggestions from code review Co-authored-by: Cesar Gonzalez <cesar.a.gonzalezcs@gmail.com> * add support for getTab in Manifest V2 * remove unneeded loadAction check * allow auto-fill button in popout window * add LoadAction type * update code to use new BrowserPopoutWindowService * access queryParams with subscribe * do not dismiss window if no loadAction was specified * rehide autofill option for non-single-action popout windows --------- Co-authored-by: Cesar Gonzalez <cesar.a.gonzalezcs@gmail.com>
2024-12-21 16:18:28 +01:00 · 2023-08-15 17:27:59 -04:00 · 2023-08-15 17:27:59 -04:00 · d95f1163bf
commit d95f1163bf
parent 41bf1247ef
11 changed files with 191 additions and 31 deletions
--- a/apps/browser/src/autofill/browser/cipher-context-menu-handler.spec.ts
+++ b/apps/browser/src/autofill/browser/cipher-context-menu-handler.spec.ts
@ -96,7 +96,7 @@ describe("CipherContextMenuHandler", () => {

      expect(cipherService.getAllDecryptedForUrl).toHaveBeenCalledWith("https://test.com");

-      expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledTimes(1);
+      expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledTimes(2);

      expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledWith(
        "Test Cipher (Test Username)",
--- a/apps/browser/src/autofill/browser/cipher-context-menu-handler.ts
+++ b/apps/browser/src/autofill/browser/cipher-context-menu-handler.ts
@ -4,7 +4,6 @@ import { StateFactory } from "@bitwarden/common/platform/factories/state-factory
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
-import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type";
 import { CipherType } from "@bitwarden/common/vault/enums/cipher-type";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";

@ -177,11 +176,7 @@ export class CipherContextMenuHandler {
  }

  private async updateForCipher(url: string, cipher: CipherView) {
-    if (
-      cipher == null ||
-      cipher.type !== CipherType.Login ||
-      cipher.reprompt !== CipherRepromptType.None
-    ) {
+    if (cipher == null || cipher.type !== CipherType.Login) {
      return;
    }

--- a/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts
+++ b/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts
@ -203,17 +203,45 @@ export class ContextMenuClickedHandler {
        if (tab == null) {
          return;
        }
-        await this.autofillAction(tab, cipher);
+
+        if (cipher.reprompt !== CipherRepromptType.None) {
+          await BrowserApi.tabSendMessageData(tab, "passwordReprompt", {
+            cipherId: cipher.id,
+            action: AUTOFILL_ID,
+          });
+        } else {
+          await this.autofillAction(tab, cipher);
+        }
+
        break;
      case COPY_USERNAME_ID:
        this.copyToClipboard({ text: cipher.login.username, tab: tab });
        break;
      case COPY_PASSWORD_ID:
-        this.copyToClipboard({ text: cipher.login.password, tab: tab });
-        this.eventCollectionService.collect(EventType.Cipher_ClientCopiedPassword, cipher.id);
+        if (cipher.reprompt !== CipherRepromptType.None) {
+          await BrowserApi.tabSendMessageData(tab, "passwordReprompt", {
+            cipherId: cipher.id,
+            action: COPY_PASSWORD_ID,
+          });
+        } else {
+          this.copyToClipboard({ text: cipher.login.password, tab: tab });
+          this.eventCollectionService.collect(EventType.Cipher_ClientCopiedPassword, cipher.id);
+        }
+
        break;
      case COPY_VERIFICATIONCODE_ID:
-        this.copyToClipboard({ text: await this.totpService.getCode(cipher.login.totp), tab: tab });
+        if (cipher.reprompt !== CipherRepromptType.None) {
+          await BrowserApi.tabSendMessageData(tab, "passwordReprompt", {
+            cipherId: cipher.id,
+            action: COPY_VERIFICATIONCODE_ID,
+          });
+        } else {
+          this.copyToClipboard({
+            text: await this.totpService.getCode(cipher.login.totp),
+            tab: tab,
+          });
+        }
+
        break;
    }
  }
--- a/apps/browser/src/autofill/content/message_handler.ts
+++ b/apps/browser/src/autofill/content/message_handler.ts
@ -28,6 +28,7 @@ window.addEventListener(

 const forwardCommands = [
  "promptForLogin",
+  "passwordReprompt",
  "addToLockedVaultPendingNotifications",
  "unlockCompleted",
  "addedCipher",
--- a/apps/browser/src/autofill/services/autofill.service.ts
+++ b/apps/browser/src/autofill/services/autofill.service.ts
@ -234,7 +234,16 @@ export default class AutofillService implements AutofillServiceInterface {
      }
    }

-    if (cipher == null || cipher.reprompt !== CipherRepromptType.None) {
+    if (cipher == null) {
+      return null;
+    }
+
+    if (cipher.reprompt !== CipherRepromptType.None) {
+      await BrowserApi.tabSendMessageData(tab, "passwordReprompt", {
+        cipherId: cipher.id,
+        action: "autofill",
+      });
+
      return null;
    }

--- a/apps/browser/src/background/runtime.background.ts
+++ b/apps/browser/src/background/runtime.background.ts
@ -61,6 +61,8 @@ export default class RuntimeBackground {
  }

  async processMessage(msg: any, sender: chrome.runtime.MessageSender, sendResponse: any) {
+    const cipherId = msg.data?.cipherId;
+
    switch (msg.command) {
      case "loggedIn":
      case "unlocked": {
@ -68,7 +70,7 @@ export default class RuntimeBackground {

        if (this.lockedVaultPendingNotifications?.length > 0) {
          item = this.lockedVaultPendingNotifications.pop();
-          await this.browserPopoutWindowService.closeLoginPrompt();
+          await this.browserPopoutWindowService.closeUnlockPrompt();
        }

        await this.main.refreshBadge();
@ -108,13 +110,22 @@ export default class RuntimeBackground {
        break;
      case "promptForLogin":
      case "bgReopenPromptForLogin":
-        await this.browserPopoutWindowService.openLoginPrompt(sender.tab?.windowId);
+        await this.browserPopoutWindowService.openUnlockPrompt(sender.tab?.windowId);
+        break;
+      case "passwordReprompt":
+        if (cipherId) {
+          await this.browserPopoutWindowService.openPasswordRepromptPrompt(sender.tab?.windowId, {
+            cipherId: cipherId,
+            senderTabId: sender.tab.id,
+            action: msg.data?.action,
+          });
+        }
        break;
      case "openAddEditCipher": {
        const addEditCipherUrl =
-          msg.data?.cipherId == null
+          cipherId == null
            ? "popup/index.html#/edit-cipher"
-            : "popup/index.html#/edit-cipher?cipherId=" + msg.data.cipherId;
+            : "popup/index.html#/edit-cipher?cipherId=" + cipherId;

        BrowserApi.openBitwardenExtensionTab(addEditCipherUrl, true);
        break;
--- a/apps/browser/src/platform/browser/browser-api.ts
+++ b/apps/browser/src/platform/browser/browser-api.ts
@ -42,11 +42,20 @@ export class BrowserApi {
    });
  }

-  static async getTab(tabId: number) {
-    if (tabId == null) {
+  static async getTab(tabId: number): Promise<chrome.tabs.Tab> | null {
+    if (!tabId) {
      return null;
    }
-    return await chrome.tabs.get(tabId);
+
+    if (BrowserApi.manifestVersion === 3) {
+      return await chrome.tabs.get(tabId);
+    }
+
+    return new Promise((resolve) =>
+      chrome.tabs.get(tabId, (tab) => {
+        resolve(tab);
+      })
+    );
  }

  static async getTabFromCurrentWindow(): Promise<chrome.tabs.Tab> | null {
--- a/apps/browser/src/platform/popup/abstractions/browser-popout-window.service.ts
+++ b/apps/browser/src/platform/popup/abstractions/browser-popout-window.service.ts
@ -1,6 +1,15 @@
 interface BrowserPopoutWindowService {
-  openLoginPrompt(senderWindowId: number): Promise<void>;
-  closeLoginPrompt(): Promise<void>;
+  openUnlockPrompt(senderWindowId: number): Promise<void>;
+  closeUnlockPrompt(): Promise<void>;
+  openPasswordRepromptPrompt(
+    senderWindowId: number,
+    promptData: {
+      action: string;
+      cipherId: string;
+      senderTabId: number;
+    }
+  ): Promise<void>;
+  closePasswordRepromptPrompt(): Promise<void>;
 }

 export { BrowserPopoutWindowService };
--- a/apps/browser/src/platform/popup/browser-popout-window.service.ts
+++ b/apps/browser/src/platform/popup/browser-popout-window.service.ts
@ -11,20 +11,48 @@ class BrowserPopoutWindowService implements BrowserPopupWindowServiceInterface {
    height: 800,
  };

-  async openLoginPrompt(senderWindowId: number) {
-    await this.closeLoginPrompt();
-    await this.openPopoutWindow(
+  async openUnlockPrompt(senderWindowId: number) {
+    await this.closeUnlockPrompt();
+    await this.openSingleActionPopout(
      senderWindowId,
      "popup/index.html?uilocation=popout",
-      "loginPrompt"
+      "unlockPrompt"
    );
  }

-  async closeLoginPrompt() {
-    await this.closeSingleActionPopout("loginPrompt");
+  async closeUnlockPrompt() {
+    await this.closeSingleActionPopout("unlockPrompt");
  }

-  private async openPopoutWindow(
+  async openPasswordRepromptPrompt(
+    senderWindowId: number,
+    {
+      cipherId,
+      senderTabId,
+      action,
+    }: {
+      cipherId: string;
+      senderTabId: number;
+      action: string;
+    }
+  ) {
+    await this.closePasswordRepromptPrompt();
+
+    const promptWindowPath =
+      "popup/index.html#/view-cipher" +
+      "?uilocation=popout" +
+      `&cipherId=${cipherId}` +
+      `&senderTabId=${senderTabId}` +
+      `&action=${action}`;
+
+    await this.openSingleActionPopout(senderWindowId, promptWindowPath, "passwordReprompt");
+  }
+
+  async closePasswordRepromptPrompt() {
+    await this.closeSingleActionPopout("passwordReprompt");
+  }
+
+  private async openSingleActionPopout(
    senderWindowId: number,
    popupWindowURL: string,
    singleActionPopoutKey: string
--- a/apps/browser/src/vault/popup/components/vault/view.component.html
+++ b/apps/browser/src/vault/popup/components/vault/view.component.html
@ -555,7 +555,11 @@
        class="box-content-row"
        appStopClick
        (click)="fillCipher()"
-        *ngIf="cipher.type !== cipherType.SecureNote && !cipher.isDeleted && !inPopout"
+        *ngIf="
+          cipher.type !== cipherType.SecureNote &&
+          !cipher.isDeleted &&
+          (!this.inPopout || this.loadAction)
+        "
      >
        <div class="row-main text-primary">
          <div class="icon text-primary" aria-hidden="true">
--- a/apps/browser/src/vault/popup/components/vault/view.component.ts
+++ b/apps/browser/src/vault/popup/components/vault/view.component.ts
@ -1,6 +1,7 @@
 import { Location } from "@angular/common";
 import { ChangeDetectorRef, Component, NgZone } from "@angular/core";
 import { ActivatedRoute, Router } from "@angular/router";
+import { Subject, takeUntil } from "rxjs";
 import { first } from "rxjs/operators";

 import { DialogServiceAbstraction } from "@bitwarden/angular/services/dialog";
@ -31,6 +32,17 @@ import { PopupUtilsService } from "../../../../popup/services/popup-utils.servic

 const BroadcasterSubscriptionId = "ChildViewComponent";

+export const AUTOFILL_ID = "autofill";
+export const COPY_USERNAME_ID = "copy-username";
+export const COPY_PASSWORD_ID = "copy-password";
+export const COPY_VERIFICATIONCODE_ID = "copy-totp";
+
+type LoadAction =
+  | typeof AUTOFILL_ID
+  | typeof COPY_USERNAME_ID
+  | typeof COPY_PASSWORD_ID
+  | typeof COPY_VERIFICATIONCODE_ID;
+
@Component({
  selector: "app-vault-view",
  templateUrl: "view.component.html",
@ -39,10 +51,15 @@ export class ViewComponent extends BaseViewComponent {
  showAttachments = true;
  pageDetails: any[] = [];
  tab: any;
+  senderTabId?: number;
+  loadAction?: LoadAction;
+  uilocation?: "popout" | "popup" | "sidebar" | "tab";
  loadPageDetailsTimeout: number;
  inPopout = false;
  cipherType = CipherType;

+  private destroy$ = new Subject<void>();
+
  constructor(
    cipherService: CipherService,
    folderService: FolderService,
@ -93,7 +110,14 @@ export class ViewComponent extends BaseViewComponent {
  }

  ngOnInit() {
-    this.inPopout = this.popupUtilsService.inPopout(window);
+    this.route.queryParams.pipe(takeUntil(this.destroy$)).subscribe((value) => {
+      this.loadAction = value?.action;
+      this.senderTabId = parseInt(value?.senderTabId, 10) || undefined;
+      this.uilocation = value?.uilocation;
+    });
+
+    this.inPopout = this.uilocation === "popout" || this.popupUtilsService.inPopout(window);
+
    // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe
    this.route.queryParams.pipe(first()).subscribe(async (params) => {
      if (params.cipherId) {
@ -134,6 +158,8 @@ export class ViewComponent extends BaseViewComponent {
  }

  ngOnDestroy() {
+    this.destroy$.next();
+    this.destroy$.complete();
    super.ngOnDestroy();
    this.broadcasterService.unsubscribe(BroadcasterSubscriptionId);
  }
@ -141,6 +167,27 @@ export class ViewComponent extends BaseViewComponent {
  async load() {
    await super.load();
    await this.loadPageDetails();
+
+    switch (this.loadAction) {
+      case AUTOFILL_ID:
+        this.fillCipher();
+        return;
+      case COPY_USERNAME_ID:
+        await this.copy(this.cipher.login.username, "username", "Username");
+        break;
+      case COPY_PASSWORD_ID:
+        await this.copy(this.cipher.login.password, "password", "Password");
+        break;
+      case COPY_VERIFICATIONCODE_ID:
+        await this.copy(this.cipher.login.totp, "verificationCodeTotp", "TOTP");
+        break;
+      default:
+        break;
+    }
+
+    if (this.inPopout && this.loadAction) {
+      this.close();
+    }
  }

  async edit() {
@ -191,6 +238,10 @@ export class ViewComponent extends BaseViewComponent {
    const didAutofill = await this.doAutofill();
    if (didAutofill) {
      this.platformUtilsService.showToast("success", null, this.i18nService.t("autoFillSuccess"));
+
+      if (this.inPopout) {
+        this.close();
+      }
    }
  }

@ -255,15 +306,30 @@ export class ViewComponent extends BaseViewComponent {
  }

  close() {
+    if (this.senderTabId) {
+      BrowserApi.focusTab(this.senderTabId);
+    }
+
+    if (this.inPopout) {
+      window.close();
+      return;
+    }
+
    this.location.back();
  }

  private async loadPageDetails() {
    this.pageDetails = [];
    this.tab = await BrowserApi.getTabFromCurrentWindow();
-    if (this.tab == null) {
+
+    if (this.senderTabId) {
+      this.tab = await BrowserApi.getTab(this.senderTabId);
+    }
+
+    if (!this.tab) {
      return;
    }
+
    BrowserApi.tabSendMessage(this.tab, {
      command: "collectPageDetails",
      tab: this.tab,