From 2d6d1dfe5341738b54d2749024e454e51aae166f Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Sun, 21 Dec 2025 21:46:18 +0100 Subject: [PATCH] [PM-29929] Exclude organization vault items in data recovery tool (#18044) * Exclude organization vault items in data recovery tool * Allow undefined organization id --- .../data-recovery/steps/cipher-step.spec.ts | 241 ++++++++++++++++++ .../data-recovery/steps/cipher-step.ts | 6 +- 2 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 apps/web/src/app/key-management/data-recovery/steps/cipher-step.spec.ts diff --git a/apps/web/src/app/key-management/data-recovery/steps/cipher-step.spec.ts b/apps/web/src/app/key-management/data-recovery/steps/cipher-step.spec.ts new file mode 100644 index 00000000000..a894fce0c41 --- /dev/null +++ b/apps/web/src/app/key-management/data-recovery/steps/cipher-step.spec.ts @@ -0,0 +1,241 @@ +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 { DialogService } from "@bitwarden/components"; +import { UserId } from "@bitwarden/user-core"; + +import { LogRecorder } from "../log-recorder"; + +import { CipherStep } from "./cipher-step"; +import { RecoveryWorkingData } from "./recovery-step"; + +describe("CipherStep", () => { + let cipherStep: CipherStep; + let apiService: MockProxy; + let cipherEncryptionService: MockProxy; + let dialogService: MockProxy; + let logger: MockProxy; + + beforeEach(() => { + apiService = mock(); + cipherEncryptionService = mock(); + dialogService = mock(); + logger = mock(); + + cipherStep = new CipherStep(apiService, cipherEncryptionService, dialogService); + }); + + describe("runDiagnostics", () => { + it("returns false and logs error when userId is missing", async () => { + const workingData: RecoveryWorkingData = { + userId: null, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [], + folders: [], + }; + + const result = await cipherStep.runDiagnostics(workingData, logger); + + expect(result).toBe(false); + expect(logger.record).toHaveBeenCalledWith("Missing user ID"); + }); + + it("returns true when all user ciphers are decryptable", async () => { + const userId = "user-id" as UserId; + const cipher1 = { id: "cipher-1", organizationId: null } as Cipher; + const cipher2 = { id: "cipher-2", organizationId: null } as Cipher; + + const workingData: RecoveryWorkingData = { + userId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [cipher1, cipher2], + folders: [], + }; + + cipherEncryptionService.decrypt.mockResolvedValue({} as any); + + const result = await cipherStep.runDiagnostics(workingData, logger); + + expect(result).toBe(true); + expect(cipherEncryptionService.decrypt).toHaveBeenCalledWith(cipher1, userId); + expect(cipherEncryptionService.decrypt).toHaveBeenCalledWith(cipher2, userId); + }); + + it("filters out organization ciphers (organizationId !== null) and only processes user ciphers", async () => { + const userId = "user-id" as UserId; + const userCipher = { id: "user-cipher", organizationId: null } as Cipher; + const orgCipher1 = { id: "org-cipher-1", organizationId: "org-1" } as Cipher; + const orgCipher2 = { id: "org-cipher-2", organizationId: "org-2" } as Cipher; + + const workingData: RecoveryWorkingData = { + userId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [userCipher, orgCipher1, orgCipher2], + folders: [], + }; + + cipherEncryptionService.decrypt.mockResolvedValue({} as any); + + const result = await cipherStep.runDiagnostics(workingData, logger); + + expect(result).toBe(true); + // Only user cipher should be processed + expect(cipherEncryptionService.decrypt).toHaveBeenCalledTimes(1); + expect(cipherEncryptionService.decrypt).toHaveBeenCalledWith(userCipher, userId); + // Organization ciphers should not be processed + expect(cipherEncryptionService.decrypt).not.toHaveBeenCalledWith(orgCipher1, userId); + expect(cipherEncryptionService.decrypt).not.toHaveBeenCalledWith(orgCipher2, userId); + }); + + it("returns false and records undecryptable user ciphers", async () => { + const userId = "user-id" as UserId; + const cipher1 = { id: "cipher-1", organizationId: null } as Cipher; + const cipher2 = { id: "cipher-2", organizationId: null } as Cipher; + const cipher3 = { id: "cipher-3", organizationId: null } as Cipher; + + const workingData: RecoveryWorkingData = { + userId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [cipher1, cipher2, cipher3], + folders: [], + }; + + cipherEncryptionService.decrypt + .mockResolvedValueOnce({} as any) // cipher1 succeeds + .mockRejectedValueOnce(new Error("Decryption failed")) // cipher2 fails + .mockRejectedValueOnce(new Error("Decryption failed")); // cipher3 fails + + const result = await cipherStep.runDiagnostics(workingData, logger); + + expect(result).toBe(false); + expect(logger.record).toHaveBeenCalledWith("Cipher ID cipher-2 was undecryptable"); + expect(logger.record).toHaveBeenCalledWith("Cipher ID cipher-3 was undecryptable"); + expect(logger.record).toHaveBeenCalledWith("Found 2 undecryptable ciphers"); + }); + }); + + describe("canRecover", () => { + it("returns false when there are no undecryptable ciphers", async () => { + const userId = "user-id" as UserId; + const workingData: RecoveryWorkingData = { + userId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [{ id: "cipher-1", organizationId: null } as Cipher], + folders: [], + }; + + cipherEncryptionService.decrypt.mockResolvedValue({} as any); + + await cipherStep.runDiagnostics(workingData, logger); + const result = cipherStep.canRecover(workingData); + + expect(result).toBe(false); + }); + + it("returns true when there are undecryptable ciphers", async () => { + const userId = "user-id" as UserId; + const workingData: RecoveryWorkingData = { + userId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [{ id: "cipher-1", organizationId: null } as Cipher], + folders: [], + }; + + cipherEncryptionService.decrypt.mockRejectedValue(new Error("Decryption failed")); + + await cipherStep.runDiagnostics(workingData, logger); + const result = cipherStep.canRecover(workingData); + + expect(result).toBe(true); + }); + }); + + describe("runRecovery", () => { + it("logs and returns early when there are no undecryptable ciphers", async () => { + const workingData: RecoveryWorkingData = { + userId: "user-id" as UserId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [], + folders: [], + }; + + await cipherStep.runRecovery(workingData, logger); + + expect(logger.record).toHaveBeenCalledWith("No undecryptable ciphers to recover"); + expect(dialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(apiService.deleteCipher).not.toHaveBeenCalled(); + }); + + it("throws error when user cancels deletion", async () => { + const userId = "user-id" as UserId; + const workingData: RecoveryWorkingData = { + userId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [{ id: "cipher-1", organizationId: null } as Cipher], + folders: [], + }; + + cipherEncryptionService.decrypt.mockRejectedValue(new Error("Decryption failed")); + await cipherStep.runDiagnostics(workingData, logger); + + dialogService.openSimpleDialog.mockResolvedValue(false); + + await expect(cipherStep.runRecovery(workingData, logger)).rejects.toThrow( + "Cipher recovery cancelled by user", + ); + + expect(logger.record).toHaveBeenCalledWith("Showing confirmation dialog for 1 ciphers"); + expect(logger.record).toHaveBeenCalledWith("User cancelled cipher deletion"); + expect(apiService.deleteCipher).not.toHaveBeenCalled(); + }); + + it("deletes undecryptable ciphers when user confirms", async () => { + const userId = "user-id" as UserId; + const cipher1 = { id: "cipher-1", organizationId: null } as Cipher; + const cipher2 = { id: "cipher-2", organizationId: null } as Cipher; + + const workingData: RecoveryWorkingData = { + userId, + userKey: null, + encryptedPrivateKey: null, + isPrivateKeyCorrupt: false, + ciphers: [cipher1, cipher2], + folders: [], + }; + + cipherEncryptionService.decrypt.mockRejectedValue(new Error("Decryption failed")); + await cipherStep.runDiagnostics(workingData, logger); + + dialogService.openSimpleDialog.mockResolvedValue(true); + apiService.deleteCipher.mockResolvedValue(undefined); + + await cipherStep.runRecovery(workingData, logger); + + expect(logger.record).toHaveBeenCalledWith("Showing confirmation dialog for 2 ciphers"); + expect(logger.record).toHaveBeenCalledWith("Deleting 2 ciphers"); + expect(apiService.deleteCipher).toHaveBeenCalledWith("cipher-1"); + expect(apiService.deleteCipher).toHaveBeenCalledWith("cipher-2"); + expect(logger.record).toHaveBeenCalledWith("Deleted cipher cipher-1"); + expect(logger.record).toHaveBeenCalledWith("Deleted cipher cipher-2"); + expect(logger.record).toHaveBeenCalledWith("Successfully deleted 2 ciphers"); + }); + }); +}); diff --git a/apps/web/src/app/key-management/data-recovery/steps/cipher-step.ts b/apps/web/src/app/key-management/data-recovery/steps/cipher-step.ts index 34e8cbdc9f3..b44e8afc54d 100644 --- a/apps/web/src/app/key-management/data-recovery/steps/cipher-step.ts +++ b/apps/web/src/app/key-management/data-recovery/steps/cipher-step.ts @@ -24,7 +24,11 @@ export class CipherStep implements RecoveryStep { } this.undecryptableCipherIds = []; - for (const cipher of workingData.ciphers) { + // The tool is currently only implemented to handle ciphers that are corrupt for a user. For an organization, the case of + // local user not having access to the organization key is not properly handled here, and should be implemented separately. + // For now, this just filters out and does not consider corrupt organization ciphers. + const userCiphers = workingData.ciphers.filter((c) => c.organizationId == null); + for (const cipher of userCiphers) { try { await this.cipherService.decrypt(cipher, workingData.userId); } catch {