diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts index a9031ea2630..28be3439f24 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts +++ b/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); diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts index 349ea136ecf..69bce9c47d2 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts +++ b/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,21 +140,39 @@ 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); this.showFooter$ = of( @@ -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 { 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( - this.cipher.login.totp, - "totp", - this.cipher, - ); + actionSuccess = await this.copyCipherFieldService.copy(cipher.login.totp, "totp", 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; } }