mirror of
https://github.com/bitwarden/browser
synced 2025-12-16 16:23:44 +00:00
[PM-23085] Use SDK to get rotated cipher data (#15670)
* [PM-23085] Add encryptWithKey method to CipherEncryptionService * [PM-23085] Use new encryptWithKey() SDK method in getRotatedData() based on feature flag * [PM-23085] Rename cipher encryption method to encryptCipherForRotation to better reflect intended use case * [PM-23085] Update @bitwarden/sdk-internal package version * [PM-23085] Fix failing test after method rename * [PM-23085] Fix other failing test * [PM-23085] Typo
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
import { UserKey } from "@bitwarden/common/types/key";
|
||||||
import { EncryptionContext } from "@bitwarden/common/vault/abstractions/cipher.service";
|
import { EncryptionContext } from "@bitwarden/common/vault/abstractions/cipher.service";
|
||||||
import { CipherListView } from "@bitwarden/sdk-internal";
|
import { CipherListView } from "@bitwarden/sdk-internal";
|
||||||
|
|
||||||
@@ -32,6 +33,18 @@ export abstract class CipherEncryptionService {
|
|||||||
userId: UserId,
|
userId: UserId,
|
||||||
): Promise<EncryptionContext | undefined>;
|
): Promise<EncryptionContext | undefined>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Encrypts a cipher for a given userId with a new key for key rotation.
|
||||||
|
* @param model The cipher view to encrypt
|
||||||
|
* @param userId The user ID to initialize the SDK client with
|
||||||
|
* @param newKey The new key to use for re-encryption
|
||||||
|
*/
|
||||||
|
abstract encryptCipherForRotation(
|
||||||
|
model: CipherView,
|
||||||
|
userId: UserId,
|
||||||
|
newKey: UserKey,
|
||||||
|
): Promise<EncryptionContext | undefined>;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Decrypts a cipher using the SDK for the given userId.
|
* Decrypts a cipher using the SDK for the given userId.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -397,7 +397,7 @@ describe("Cipher Service", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("encryptWithCipherKey", () => {
|
describe("encryptCipherForRotation", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
jest.spyOn<any, string>(cipherService, "encryptCipherWithCipherKey");
|
jest.spyOn<any, string>(cipherService, "encryptCipherWithCipherKey");
|
||||||
keyService.getOrgKey.mockReturnValue(
|
keyService.getOrgKey.mockReturnValue(
|
||||||
@@ -534,6 +534,26 @@ describe("Cipher Service", () => {
|
|||||||
cipherService.getRotatedData(originalUserKey, newUserKey, mockUserId),
|
cipherService.getRotatedData(originalUserKey, newUserKey, mockUserId),
|
||||||
).rejects.toThrow("Cannot rotate ciphers when decryption failures are present");
|
).rejects.toThrow("Cannot rotate ciphers when decryption failures are present");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("uses the sdk to re-encrypt ciphers when feature flag is enabled", async () => {
|
||||||
|
configService.getFeatureFlag
|
||||||
|
.calledWith(FeatureFlag.PM22136_SdkCipherEncryption)
|
||||||
|
.mockResolvedValue(true);
|
||||||
|
|
||||||
|
cipherEncryptionService.encryptCipherForRotation.mockResolvedValue({
|
||||||
|
cipher: encryptionContext.cipher,
|
||||||
|
encryptedFor: mockUserId,
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await cipherService.getRotatedData(originalUserKey, newUserKey, mockUserId);
|
||||||
|
|
||||||
|
expect(result).toHaveLength(2);
|
||||||
|
expect(cipherEncryptionService.encryptCipherForRotation).toHaveBeenCalledWith(
|
||||||
|
expect.any(CipherView),
|
||||||
|
mockUserId,
|
||||||
|
newUserKey,
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("decrypt", () => {
|
describe("decrypt", () => {
|
||||||
|
|||||||
@@ -1512,9 +1512,16 @@ export class CipherService implements CipherServiceAbstraction {
|
|||||||
if (userCiphers.length === 0) {
|
if (userCiphers.length === 0) {
|
||||||
return encryptedCiphers;
|
return encryptedCiphers;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const useSdkEncryption = await this.configService.getFeatureFlag(
|
||||||
|
FeatureFlag.PM22136_SdkCipherEncryption,
|
||||||
|
);
|
||||||
|
|
||||||
encryptedCiphers = await Promise.all(
|
encryptedCiphers = await Promise.all(
|
||||||
userCiphers.map(async (cipher) => {
|
userCiphers.map(async (cipher) => {
|
||||||
const encryptedCipher = await this.encrypt(cipher, userId, newUserKey, originalUserKey);
|
const encryptedCipher = useSdkEncryption
|
||||||
|
? await this.cipherEncryptionService.encryptCipherForRotation(cipher, userId, newUserKey)
|
||||||
|
: await this.encrypt(cipher, userId, newUserKey, originalUserKey);
|
||||||
return new CipherWithIdRequest(encryptedCipher);
|
return new CipherWithIdRequest(encryptedCipher);
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -1,6 +1,9 @@
|
|||||||
import { mock } from "jest-mock-extended";
|
import { mock } from "jest-mock-extended";
|
||||||
import { of } from "rxjs";
|
import { of } from "rxjs";
|
||||||
|
|
||||||
|
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||||
|
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||||
|
import { UserKey } from "@bitwarden/common/types/key";
|
||||||
import { Fido2Credential } from "@bitwarden/common/vault/models/domain/fido2-credential";
|
import { Fido2Credential } from "@bitwarden/common/vault/models/domain/fido2-credential";
|
||||||
import {
|
import {
|
||||||
Fido2Credential as SdkFido2Credential,
|
Fido2Credential as SdkFido2Credential,
|
||||||
@@ -91,6 +94,7 @@ describe("DefaultCipherEncryptionService", () => {
|
|||||||
vault: jest.fn().mockReturnValue({
|
vault: jest.fn().mockReturnValue({
|
||||||
ciphers: jest.fn().mockReturnValue({
|
ciphers: jest.fn().mockReturnValue({
|
||||||
encrypt: jest.fn(),
|
encrypt: jest.fn(),
|
||||||
|
encrypt_cipher_for_rotation: jest.fn(),
|
||||||
set_fido2_credentials: jest.fn(),
|
set_fido2_credentials: jest.fn(),
|
||||||
decrypt: jest.fn(),
|
decrypt: jest.fn(),
|
||||||
decrypt_list: jest.fn(),
|
decrypt_list: jest.fn(),
|
||||||
@@ -247,6 +251,31 @@ describe("DefaultCipherEncryptionService", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("encryptCipherForRotation", () => {
|
||||||
|
it("should call the sdk method to encrypt the cipher with a new key for rotation", async () => {
|
||||||
|
mockSdkClient.vault().ciphers().encrypt_cipher_for_rotation.mockReturnValue({
|
||||||
|
cipher: sdkCipher,
|
||||||
|
encryptedFor: userId,
|
||||||
|
});
|
||||||
|
|
||||||
|
const newUserKey: UserKey = new SymmetricCryptoKey(
|
||||||
|
Utils.fromUtf8ToArray("00000000000000000000000000000000"),
|
||||||
|
) as UserKey;
|
||||||
|
|
||||||
|
const result = await cipherEncryptionService.encryptCipherForRotation(
|
||||||
|
cipherViewObj,
|
||||||
|
userId,
|
||||||
|
newUserKey,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result).toBeDefined();
|
||||||
|
expect(mockSdkClient.vault().ciphers().encrypt_cipher_for_rotation).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({ id: cipherId }),
|
||||||
|
newUserKey.toBase64(),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("moveToOrganization", () => {
|
describe("moveToOrganization", () => {
|
||||||
it("should call the sdk method to move a cipher to an organization", async () => {
|
it("should call the sdk method to move a cipher to an organization", async () => {
|
||||||
const expectedCipher: Cipher = {
|
const expectedCipher: Cipher = {
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { EMPTY, catchError, firstValueFrom, map } from "rxjs";
|
import { EMPTY, catchError, firstValueFrom, map } from "rxjs";
|
||||||
|
|
||||||
|
import { UserKey } from "@bitwarden/common/types/key";
|
||||||
import { EncryptionContext } from "@bitwarden/common/vault/abstractions/cipher.service";
|
import { EncryptionContext } from "@bitwarden/common/vault/abstractions/cipher.service";
|
||||||
import {
|
import {
|
||||||
CipherListView,
|
CipherListView,
|
||||||
@@ -84,6 +85,39 @@ export class DefaultCipherEncryptionService implements CipherEncryptionService {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async encryptCipherForRotation(
|
||||||
|
model: CipherView,
|
||||||
|
userId: UserId,
|
||||||
|
newKey: UserKey,
|
||||||
|
): Promise<EncryptionContext | undefined> {
|
||||||
|
return firstValueFrom(
|
||||||
|
this.sdkService.userClient$(userId).pipe(
|
||||||
|
map((sdk) => {
|
||||||
|
if (!sdk) {
|
||||||
|
throw new Error("SDK not available");
|
||||||
|
}
|
||||||
|
|
||||||
|
using ref = sdk.take();
|
||||||
|
const sdkCipherView = this.toSdkCipherView(model, ref.value);
|
||||||
|
|
||||||
|
const encryptionContext = ref.value
|
||||||
|
.vault()
|
||||||
|
.ciphers()
|
||||||
|
.encrypt_cipher_for_rotation(sdkCipherView, newKey.toBase64());
|
||||||
|
|
||||||
|
return {
|
||||||
|
cipher: Cipher.fromSdkCipher(encryptionContext.cipher)!,
|
||||||
|
encryptedFor: asUuid<UserId>(encryptionContext.encryptedFor),
|
||||||
|
};
|
||||||
|
}),
|
||||||
|
catchError((error: unknown) => {
|
||||||
|
this.logService.error(`Failed to rotate cipher data: ${error}`);
|
||||||
|
return EMPTY;
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
async decrypt(cipher: Cipher, userId: UserId): Promise<CipherView> {
|
async decrypt(cipher: Cipher, userId: UserId): Promise<CipherView> {
|
||||||
return firstValueFrom(
|
return firstValueFrom(
|
||||||
this.sdkService.userClient$(userId).pipe(
|
this.sdkService.userClient$(userId).pipe(
|
||||||
|
|||||||
8
package-lock.json
generated
8
package-lock.json
generated
@@ -23,7 +23,7 @@
|
|||||||
"@angular/platform-browser": "19.2.14",
|
"@angular/platform-browser": "19.2.14",
|
||||||
"@angular/platform-browser-dynamic": "19.2.14",
|
"@angular/platform-browser-dynamic": "19.2.14",
|
||||||
"@angular/router": "19.2.14",
|
"@angular/router": "19.2.14",
|
||||||
"@bitwarden/sdk-internal": "0.2.0-main.231",
|
"@bitwarden/sdk-internal": "0.2.0-main.237",
|
||||||
"@electron/fuses": "1.8.0",
|
"@electron/fuses": "1.8.0",
|
||||||
"@emotion/css": "11.13.5",
|
"@emotion/css": "11.13.5",
|
||||||
"@koa/multer": "4.0.0",
|
"@koa/multer": "4.0.0",
|
||||||
@@ -4622,9 +4622,9 @@
|
|||||||
"link": true
|
"link": true
|
||||||
},
|
},
|
||||||
"node_modules/@bitwarden/sdk-internal": {
|
"node_modules/@bitwarden/sdk-internal": {
|
||||||
"version": "0.2.0-main.231",
|
"version": "0.2.0-main.237",
|
||||||
"resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.231.tgz",
|
"resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.237.tgz",
|
||||||
"integrity": "sha512-fDKB/RFVvkRPWlhL/qhPAdJDjD1EpFjpEjjpY0v5QNGalh6NCztOr1OcMc4kvipPp4g+epZjs3SPN38K6R+7zw==",
|
"integrity": "sha512-1psCagsmUo2QeIw/xFW/OCfSInl6Gu+LYldbdLuv1z26FurrgmAv8BejDaPRx006BRn0z0hn6TlZtteaZS762w==",
|
||||||
"license": "GPL-3.0",
|
"license": "GPL-3.0",
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"type-fest": "^4.41.0"
|
"type-fest": "^4.41.0"
|
||||||
|
|||||||
@@ -158,7 +158,7 @@
|
|||||||
"@angular/platform-browser": "19.2.14",
|
"@angular/platform-browser": "19.2.14",
|
||||||
"@angular/platform-browser-dynamic": "19.2.14",
|
"@angular/platform-browser-dynamic": "19.2.14",
|
||||||
"@angular/router": "19.2.14",
|
"@angular/router": "19.2.14",
|
||||||
"@bitwarden/sdk-internal": "0.2.0-main.231",
|
"@bitwarden/sdk-internal": "0.2.0-main.237",
|
||||||
"@electron/fuses": "1.8.0",
|
"@electron/fuses": "1.8.0",
|
||||||
"@emotion/css": "11.13.5",
|
"@emotion/css": "11.13.5",
|
||||||
"@koa/multer": "4.0.0",
|
"@koa/multer": "4.0.0",
|
||||||
|
|||||||
Reference in New Issue
Block a user