Km/data recovery improvement

apps/web/src/app/key-management/data-recovery/data-recovery.component.spec.ts

@@ -98,12 +98,14 @@ describe("DataRecoveryComponent", () => {
 
     it("should initialize steps in correct order", () => {
       const steps = component.steps();
-      expect(steps.length).toBe(5);
+      expect(steps.length).toBe(7);
       expect(steps[0].title).toBe("recoveryStepUserInfoTitle_used-i18n");
       expect(steps[1].title).toBe("recoveryStepSyncTitle_used-i18n");
       expect(steps[2].title).toBe("recoveryStepPrivateKeyTitle_used-i18n");
       expect(steps[3].title).toBe("recoveryStepFoldersTitle_used-i18n");
-      expect(steps[4].title).toBe("recoveryStepCipherTitle_used-i18n");
+      expect(steps[4].title).toBe("recoveryStepFido2Title_used-i18n");
+      expect(steps[5].title).toBe("recoveryStepCipherTitle_used-i18n");
+      expect(steps[6].title).toBe("recoveryStepAttachmentsTitle_used-i18n");
     });
   });
 
@@ -112,7 +114,7 @@ describe("DataRecoveryComponent", () => {
 
     beforeEach(() => {
       // Create mock steps
-      mockSteps = Array(5)
+      mockSteps = Array(7)
         .fill(null)
         .map(() => {
           const mockStep = mock<RecoveryStep>();
@@ -233,9 +235,10 @@ describe("DataRecoveryComponent", () => {
         encryptedPrivateKey: null,
         ciphers: [],
         folders: [],
+        fido2CorruptCipherIds: [],
       };
 
-      mockSteps = Array(5)
+      mockSteps = Array(7)
         .fill(null)
         .map(() => {
           const mockStep = mock<RecoveryStep>();

apps/web/src/app/key-management/data-recovery/data-recovery.component.ts

@@ -27,6 +27,8 @@ import {
   RecoveryWorkingData,
   FolderStep,
   CipherStep,
+  Fido2Step,
+  AttachmentStep,
 } from "./steps";
 
 export const StepStatus = Object.freeze({
@@ -75,7 +77,9 @@ export class DataRecoveryComponent {
       this.cryptoFunctionService,
     ),
     new FolderStep(this.folderApiService, this.dialogService),
+    new Fido2Step(this.cipherEncryptService, this.apiService, this.dialogService),
     new CipherStep(this.apiService, this.cipherEncryptService, this.dialogService),
+    new AttachmentStep(this.cipherEncryptService, this.apiService, this.dialogService),
   ];
   private workingData: RecoveryWorkingData | null = null;
 
@@ -108,6 +112,7 @@ export class DataRecoveryComponent {
       encryptedPrivateKey: null,
       ciphers: [],
       folders: [],
+      fido2CorruptCipherIds: [],
     };
 
     await this.runDiagnosticsInternal();

apps/web/src/app/key-management/data-recovery/steps/attachment-step.spec.ts

@@ -0,0 +1,176 @@
+import { mock, MockProxy } from "jest-mock-extended";
+
+import { ApiService } from "@bitwarden/common/abstractions/api.service";
+import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service";
+import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
+import { AttachmentView } from "@bitwarden/common/vault/models/view/attachment.view";
+import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
+import { DialogService } from "@bitwarden/components";
+import { UserId } from "@bitwarden/user-core";
+
+import { LogRecorder } from "../log-recorder";
+
+import { AttachmentStep } from "./attachment-step";
+import { RecoveryWorkingData } from "./recovery-step";
+
+function attachment(id: string, hasDecryptionError: boolean): AttachmentView {
+  const view = new AttachmentView();
+  view.id = id;
+  view.hasDecryptionError = hasDecryptionError;
+  return view;
+}
+
+function viewWithAttachments(attachments: AttachmentView[]): CipherView {
+  return { decryptionFailure: false, attachments } as CipherView;
+}
+
+function buildWorkingData(ciphers: Cipher[]): RecoveryWorkingData {
+  return {
+    userId: "user-id" as UserId,
+    userKey: null,
+    encryptedPrivateKey: null,
+    isPrivateKeyCorrupt: false,
+    ciphers,
+    folders: [],
+    fido2CorruptCipherIds: [],
+  };
+}
+
+describe("AttachmentStep", () => {
+  let attachmentStep: AttachmentStep;
+  let cipherEncryptionService: MockProxy<CipherEncryptionService>;
+  let apiService: MockProxy<ApiService>;
+  let dialogService: MockProxy<DialogService>;
+  let logger: MockProxy<LogRecorder>;
+
+  beforeEach(() => {
+    cipherEncryptionService = mock<CipherEncryptionService>();
+    apiService = mock<ApiService>();
+    dialogService = mock<DialogService>();
+    logger = mock<LogRecorder>();
+
+    attachmentStep = new AttachmentStep(cipherEncryptionService, apiService, dialogService);
+  });
+
+  describe("runDiagnostics", () => {
+    it("returns false and logs when userId is missing", async () => {
+      const workingData = buildWorkingData([]);
+      workingData.userId = null;
+
+      const result = await attachmentStep.runDiagnostics(workingData, logger);
+
+      expect(result).toBe(false);
+      expect(logger.record).toHaveBeenCalledWith("Missing user ID");
+    });
+
+    it("returns true when no attachments have decryption errors", async () => {
+      const cipher = { id: "cipher-1", organizationId: null } as Cipher;
+      const workingData = buildWorkingData([cipher]);
+
+      cipherEncryptionService.decrypt.mockResolvedValue(
+        viewWithAttachments([attachment("att-1", false)]),
+      );
+
+      const result = await attachmentStep.runDiagnostics(workingData, logger);
+
+      expect(result).toBe(true);
+    });
+
+    it("records attachments with decryption errors", async () => {
+      const cipher = { id: "cipher-1", organizationId: null } as Cipher;
+      const workingData = buildWorkingData([cipher]);
+
+      cipherEncryptionService.decrypt.mockResolvedValue(
+        viewWithAttachments([attachment("att-good", false), attachment("att-bad", true)]),
+      );
+
+      const result = await attachmentStep.runDiagnostics(workingData, logger);
+
+      expect(result).toBe(false);
+      expect(attachmentStep["corruptAttachments"]).toEqual([
+        { cipherId: "cipher-1", attachmentId: "att-bad" },
+      ]);
+      expect(logger.record).toHaveBeenCalledWith("Found 1 corrupt attachments");
+    });
+
+    it("skips ciphers that fail to decrypt entirely", async () => {
+      const cipher = { id: "cipher-1", organizationId: null } as Cipher;
+      const workingData = buildWorkingData([cipher]);
+
+      cipherEncryptionService.decrypt.mockRejectedValue(new Error("Decryption failed"));
+
+      const result = await attachmentStep.runDiagnostics(workingData, logger);
+
+      expect(result).toBe(true);
+      expect(attachmentStep["corruptAttachments"]).toEqual([]);
+    });
+
+    it("skips organization ciphers", async () => {
+      const orgCipher = { id: "org-cipher", organizationId: "org-1" } as Cipher;
+      const workingData = buildWorkingData([orgCipher]);
+
+      const result = await attachmentStep.runDiagnostics(workingData, logger);
+
+      expect(result).toBe(true);
+      expect(cipherEncryptionService.decrypt).not.toHaveBeenCalled();
+    });
+  });
+
+  describe("canRecover", () => {
+    it("returns true only when corrupt attachments were found", async () => {
+      const cipher = { id: "cipher-1", organizationId: null } as Cipher;
+      const workingData = buildWorkingData([cipher]);
+
+      cipherEncryptionService.decrypt.mockResolvedValue(
+        viewWithAttachments([attachment("att-bad", true)]),
+      );
+      await attachmentStep.runDiagnostics(workingData, logger);
+
+      expect(attachmentStep.canRecover(workingData)).toBe(true);
+    });
+  });
+
+  describe("runRecovery", () => {
+    it("returns early when there is nothing to recover", async () => {
+      const workingData = buildWorkingData([]);
+
+      await attachmentStep.runRecovery(workingData, logger);
+
+      expect(dialogService.openSimpleDialog).not.toHaveBeenCalled();
+      expect(apiService.deleteCipherAttachment).not.toHaveBeenCalled();
+    });
+
+    it("throws when the user cancels", async () => {
+      const cipher = { id: "cipher-1", organizationId: null } as Cipher;
+      const workingData = buildWorkingData([cipher]);
+      cipherEncryptionService.decrypt.mockResolvedValue(
+        viewWithAttachments([attachment("att-bad", true)]),
+      );
+      await attachmentStep.runDiagnostics(workingData, logger);
+
+      dialogService.openSimpleDialog.mockResolvedValue(false);
+
+      await expect(attachmentStep.runRecovery(workingData, logger)).rejects.toThrow(
+        "Attachment recovery cancelled by user",
+      );
+      expect(apiService.deleteCipherAttachment).not.toHaveBeenCalled();
+    });
+
+    it("deletes corrupt attachments when confirmed", async () => {
+      const cipher = { id: "cipher-1", organizationId: null } as Cipher;
+      const workingData = buildWorkingData([cipher]);
+      cipherEncryptionService.decrypt.mockResolvedValue(
+        viewWithAttachments([attachment("att-bad", true)]),
+      );
+      await attachmentStep.runDiagnostics(workingData, logger);
+
+      dialogService.openSimpleDialog.mockResolvedValue(true);
+      apiService.deleteCipherAttachment.mockResolvedValue(undefined as any);
+
+      await attachmentStep.runRecovery(workingData, logger);
+
+      expect(apiService.deleteCipherAttachment).toHaveBeenCalledWith("cipher-1", "att-bad");
+      expect(logger.record).toHaveBeenCalledWith("Deleted attachment att-bad from cipher cipher-1");
+    });
+  });
+});

apps/web/src/app/key-management/data-recovery/steps/attachment-step.ts

@@ -0,0 +1,102 @@
+import { ApiService } from "@bitwarden/common/abstractions/api.service";
+import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service";
+import { DialogService } from "@bitwarden/components";
+
+import { LogRecorder } from "../log-recorder";
+
+import { logCipherCorruption } from "./corruption-log";
+import { RecoveryStep, RecoveryWorkingData } from "./recovery-step";
+
+interface CorruptAttachment {
+  cipherId: string;
+  attachmentId: string;
+}
+
+/**
+ * Detects attachments whose key or filename fail to decrypt (surfaced as
+ * `AttachmentView.hasDecryptionError`) and repairs them by deleting the corrupt attachments.
+ */
+export class AttachmentStep implements RecoveryStep {
+  title = "recoveryStepAttachmentsTitle";
+
+  private corruptAttachments: CorruptAttachment[] = [];
+
+  constructor(
+    private cipherService: CipherEncryptionService,
+    private apiService: ApiService,
+    private dialogService: DialogService,
+  ) {}
+
+  async runDiagnostics(workingData: RecoveryWorkingData, logger: LogRecorder): Promise<boolean> {
+    if (!workingData.userId) {
+      logger.record("Missing user ID");
+      return false;
+    }
+
+    this.corruptAttachments = [];
+
+    const userCiphers = workingData.ciphers.filter((c) => c.organizationId == null);
+    for (const cipher of userCiphers) {
+      let view;
+      try {
+        view = await this.cipherService.decrypt(cipher, workingData.userId);
+      } catch {
+        // Whole-cipher (or FIDO2) decryption failures are handled by other steps.
+        continue;
+      }
+
+      const corrupt = view.attachments?.filter((a) => a.hasDecryptionError && a.id != null) ?? [];
+      for (const attachment of corrupt) {
+        this.corruptAttachments.push({ cipherId: cipher.id, attachmentId: attachment.id! });
+        logCipherCorruption(cipher, logger, `attachment ${attachment.id}`);
+      }
+    }
+
+    logger.record(`Found ${this.corruptAttachments.length} corrupt attachments`);
+
+    return this.corruptAttachments.length == 0;
+  }
+
+  canRecover(workingData: RecoveryWorkingData): boolean {
+    return this.corruptAttachments.length > 0;
+  }
+
+  async runRecovery(workingData: RecoveryWorkingData, logger: LogRecorder): Promise<void> {
+    if (this.corruptAttachments.length === 0) {
+      logger.record("No corrupt attachments to recover");
+      return;
+    }
+
+    logger.record(`Showing confirmation dialog for ${this.corruptAttachments.length} attachments`);
+
+    const confirmed = await this.dialogService.openSimpleDialog({
+      title: { key: "recoveryDeleteAttachmentsTitle" },
+      content: { key: "recoveryDeleteAttachmentsDesc" },
+      acceptButtonText: { key: "ok" },
+      cancelButtonText: { key: "cancel" },
+      type: "danger",
+    });
+
+    if (!confirmed) {
+      logger.record("User cancelled attachment deletion");
+      throw new Error("Attachment recovery cancelled by user");
+    }
+
+    logger.record(`Deleting ${this.corruptAttachments.length} attachments`);
+
+    for (const { cipherId, attachmentId } of this.corruptAttachments) {
+      try {
+        await this.apiService.deleteCipherAttachment(cipherId, attachmentId);
+        logger.record(`Deleted attachment ${attachmentId} from cipher ${cipherId}`);
+      } catch (error) {
+        const errorMessage = error instanceof Error ? error.message : String(error);
+        logger.record(
+          `Failed to delete attachment ${attachmentId} from cipher ${cipherId}: ${errorMessage}`,
+        );
+        throw error;
+      }
+    }
+
+    logger.record(`Successfully deleted ${this.corruptAttachments.length} attachments`);
+  }
+}