[PM-24978] Corrupt Attachment Keys (#17790)

* display translated content for attachments that cannot be downloaded

* consume decryption failure from the sdk for attachments

* add decryption errors from sdk

* only show fix attachment issues for when key is null and it does not have a decryption failure

* separate decryption failure state in view

apps/browser/src/_locales/en/messages.json

@@ -5966,6 +5966,9 @@
   "cardNumberLabel": {
     "message": "Card number"
   },
+  "errorCannotDecrypt": {
+    "message": "Error: Cannot decrypt"
+  },
   "removeMasterPasswordForOrgUserKeyConnector": {
     "message": "Your organization is no longer using master passwords to log into Bitwarden. To continue, verify the organization and domain."
   },
@@ -6138,4 +6141,4 @@
     "message": "Individuals will need to enter the password to view this Send",
     "description": "'Send' is a noun and the name of a feature called 'Bitwarden Send'. It should not be translated."
   }
-}
+}

apps/desktop/src/locales/en/messages.json

@@ -4486,6 +4486,9 @@
   "sessionTimeoutSettingsAction": {
     "message": "Timeout action"
   },
+  "errorCannotDecrypt": {
+    "message": "Error: Cannot decrypt"
+  },
   "sessionTimeoutHeader": {
     "message": "Session timeout"
   },

apps/web/src/locales/en/messages.json

@@ -12582,6 +12582,9 @@
   "confirmNoSelectedCriticalApplicationsDesc": {
     "message": "Are you sure you want to continue?"
   },
+  "errorCannotDecrypt": {
+    "message": "Error: Cannot decrypt"
+  },
   "userVerificationFailed": {
     "message": "User verification failed."
   },

libs/common/src/vault/models/domain/attachment.ts

@@ -47,6 +47,12 @@ export class Attachment extends Domain {
     if (this.key != null) {
       view.key = await this.decryptAttachmentKey(decryptionKey);
       view.encryptedKey = this.key; // Keep the encrypted key for the view
+
+      // When the attachment key couldn't be decrypted, mark a decryption error
+      // The file won't be able to be downloaded in these cases
+      if (!view.key) {
+        view.hasDecryptionError = true;
+      }
     }
 
     return view;

libs/common/src/vault/models/view/attachment.view.ts

@@ -2,7 +2,7 @@ import { Jsonify } from "type-fest";
 
 import { AttachmentView as SdkAttachmentView } from "@bitwarden/sdk-internal";
 
-import { EncString } from "../../../key-management/crypto/models/enc-string";
+import { DECRYPT_ERROR, EncString } from "../../../key-management/crypto/models/enc-string";
 import { View } from "../../../models/view/view";
 import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
 import { Attachment } from "../domain/attachment";
@@ -18,6 +18,7 @@ export class AttachmentView implements View {
    * The SDK returns an encrypted key for the attachment.
    */
   encryptedKey: EncString | undefined;
+  private _hasDecryptionError?: boolean;
 
   constructor(a?: Attachment) {
     if (!a) {
@@ -41,6 +42,14 @@ export class AttachmentView implements View {
     return 0;
   }
 
+  get hasDecryptionError(): boolean {
+    return this._hasDecryptionError || this.fileName === DECRYPT_ERROR;
+  }
+
+  set hasDecryptionError(value: boolean) {
+    this._hasDecryptionError = value;
+  }
+
   static fromJSON(obj: Partial<Jsonify<AttachmentView>>): AttachmentView {
     const key = obj.key == null ? null : SymmetricCryptoKey.fromJSON(obj.key);
 
@@ -76,7 +85,10 @@ export class AttachmentView implements View {
   /**
    * Converts the SDK AttachmentView to a AttachmentView.
    */
-  static fromSdkAttachmentView(obj: SdkAttachmentView): AttachmentView | undefined {
+  static fromSdkAttachmentView(
+    obj: SdkAttachmentView,
+    failure = false,
+  ): AttachmentView | undefined {
     if (!obj) {
       return undefined;
     }
@@ -90,6 +102,7 @@ export class AttachmentView implements View {
     // TODO: PM-23005 - Temporary field, should be removed when encrypted migration is complete
     view.key = obj.decryptedKey ? SymmetricCryptoKey.fromString(obj.decryptedKey) : undefined;
     view.encryptedKey = obj.key ? new EncString(obj.key) : undefined;
+    view._hasDecryptionError = failure;
 
     return view;
   }

libs/common/src/vault/models/view/cipher.view.ts

@@ -280,6 +280,17 @@ export class CipherView implements View, InitializerMetadata {
       return undefined;
     }
 
+    const attachments = obj.attachments?.map((a) => AttachmentView.fromSdkAttachmentView(a)!) ?? [];
+
+    if (obj.attachmentDecryptionFailures?.length) {
+      obj.attachmentDecryptionFailures.forEach((attachment) => {
+        const attachmentView = AttachmentView.fromSdkAttachmentView(attachment, true);
+        if (attachmentView) {
+          attachments.push(attachmentView);
+        }
+      });
+    }
+
     const cipherView = new CipherView();
     cipherView.id = uuidAsString(obj.id);
     cipherView.organizationId = uuidAsString(obj.organizationId);
@@ -295,8 +306,7 @@ export class CipherView implements View, InitializerMetadata {
     cipherView.edit = obj.edit;
     cipherView.viewPassword = obj.viewPassword;
     cipherView.localData = fromSdkLocalData(obj.localData);
-    cipherView.attachments =
-      obj.attachments?.map((a) => AttachmentView.fromSdkAttachmentView(a)!) ?? [];
+    cipherView.attachments = attachments;
     cipherView.fields = obj.fields?.map((f) => FieldView.fromSdkFieldView(f)!) ?? [];
     cipherView.passwordHistory =
       obj.passwordHistory?.map((ph) => PasswordHistoryView.fromSdkPasswordHistoryView(ph)!) ?? [];

libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.html

@@ -4,52 +4,76 @@
   <ul aria-labelledby="attachments" class="tw-list-none tw-pl-0">
     @for (attachment of attachments; track attachment.id) {
       <li>
-        <bit-item>
-          <bit-item-content>
-            <span data-testid="file-name" [title]="attachment.fileName">{{
-              attachment.fileName
-            }}</span>
-            <span slot="secondary" data-testid="file-size">{{ attachment.sizeName }}</span>
-            <i
-              *ngIf="attachment.key == null"
-              slot="default-trailing"
-              class="bwi bwi-exclamation-triangle bwi-sm tw-text-muted"
-              [appA11yTitle]="'fixEncryptionTooltip' | i18n"
-            ></i>
-          </bit-item-content>
-
-          <ng-container slot="end">
-            <bit-item-action>
-              @if (attachment.key != null) {
-                <app-download-attachment
-                  [admin]="admin() && organization()?.canEditAllCiphers"
-                  [cipher]="cipher()"
-                  [attachment]="attachment"
-                ></app-download-attachment>
-              } @else {
-                <button
-                  [bitAction]="fixOldAttachment(attachment)"
-                  bitButton
-                  buttonType="primary"
-                  size="small"
-                  type="button"
-                >
-                  {{ "fixEncryption" | i18n }}
-                </button>
+        @if (!attachment.hasDecryptionError) {
+          <bit-item>
+            <bit-item-content>
+              <span data-testid="file-name" [title]="attachment.fileName">
+                {{ attachment.fileName }}
+              </span>
+              <span slot="secondary" data-testid="file-size">{{ attachment.sizeName }}</span>
+              @if (attachment.key == null) {
+                <i
+                  slot="default-trailing"
+                  class="bwi bwi-exclamation-triangle bwi-sm tw-text-muted"
+                  [appA11yTitle]="'fixEncryptionTooltip' | i18n"
+                ></i>
               }
-            </bit-item-action>
-            @if (cipher().edit) {
+            </bit-item-content>
+
+            <ng-container slot="end">
               <bit-item-action>
-                <app-delete-attachment
-                  [admin]="admin() && organization()?.canEditAllCiphers"
-                  [cipherId]="cipher().id"
-                  [attachment]="attachment"
-                  (onDeletionSuccess)="removeAttachment(attachment)"
-                ></app-delete-attachment>
+                @if (attachment.key != null) {
+                  <app-download-attachment
+                    [admin]="admin() && organization()?.canEditAllCiphers"
+                    [cipher]="cipher()"
+                    [attachment]="attachment"
+                  ></app-download-attachment>
+                } @else {
+                  <button
+                    [bitAction]="fixOldAttachment(attachment)"
+                    bitButton
+                    buttonType="primary"
+                    size="small"
+                    type="button"
+                  >
+                    {{ "fixEncryption" | i18n }}
+                  </button>
+                }
               </bit-item-action>
-            }
-          </ng-container>
-        </bit-item>
+              @if (cipher().edit) {
+                <bit-item-action>
+                  <app-delete-attachment
+                    [admin]="admin() && organization()?.canEditAllCiphers"
+                    [cipherId]="cipher().id"
+                    [attachment]="attachment"
+                    (onDeletionSuccess)="removeAttachment(attachment)"
+                  ></app-delete-attachment>
+                </bit-item-action>
+              }
+            </ng-container>
+          </bit-item>
+        } @else {
+          <bit-item>
+            <bit-item-content>
+              <span data-testid="file-name" [title]="'errorCannotDecrypt' | i18n">
+                {{ "errorCannotDecrypt" | i18n }}
+              </span>
+            </bit-item-content>
+
+            <ng-container slot="end">
+              @if (cipher().edit) {
+                <bit-item-action>
+                  <app-delete-attachment
+                    [admin]="admin() && organization()?.canEditAllCiphers"
+                    [cipherId]="cipher().id"
+                    [attachment]="attachment"
+                    (onDeletionSuccess)="removeAttachment(attachment)"
+                  ></app-delete-attachment>
+                </bit-item-action>
+              }
+            </ng-container>
+          </bit-item>
+        }
       </li>
     }
   </ul>

libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts

@@ -173,7 +173,7 @@ describe("CipherAttachmentsComponent", () => {
     const fileSize = fixture.debugElement.query(By.css('[data-testid="file-size"]'));
 
     expect(fileName.nativeElement.textContent.trim()).toEqual(attachment.fileName);
-    expect(fileSize.nativeElement.textContent).toEqual(attachment.sizeName);
+    expect(fileSize.nativeElement.textContent.trim()).toEqual(attachment.sizeName);
   });
 
   describe("bitSubmit", () => {

libs/vault/src/cipher-view/attachments/attachments-v2-view.component.html

@@ -5,8 +5,12 @@
   <bit-item-group>
     <bit-item *ngFor="let attachment of cipher.attachments">
       <bit-item-content>
-        <span data-testid="file-name" [title]="attachment.fileName">{{ attachment.fileName }}</span>
-        <span slot="secondary" data-testid="file-size">{{ attachment.sizeName }}</span>
+        <span data-testid="file-name" [title]="getAttachmentFileName(attachment)">
+          {{ getAttachmentFileName(attachment) }}
+        </span>
+        @if (!attachment.hasDecryptionError) {
+          <span slot="secondary" data-testid="file-size">{{ attachment.sizeName }}</span>
+        }
       </bit-item-content>
       <ng-container slot="end">
         <bit-item-action class="tw-pr-4 [@media(min-width:650px)]:tw-pr-6">

libs/vault/src/cipher-view/attachments/attachments-v2-view.component.ts

@@ -8,9 +8,11 @@ import { NEVER, switchMap } from "rxjs";
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { StateProvider } from "@bitwarden/common/platform/state";
 import { EmergencyAccessId, OrganizationId } from "@bitwarden/common/types/guid";
 import { OrgKey } from "@bitwarden/common/types/key";
+import { AttachmentView } from "@bitwarden/common/vault/models/view/attachment.view";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import {
   ItemModule,
@@ -59,6 +61,7 @@ export class AttachmentsV2ViewComponent {
     private billingAccountProfileStateService: BillingAccountProfileStateService,
     private stateProvider: StateProvider,
     private accountService: AccountService,
+    private i18nService: I18nService,
   ) {
     this.subscribeToHasPremiumCheck();
     this.subscribeToOrgKey();
@@ -89,4 +92,12 @@ export class AttachmentsV2ViewComponent {
         }
       });
   }
+
+  getAttachmentFileName(attachment: AttachmentView): string {
+    if (attachment.hasDecryptionError) {
+      return this.i18nService.t("errorCannotDecrypt");
+    }
+
+    return attachment.fileName ?? "";
+  }
 }

libs/vault/src/components/download-attachment/download-attachment.component.spec.ts

@@ -36,12 +36,11 @@ describe("DownloadAttachmentComponent", () => {
     .mockResolvedValue({ url: "https://www.downloadattachement.com" });
   const download = jest.fn();
 
-  const attachment = {
-    id: "222-3333-4444",
-    url: "https://www.attachment.com",
-    fileName: "attachment-filename",
-    size: "1234",
-  } as AttachmentView;
+  const attachment = new AttachmentView();
+  attachment.id = "222-3333-4444";
+  attachment.url = "https://www.attachment.com";
+  attachment.fileName = "attachment-filename";
+  attachment.size = "1234";
 
   const cipherView = {
     id: "5555-444-3333",
@@ -123,7 +122,12 @@ describe("DownloadAttachmentComponent", () => {
     });
 
     it("hides download button when the attachment has decryption failure", () => {
-      const decryptFailureAttachment = { ...attachment, fileName: DECRYPT_ERROR };
+      const decryptFailureAttachment = new AttachmentView();
+      decryptFailureAttachment.id = attachment.id;
+      decryptFailureAttachment.url = attachment.url;
+      decryptFailureAttachment.size = attachment.size;
+      decryptFailureAttachment.fileName = DECRYPT_ERROR;
+
       fixture.componentRef.setInput("attachment", decryptFailureAttachment);
       fixture.detectChanges();
 

libs/vault/src/components/download-attachment/download-attachment.component.ts

@@ -4,7 +4,6 @@ import { firstValueFrom } from "rxjs";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
-import { DECRYPT_ERROR } from "@bitwarden/common/key-management/crypto/models/enc-string";
 import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
 import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@@ -46,9 +45,7 @@ export class DownloadAttachmentComponent {
     private cipherService: CipherService,
   ) {}
 
-  protected readonly isDecryptionFailure = computed(
-    () => this.attachment().fileName === DECRYPT_ERROR,
-  );
+  protected readonly isDecryptionFailure = computed(() => this.attachment().hasDecryptionError);
 
   /** Download the attachment */
   download = async () => {