[PM-23157] Autofill - Master Reprompt (#15431)

* delay setting `this.cipher` until reprompt is verified

* handle failures of reprompt

* do not show cipher after copy password or copy totp fails

apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts

@@ -20,7 +20,7 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
 import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
-import { CipherType } from "@bitwarden/common/vault/enums";
+import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
 import { DialogService, ToastService } from "@bitwarden/components";
@@ -241,6 +241,93 @@ describe("ViewV2Component", () => {
       expect(copy).toHaveBeenCalledTimes(1);
     }));
 
+    it("does not set the cipher until reprompt is complete", fakeAsync(() => {
+      let promptPromise: (val?: unknown) => void;
+      mockCipherService.decrypt.mockImplementationOnce(() =>
+        Promise.resolve({
+          ...mockCipher,
+          reprompt: CipherRepromptType.Password,
+        }),
+      );
+      doAutofill.mockImplementationOnce(() => {
+        return new Promise((resolve) => {
+          // store the promise resolver to manually trigger the promise resolve
+          promptPromise = resolve;
+        });
+      });
+
+      params$.next({ action: AUTOFILL_ID });
+
+      flush(); // Flush all pending actions
+
+      expect(component.cipher).toBeUndefined();
+      expect(doAutofill).toHaveBeenCalled();
+
+      promptPromise!(true); // resolve the password prompt
+
+      flush();
+      expect(component.cipher).toEqual({ ...mockCipher, reprompt: CipherRepromptType.Password });
+    }));
+
+    it("does not set the cipher at all if doAutofill fails and reprompt is active", fakeAsync(() => {
+      let promptPromise: (val?: unknown) => void;
+      mockCipherService.decrypt.mockImplementationOnce(() =>
+        Promise.resolve({
+          ...mockCipher,
+          reprompt: CipherRepromptType.Password,
+        }),
+      );
+      doAutofill.mockImplementationOnce(() => {
+        return new Promise((resolve) => {
+          // store the promise resolver to manually trigger the promise resolve
+          promptPromise = resolve;
+        });
+      });
+
+      params$.next({ action: AUTOFILL_ID });
+
+      flush(); // Flush all pending actions
+
+      expect(component.cipher).toBeUndefined();
+      expect(doAutofill).toHaveBeenCalled();
+
+      promptPromise!(false); // resolve the password prompt
+
+      flush();
+      expect(component.cipher).toBeUndefined();
+    }));
+
+    it.each([COPY_PASSWORD_ID, COPY_VERIFICATION_CODE_ID])(
+      "does not set cipher when copy fails for %s",
+      fakeAsync((action: string) => {
+        let promptPromise: (val?: unknown) => void;
+        mockCipherService.decrypt.mockImplementationOnce(() =>
+          Promise.resolve({
+            ...mockCipher,
+            reprompt: CipherRepromptType.Password,
+          }),
+        );
+        copy.mockImplementationOnce(() => {
+          return new Promise((resolve) => {
+            // store the promise resolver to manually trigger the promise resolve
+            promptPromise = resolve;
+          });
+        });
+
+        params$.next({ action });
+
+        flush(); // Flush all pending actions
+
+        expect(component.cipher).toBeUndefined();
+        expect(copy).toHaveBeenCalled();
+
+        promptPromise!(false); // resolve the password prompt
+
+        flush();
+        expect(component.cipher).toBeUndefined();
+      }),
+    );
+
     it("closes the popout after a load action", fakeAsync(() => {
       jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValueOnce(true);
       jest.spyOn(BrowserPopupUtils, "inSingleActionPopout").mockReturnValueOnce(true);

apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts

@@ -31,6 +31,7 @@ import { ViewPasswordHistoryService } from "@bitwarden/common/vault/abstractions
 import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
+import { filterOutNullish } from "@bitwarden/common/vault/utils/observable-utilities";
 import {
   AsyncActionsModule,
   ButtonModule,
@@ -139,20 +140,38 @@ export class ViewV2Component {
           this.loadAction = params.action;
           this.senderTabId = params.senderTabId ? parseInt(params.senderTabId, 10) : undefined;
 
-          const activeUserId = await firstValueFrom(
+          this.activeUserId = await firstValueFrom(
             this.accountService.activeAccount$.pipe(getUserId),
           );
-          const cipher = await this.getCipherData(params.cipherId, activeUserId);
-          return { activeUserId, cipher };
-        }),
-        switchMap(async ({ activeUserId, cipher }) => {
-          this.cipher = cipher;
-          this.headerText = this.setHeader(cipher.type);
-          this.activeUserId = activeUserId;
 
+          const cipher = await this.getCipherData(params.cipherId, this.activeUserId);
+          this.headerText = this.setHeader(cipher.type);
+
+          // Handling the load action needs to take place before setting `this.cipher`,
+          // This is important for scenarios where the action requires a password re-prompt.
+          // For those instances, no cipher details should be shown behind the re-prompt dialog until the password has been verified.
           if (this.loadAction) {
-            await this._handleLoadAction(this.loadAction, this.senderTabId);
+            const success = await this._handleLoadAction(this.loadAction, cipher, this.senderTabId);
+
+            // When the action is not successful and the cipher has a reprompt enabled,
+            // The cipher details can flash on the screen before the popout closes,
+            // pass `null` to prevent this.
+            if (
+              [AUTOFILL_ID, COPY_PASSWORD_ID, COPY_VERIFICATION_CODE_ID].includes(
+                this.loadAction,
+              ) &&
+              success === false &&
+              cipher.reprompt !== CipherRepromptType.None
+            ) {
+              return null;
             }
+          }
+
+          return cipher;
+        }),
+        filterOutNullish(),
+        switchMap(async (cipher) => {
+          this.cipher = cipher;
 
           this.canDeleteCipher$ = this.cipherAuthorizationService.canDeleteCipher$(cipher);
 
@@ -262,50 +281,46 @@ export class ViewV2Component {
    * via the extension context menu. It is necessary to render the view for items that have password
    * reprompt enabled.
    * @param loadAction
+   * @param cipher - The cipher being viewed, passed as a param because `this.cipher` may not be set yet.
    * @param senderTabId
    * @private
    */
   private async _handleLoadAction(
     loadAction: LoadAction,
+    cipher: CipherView,
     senderTabId?: number,
   ): Promise<void | boolean> {
     let actionSuccess = false;
 
-    // Both vaultPopupAutofillService and copyCipherFieldService will perform password re-prompting internally.
-
     switch (loadAction) {
       case SHOW_AUTOFILL_BUTTON:
         // This action simply shows the cipher view, no need to do anything.
         if (
-          this.cipher.reprompt !== CipherRepromptType.None &&
+          cipher.reprompt !== CipherRepromptType.None &&
           !(await this.passwordRepromptService.showPasswordPrompt())
         ) {
-          await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${this.cipher.id}`);
+          await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${cipher.id}`);
         }
         return;
       case AUTOFILL_ID:
-        actionSuccess = await this.vaultPopupAutofillService.doAutofill(this.cipher, false);
+        actionSuccess = await this.vaultPopupAutofillService.doAutofill(cipher, false);
         break;
       case COPY_USERNAME_ID:
         actionSuccess = await this.copyCipherFieldService.copy(
-          this.cipher.login.username,
+          cipher.login.username,
           "username",
-          this.cipher,
+          cipher,
         );
         break;
       case COPY_PASSWORD_ID:
         actionSuccess = await this.copyCipherFieldService.copy(
-          this.cipher.login.password,
+          cipher.login.password,
           "password",
-          this.cipher,
+          cipher,
         );
         break;
       case COPY_VERIFICATION_CODE_ID:
-        actionSuccess = await this.copyCipherFieldService.copy(
+        actionSuccess = await this.copyCipherFieldService.copy(cipher.login.totp, "totp", cipher);
-          this.cipher.login.totp,
-          "totp",
-          this.cipher,
-        );
         break;
       case UPDATE_PASSWORD: {
         const repromptSuccess = await this.passwordRepromptService.showPasswordPrompt();
@@ -315,7 +330,7 @@ export class ViewV2Component {
           success: repromptSuccess,
         });
 
-        await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${this.cipher.id}`);
+        await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${cipher.id}`);
 
         break;
       }
@@ -329,7 +344,7 @@ export class ViewV2Component {
             senderTabId
           ) {
             await BrowserApi.focusTab(senderTabId);
-            await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${this.cipher.id}`);
+            await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${cipher.id}`);
           } else {
             await this.popupRouterCacheService.back();
           }
@@ -337,5 +352,7 @@ export class ViewV2Component {
         actionSuccess ? 1000 : 0,
       );
     }
+
+    return actionSuccess;
   }
 }