From d72dd2ea7671c9d4518eb36c76b1ec85711500cd Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Wed, 8 Jan 2025 08:42:46 -0800 Subject: [PATCH] [PM-16098] Improved cipher decryption error handling (#12468) * [PM-16098] Add decryptionFailure flag to CipherView * [PM-16098] Add failedToDecryptCiphers$ observable to CipherService * [PM-16098] Introduce decryption-failure-dialog.component * [PM-16098] Disable cipher rows for the Web Vault * [PM-16098] Show decryption error dialog on vault load or when attempting to view/edit a corrupted cipher * [PM-16098] Browser - Show decryption error dialog on vault load or when attempting to view/edit a corrupted cipher * [PM-16098] Desktop - Show decryption error dialog on vault load or when attempting to view a corrupted cipher. Remove edit/clone context menu options and footer actions. * [PM-16098] Add CS link to decryption failure dialog * [PM-16098] Return cipherViews and move filtering of isDeleted to consumers * [PM-16098] Throw an error when retrieving cipher data for key rotation when a decryption failure is present * [PM-16098] Properly filter out deleted, corrupted ciphers when showing dialog within the Vault * [PM-16098] Show the decryption error dialog when attempting to view a cipher in trash and disable the restore option * [PM-16098] Exclude failed to decrypt ciphers from getAllDecrypted method and cipherViews$ observable * [PM-16098] Avoid re-sorting remainingCiphers$ as it was redundant * [PM-16098] Update tests * [PM-16098] Prevent opening view dialog in AC for corrupted ciphers * [PM-16098] Remove withLatestFrom operator that was causing race conditions when navigating away from the individual vault * [PM-16098] Ensure decryption error dialog is only shown once on Desktop when switching accounts --- apps/browser/src/_locales/en/messages.json | 14 ++++ .../item-more-options.component.html | 1 + .../vault-list-items-container.component.ts | 17 ++++- .../components/vault-v2/vault-v2.component.ts | 27 ++++++-- .../vault-popup-items.service.spec.ts | 16 +---- .../services/vault-popup-items.service.ts | 7 +- .../trash-list-items-container.component.html | 7 +- .../trash-list-items-container.component.ts | 15 +++- apps/desktop/src/app/app.module.ts | 4 +- apps/desktop/src/locales/en/messages.json | 14 ++++ .../src/vault/app/vault/vault.component.ts | 34 +++++++++- .../src/vault/app/vault/view.component.html | 56 +++++++-------- .../src/vault/app/vault/view.component.ts | 11 ++- .../vault-item-dialog.component.ts | 10 +++ .../vault-cipher-row.component.html | 23 ++++++- .../vault-items/vault-cipher-row.component.ts | 7 ++ .../vault/individual-vault/vault.component.ts | 36 +++++++++- .../app/vault/org-vault/vault.component.ts | 19 +++++- apps/web/src/locales/en/messages.json | 14 ++++ .../vault/components/vault-items.component.ts | 8 ++- .../src/vault/abstractions/cipher.service.ts | 6 ++ libs/common/src/vault/models/domain/cipher.ts | 8 ++- .../src/vault/models/view/cipher.view.ts | 5 ++ .../src/vault/services/cipher.service.spec.ts | 13 ++++ .../src/vault/services/cipher.service.ts | 68 +++++++++++++++++-- .../vault/services/key-state/ciphers.state.ts | 9 +++ .../decryption-failure-dialog.component.html | 32 +++++++++ .../decryption-failure-dialog.component.ts | 59 ++++++++++++++++ libs/vault/src/index.ts | 1 + 29 files changed, 467 insertions(+), 74 deletions(-) create mode 100644 libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.html create mode 100644 libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 55c9ae8616b..b72a909252b 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2804,6 +2804,20 @@ "error": { "message": "Error" }, + "decryptionError": { + "message": "Decryption error" + }, + "couldNotDecryptVaultItemsBelow": { + "message": "Bitwarden could not decrypt the vault item(s) listed below." + }, + "contactCSToAvoidDataLossPart1": { + "message": "Contact customer success", + "description": "This is part of a larger sentence. The full sentence will read 'Contact customer success to avoid additional data loss.'" + }, + "contactCSToAvoidDataLossPart2": { + "message": "to avoid additional data loss.", + "description": "This is part of a larger sentence. The full sentence will read 'Contact customer success to avoid additional data loss.'" + }, "generateUsername": { "message": "Generate username" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html index 7f87f32fcd4..4c7067df53a 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html @@ -5,6 +5,7 @@ size="small" [attr.aria-label]="'moreOptionsLabel' | i18n: cipher.name" [title]="'moreOptionsTitle' | i18n: cipher.name" + [disabled]="cipher.decryptionFailure" [bitMenuTriggerFor]="moreOptions" > diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts index 29c9f14e2aa..5bcdaf56bbd 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts @@ -18,19 +18,25 @@ import { map } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { CipherId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { BadgeModule, ButtonModule, CompactModeService, + DialogService, IconButtonModule, ItemModule, SectionComponent, SectionHeaderComponent, TypographyModule, } from "@bitwarden/components"; -import { OrgIconDirective, PasswordRepromptService } from "@bitwarden/vault"; +import { + DecryptionFailureDialogComponent, + OrgIconDirective, + PasswordRepromptService, +} from "@bitwarden/vault"; import { BrowserApi } from "../../../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils"; @@ -55,6 +61,7 @@ import { ItemMoreOptionsComponent } from "../item-more-options/item-more-options ItemMoreOptionsComponent, OrgIconDirective, ScrollingModule, + DecryptionFailureDialogComponent, ], selector: "app-vault-list-items-container", templateUrl: "vault-list-items-container.component.html", @@ -158,6 +165,7 @@ export class VaultListItemsContainerComponent implements AfterViewInit { private cipherService: CipherService, private router: Router, private platformUtilsService: PlatformUtilsService, + private dialogService: DialogService, ) {} async ngAfterViewInit() { @@ -209,6 +217,13 @@ export class VaultListItemsContainerComponent implements AfterViewInit { this.viewCipherTimeout = window.setTimeout( async () => { try { + if (cipher.decryptionFailure) { + DecryptionFailureDialogComponent.open(this.dialogService, { + cipherIds: [cipher.id as CipherId], + }); + return; + } + const repromptPassed = await this.passwordRepromptService.passwordRepromptCheck(cipher); if (!repromptPassed) { return; diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index 9970c115bb7..a3d8f3ffe31 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -1,15 +1,17 @@ import { ScrollingModule } from "@angular/cdk/scrolling"; import { CommonModule } from "@angular/common"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component, DestroyRef, OnDestroy, OnInit } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { RouterLink } from "@angular/router"; import { combineLatest, Observable, shareReplay, switchMap } from "rxjs"; +import { filter, map, take } from "rxjs/operators"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CipherId, CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; -import { ButtonModule, Icons, NoItemsModule } from "@bitwarden/components"; -import { VaultIcons } from "@bitwarden/vault"; +import { ButtonModule, DialogService, Icons, NoItemsModule } from "@bitwarden/components"; +import { DecryptionFailureDialogComponent, VaultIcons } from "@bitwarden/vault"; import { CurrentAccountComponent } from "../../../../auth/popup/account-switching/current-account.component"; import { PopOutComponent } from "../../../../platform/popup/components/pop-out.component"; @@ -52,6 +54,7 @@ enum VaultState { NewItemDropdownV2Component, ScrollingModule, VaultHeaderV2Component, + DecryptionFailureDialogComponent, ], providers: [VaultUiOnboardingService], }) @@ -89,6 +92,9 @@ export class VaultV2Component implements OnInit, OnDestroy { private vaultPopupItemsService: VaultPopupItemsService, private vaultPopupListFiltersService: VaultPopupListFiltersService, private vaultUiOnboardingService: VaultUiOnboardingService, + private destroyRef: DestroyRef, + private cipherService: CipherService, + private dialogService: DialogService, ) { combineLatest([ this.vaultPopupItemsService.emptyVault$, @@ -116,6 +122,19 @@ export class VaultV2Component implements OnInit, OnDestroy { async ngOnInit() { await this.vaultUiOnboardingService.showOnboardingDialog(); + + this.cipherService.failedToDecryptCiphers$ + .pipe( + map((ciphers) => ciphers.filter((c) => !c.isDeleted)), + filter((ciphers) => ciphers.length > 0), + take(1), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe((ciphers) => { + DecryptionFailureDialogComponent.open(this.dialogService, { + cipherIds: ciphers.map((c) => c.id as CipherId), + }); + }); } ngOnDestroy(): void {} diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index 5b0eb63998d..966793921d7 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -58,6 +58,7 @@ describe("VaultPopupItemsService", () => { cipherServiceMock.getAllDecrypted.mockResolvedValue(cipherList); cipherServiceMock.ciphers$ = new BehaviorSubject(null); cipherServiceMock.localData$ = new BehaviorSubject(null); + cipherServiceMock.failedToDecryptCiphers$ = new BehaviorSubject([]); searchService.searchCiphers.mockImplementation(async (_, __, ciphers) => ciphers); cipherServiceMock.filterCiphersForUrl.mockImplementation(async (ciphers) => ciphers.filter((c) => ["0", "1"].includes(c.id)), @@ -294,21 +295,6 @@ describe("VaultPopupItemsService", () => { }); }); - it("should sort by last used then by name by default", (done) => { - service.remainingCiphers$.subscribe(() => { - expect(cipherServiceMock.getLocaleSortingFunction).toHaveBeenCalled(); - done(); - }); - }); - - it("should NOT sort by last used then by name when search text is applied", (done) => { - service.applyFilter("Login"); - service.remainingCiphers$.subscribe(() => { - expect(cipherServiceMock.getLocaleSortingFunction).not.toHaveBeenCalled(); - done(); - }); - }); - it("should filter remainingCiphers$ down to search term", (done) => { const cipherList = Object.values(allCiphers); const searchText = "Login"; diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index 93aa8cdaba9..1c19a9d8d1d 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -90,6 +90,8 @@ export class VaultPopupItemsService { tap(() => this._ciphersLoading$.next()), waitUntilSync(this.syncService), switchMap(() => Utils.asyncToObservable(() => this.cipherService.getAllDecrypted())), + withLatestFrom(this.cipherService.failedToDecryptCiphers$), + map(([ciphers, failedToDecryptCiphers]) => [...failedToDecryptCiphers, ...ciphers]), shareReplay({ refCount: true, bufferSize: 1 }), ); @@ -190,11 +192,6 @@ export class VaultPopupItemsService { (cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher), ), ), - withLatestFrom(this._hasSearchText$), - map(([ciphers, hasSearchText]) => - // Do not sort alphabetically when there is search text, default to the search service scoring - hasSearchText ? ciphers : ciphers.sort(this.cipherService.getLocaleSortingFunction()), - ), shareReplay({ refCount: false, bufferSize: 1 }), ); diff --git a/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.html b/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.html index a79b6c74b03..dce3ba640d3 100644 --- a/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.html +++ b/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.html @@ -27,7 +27,12 @@

[bitMenuTriggerFor]="moreOptions" > - - - + + + + +
+ + + + + + diff --git a/libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts b/libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts new file mode 100644 index 00000000000..c183c4bb246 --- /dev/null +++ b/libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts @@ -0,0 +1,59 @@ +import { DIALOG_DATA, DialogRef } from "@angular/cdk/dialog"; +import { CommonModule } from "@angular/common"; +import { Component, inject } from "@angular/core"; + +import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { CipherId } from "@bitwarden/common/types/guid"; +import { + AnchorLinkDirective, + AsyncActionsModule, + ButtonModule, + DialogModule, + DialogService, + TypographyModule, +} from "@bitwarden/components"; + +export type DecryptionFailureDialogParams = { + cipherIds: CipherId[]; +}; + +@Component({ + standalone: true, + selector: "vault-decryption-failure-dialog", + templateUrl: "./decryption-failure-dialog.component.html", + imports: [ + DialogModule, + CommonModule, + TypographyModule, + JslibModule, + AsyncActionsModule, + ButtonModule, + AnchorLinkDirective, + ], +}) +export class DecryptionFailureDialogComponent { + protected dialogRef = inject(DialogRef); + protected params = inject(DIALOG_DATA); + protected platformUtilsService = inject(PlatformUtilsService); + + selectText(element: HTMLElement) { + const selection = window.getSelection(); + if (selection == null) { + return; + } + selection.removeAllRanges(); + const range = document.createRange(); + range.selectNodeContents(element); + selection.addRange(range); + } + + openContactSupport(event: Event) { + event.preventDefault(); + this.platformUtilsService.launchUri("https://bitwarden.com/contact"); + } + + static open(dialogService: DialogService, params: DecryptionFailureDialogParams) { + return dialogService.open(DecryptionFailureDialogComponent, { data: params }); + } +} diff --git a/libs/vault/src/index.ts b/libs/vault/src/index.ts index 0112de44241..c9a719934ac 100644 --- a/libs/vault/src/index.ts +++ b/libs/vault/src/index.ts @@ -16,5 +16,6 @@ export { DownloadAttachmentComponent } from "./components/download-attachment/do export { PasswordHistoryViewComponent } from "./components/password-history-view/password-history-view.component"; export { NewDeviceVerificationNoticePageOneComponent } from "./components/new-device-verification-notice/new-device-verification-notice-page-one.component"; export { NewDeviceVerificationNoticePageTwoComponent } from "./components/new-device-verification-notice/new-device-verification-notice-page-two.component"; +export { DecryptionFailureDialogComponent } from "./components/decryption-failure-dialog/decryption-failure-dialog.component"; export * as VaultIcons from "./icons";