1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-06 19:53:59 +00:00

[PM-30285] Add soundness check to cipher and folder recovery step (#18120)

* Add soundness check to cipher and folder recovery step

* fix tests

---------

Co-authored-by: Maciej Zieniuk <mzieniuk@bitwarden.com>
(cherry picked from commit f689fd88b7)
This commit is contained in:
Bernd Schoolmann
2025-12-29 18:31:15 +01:00
committed by Maciej Zieniuk
parent dccc7b183c
commit e0e2cf56f5
3 changed files with 42 additions and 6 deletions

View File

@@ -132,7 +132,10 @@ describe("CipherStep", () => {
userKey: null,
encryptedPrivateKey: null,
isPrivateKeyCorrupt: false,
ciphers: [{ id: "cipher-1", organizationId: null } as Cipher],
ciphers: [
{ id: "cipher-1", organizationId: null } as Cipher,
{ id: "cipher-2", organizationId: null } as Cipher,
],
folders: [],
};
@@ -144,14 +147,39 @@ describe("CipherStep", () => {
expect(result).toBe(false);
});
it("returns true when there are undecryptable ciphers", async () => {
it("returns true when there are undecryptable ciphers but at least one decryptable cipher", 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],
ciphers: [
{ id: "cipher-1", organizationId: null } as Cipher,
{ id: "cipher-2", organizationId: null } as Cipher,
],
folders: [],
};
cipherEncryptionService.decrypt.mockRejectedValueOnce(new Error("Decryption failed"));
await cipherStep.runDiagnostics(workingData, logger);
const result = cipherStep.canRecover(workingData);
expect(result).toBe(true);
});
it("returns false when all ciphers are undecryptable", 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,
{ id: "cipher-2", organizationId: null } as Cipher,
],
folders: [],
};
@@ -160,7 +188,7 @@ describe("CipherStep", () => {
await cipherStep.runDiagnostics(workingData, logger);
const result = cipherStep.canRecover(workingData);
expect(result).toBe(true);
expect(result).toBe(false);
});
});

View File

@@ -10,6 +10,7 @@ export class CipherStep implements RecoveryStep {
title = "recoveryStepCipherTitle";
private undecryptableCipherIds: string[] = [];
private decryptableCipherIds: string[] = [];
constructor(
private apiService: ApiService,
@@ -31,18 +32,21 @@ export class CipherStep implements RecoveryStep {
for (const cipher of userCiphers) {
try {
await this.cipherService.decrypt(cipher, workingData.userId);
this.decryptableCipherIds.push(cipher.id);
} catch {
logger.record(`Cipher ID ${cipher.id} was undecryptable`);
this.undecryptableCipherIds.push(cipher.id);
}
}
logger.record(`Found ${this.undecryptableCipherIds.length} undecryptable ciphers`);
logger.record(`Found ${this.decryptableCipherIds.length} decryptable ciphers`);
return this.undecryptableCipherIds.length == 0;
}
canRecover(workingData: RecoveryWorkingData): boolean {
return this.undecryptableCipherIds.length > 0;
// If everything fails to decrypt, it's a deeper issue and we shouldn't offer recovery here.
return this.undecryptableCipherIds.length > 0 && this.decryptableCipherIds.length > 0;
}
async runRecovery(workingData: RecoveryWorkingData, logger: LogRecorder): Promise<void> {

View File

@@ -11,6 +11,7 @@ export class FolderStep implements RecoveryStep {
title = "recoveryStepFoldersTitle";
private undecryptableFolderIds: string[] = [];
private decryptableFolderIds: string[] = [];
constructor(
private folderService: FolderApiServiceAbstraction,
@@ -36,18 +37,21 @@ export class FolderStep implements RecoveryStep {
folder.name.encryptedString,
workingData.userKey.toEncoded(),
);
this.decryptableFolderIds.push(folder.id);
} catch {
logger.record(`Folder name for folder ID ${folder.id} was undecryptable`);
this.undecryptableFolderIds.push(folder.id);
}
}
logger.record(`Found ${this.undecryptableFolderIds.length} undecryptable folders`);
logger.record(`Found ${this.decryptableFolderIds.length} decryptable folders`);
return this.undecryptableFolderIds.length == 0;
}
canRecover(workingData: RecoveryWorkingData): boolean {
return this.undecryptableFolderIds.length > 0;
// If everything fails to decrypt, it's a deeper issue and we shouldn't offer recovery here.
return this.undecryptableFolderIds.length > 0 && this.decryptableFolderIds.length > 0;
}
async runRecovery(workingData: RecoveryWorkingData, logger: LogRecorder): Promise<void> {