From edeb0f4597f78daab4f45d2dfd19023fbfc54625 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 28 Jul 2025 18:18:36 +0200 Subject: [PATCH] Remove decrypt with key from EncString, domain-base (#15702) --- .../crypto/models/enc-string.spec.ts | 63 +-------- .../crypto/models/enc-string.ts | 20 --- .../models/domain/domain-base.spec.ts | 131 ------------------ .../src/platform/models/domain/domain-base.ts | 65 +-------- .../src/vault/models/domain/folder.spec.ts | 12 +- libs/common/src/vault/models/domain/folder.ts | 10 +- 6 files changed, 8 insertions(+), 293 deletions(-) delete mode 100644 libs/common/src/platform/models/domain/domain-base.spec.ts diff --git a/libs/common/src/key-management/crypto/models/enc-string.spec.ts b/libs/common/src/key-management/crypto/models/enc-string.spec.ts index ec1434c55d2..ca9abb5d9d7 100644 --- a/libs/common/src/key-management/crypto/models/enc-string.spec.ts +++ b/libs/common/src/key-management/crypto/models/enc-string.spec.ts @@ -4,7 +4,7 @@ import { mock, MockProxy } from "jest-mock-extended"; // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; -import { makeEncString, makeStaticByteArray } from "../../../../spec"; +import { makeStaticByteArray } from "../../../../spec"; import { EncryptionType } from "../../../platform/enums"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { ContainerService } from "../../../platform/services/container.service"; @@ -114,67 +114,6 @@ describe("EncString", () => { }); }); - describe("decryptWithKey", () => { - const encString = new EncString(EncryptionType.Rsa2048_OaepSha256_B64, "data"); - - const keyService = mock(); - const encryptService = mock(); - encryptService.decryptString - .calledWith(encString, expect.anything()) - .mockResolvedValue("decrypted"); - - function setupEncryption() { - encryptService.encryptString.mockImplementation(async (data, key) => { - return makeEncString(data); - }); - encryptService.decryptString.mockImplementation(async (encString, key) => { - return encString.data; - }); - } - - beforeEach(() => { - (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); - }); - - it("decrypts using the provided key and encryptService", async () => { - setupEncryption(); - - const key = new SymmetricCryptoKey(makeStaticByteArray(32)); - await encString.decryptWithKey(key, encryptService); - - expect(encryptService.decryptString).toHaveBeenCalledWith(encString, key); - }); - - it("fails to decrypt when key is null", async () => { - const decrypted = await encString.decryptWithKey(null, encryptService); - - expect(decrypted).toBe("[error: cannot decrypt]"); - expect(encString.decryptedValue).toBe("[error: cannot decrypt]"); - }); - - it("fails to decrypt when encryptService is null", async () => { - const decrypted = await encString.decryptWithKey( - new SymmetricCryptoKey(makeStaticByteArray(32)), - null, - ); - - expect(decrypted).toBe("[error: cannot decrypt]"); - expect(encString.decryptedValue).toBe("[error: cannot decrypt]"); - }); - - it("fails to decrypt when encryptService throws", async () => { - encryptService.decryptString.mockRejectedValue("error"); - - const decrypted = await encString.decryptWithKey( - new SymmetricCryptoKey(makeStaticByteArray(32)), - encryptService, - ); - - expect(decrypted).toBe("[error: cannot decrypt]"); - expect(encString.decryptedValue).toBe("[error: cannot decrypt]"); - }); - }); - describe("AesCbc256_B64", () => { it("constructor", () => { const encString = new EncString(EncryptionType.AesCbc256_B64, "data", "iv"); diff --git a/libs/common/src/key-management/crypto/models/enc-string.ts b/libs/common/src/key-management/crypto/models/enc-string.ts index 3478ced0cf3..f215398d92a 100644 --- a/libs/common/src/key-management/crypto/models/enc-string.ts +++ b/libs/common/src/key-management/crypto/models/enc-string.ts @@ -6,7 +6,6 @@ import { EncryptionType, EXPECTED_NUM_PARTS_BY_ENCRYPTION_TYPE } from "../../../ import { Encrypted } from "../../../platform/interfaces/encrypted"; import { Utils } from "../../../platform/misc/utils"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; -import { EncryptService } from "../abstractions/encrypt.service"; export const DECRYPT_ERROR = "[error: cannot decrypt]"; @@ -184,25 +183,6 @@ export class EncString implements Encrypted { return this.decryptedValue; } - async decryptWithKey( - key: SymmetricCryptoKey, - encryptService: EncryptService, - decryptTrace: string = "domain-withkey", - ): Promise { - try { - if (key == null) { - throw new Error("No key to decrypt EncString"); - } - - this.decryptedValue = await encryptService.decryptString(this, key); - // FIXME: Remove when updating file. Eslint update - // eslint-disable-next-line @typescript-eslint/no-unused-vars - } catch (e) { - this.decryptedValue = DECRYPT_ERROR; - } - - return this.decryptedValue; - } private async getKeyForDecryption(orgId: string) { const keyService = Utils.getContainerService().getKeyService(); return orgId != null diff --git a/libs/common/src/platform/models/domain/domain-base.spec.ts b/libs/common/src/platform/models/domain/domain-base.spec.ts deleted file mode 100644 index 9233795eb95..00000000000 --- a/libs/common/src/platform/models/domain/domain-base.spec.ts +++ /dev/null @@ -1,131 +0,0 @@ -import { mock, MockProxy } from "jest-mock-extended"; - -import { makeEncString, makeSymmetricCryptoKey } from "../../../../spec"; -import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; -import { EncString } from "../../../key-management/crypto/models/enc-string"; - -import Domain from "./domain-base"; - -class TestDomain extends Domain { - plainText: string; - encToString: EncString; - encString2: EncString; -} - -describe("DomainBase", () => { - let encryptService: MockProxy; - const key = makeSymmetricCryptoKey(64); - - beforeEach(() => { - encryptService = mock(); - }); - - function setUpCryptography() { - encryptService.encryptString.mockImplementation((value) => - Promise.resolve(makeEncString(value)), - ); - - encryptService.decryptString.mockImplementation((value) => { - return Promise.resolve(value.data); - }); - } - - describe("decryptWithKey", () => { - it("domain property types are decryptable", async () => { - const domain = new TestDomain(); - - await domain["decryptObjWithKey"]( - // @ts-expect-error -- clear is not of type EncString - ["plainText"], - makeSymmetricCryptoKey(64), - mock(), - ); - - await domain["decryptObjWithKey"]( - // @ts-expect-error -- Clear is not of type EncString - ["encToString", "encString2", "plainText"], - makeSymmetricCryptoKey(64), - mock(), - ); - - const decrypted = await domain["decryptObjWithKey"]( - ["encToString"], - makeSymmetricCryptoKey(64), - mock(), - ); - - // @ts-expect-error -- encString2 was not decrypted - // FIXME: Remove when updating file. Eslint update - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - decrypted as { encToString: string; encString2: string; plainText: string }; - - // encString2 was not decrypted, so it's still an EncString - // FIXME: Remove when updating file. Eslint update - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - decrypted as { encToString: string; encString2: EncString; plainText: string }; - }); - - it("decrypts the encrypted properties", async () => { - setUpCryptography(); - - const domain = new TestDomain(); - - domain.encToString = await encryptService.encryptString("string", key); - - const decrypted = await domain["decryptObjWithKey"](["encToString"], key, encryptService); - - expect(decrypted).toEqual({ - encToString: "string", - }); - }); - - it("decrypts multiple encrypted properties", async () => { - setUpCryptography(); - - const domain = new TestDomain(); - - domain.encToString = await encryptService.encryptString("string", key); - domain.encString2 = await encryptService.encryptString("string2", key); - - const decrypted = await domain["decryptObjWithKey"]( - ["encToString", "encString2"], - key, - encryptService, - ); - - expect(decrypted).toEqual({ - encToString: "string", - encString2: "string2", - }); - }); - - it("does not decrypt properties that are not encrypted", async () => { - const domain = new TestDomain(); - domain.plainText = "clear"; - - const decrypted = await domain["decryptObjWithKey"]([], key, encryptService); - - expect(decrypted).toEqual({ - plainText: "clear", - }); - }); - - it("does not decrypt properties that were not requested to be decrypted", async () => { - setUpCryptography(); - - const domain = new TestDomain(); - - domain.plainText = "clear"; - domain.encToString = makeEncString("string"); - domain.encString2 = makeEncString("string2"); - - const decrypted = await domain["decryptObjWithKey"]([], key, encryptService); - - expect(decrypted).toEqual({ - plainText: "clear", - encToString: makeEncString("string"), - encString2: makeEncString("string2"), - }); - }); - }); -}); diff --git a/libs/common/src/platform/models/domain/domain-base.ts b/libs/common/src/platform/models/domain/domain-base.ts index 58282a665af..b999a5e5d15 100644 --- a/libs/common/src/platform/models/domain/domain-base.ts +++ b/libs/common/src/platform/models/domain/domain-base.ts @@ -1,6 +1,5 @@ -import { ConditionalExcept, ConditionalKeys, Constructor } from "type-fest"; +import { ConditionalExcept, ConditionalKeys } from "type-fest"; -import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; import { EncString } from "../../../key-management/crypto/models/enc-string"; import { View } from "../../../models/view/view"; @@ -89,66 +88,4 @@ export default class Domain { return viewModel as V; } - - /** - * Decrypts the requested properties of the domain object with the provided key and encrypt service. - * - * If a property is null, the result will be null. - * @see {@link EncString.decryptWithKey} for more details on decryption behavior. - * - * @param encryptedProperties The properties to decrypt. Type restricted to EncString properties of the domain object. - * @param key The key to use for decryption. - * @param encryptService The encryption service to use for decryption. - * @param _ The constructor of the domain object. Used for type inference if the domain object is not automatically inferred. - * @returns An object with the requested properties decrypted and the rest of the domain object untouched. - */ - protected async decryptObjWithKey< - TThis extends Domain, - const TEncryptedKeys extends EncStringKeys, - >( - this: TThis, - encryptedProperties: TEncryptedKeys[], - key: SymmetricCryptoKey, - encryptService: EncryptService, - _: Constructor = this.constructor as Constructor, - objectContext: string = "No Domain Context", - ): Promise> { - const decryptedObjects = []; - - for (const prop of encryptedProperties) { - const value = this[prop] as EncString; - const decrypted = await this.decryptProperty( - prop, - value, - key, - encryptService, - `Property: ${prop.toString()}; ObjectContext: ${objectContext}`, - ); - decryptedObjects.push(decrypted); - } - - const decryptedObject = decryptedObjects.reduce( - (acc, obj) => { - return { ...acc, ...obj }; - }, - { ...this }, - ); - return decryptedObject as DecryptedObject; - } - - private async decryptProperty>( - propertyKey: TEncryptedKeys, - value: EncString, - key: SymmetricCryptoKey, - encryptService: EncryptService, - decryptTrace: string, - ) { - let decrypted: string | null = null; - if (value) { - decrypted = await value.decryptWithKey(key, encryptService, decryptTrace); - } - return { - [propertyKey]: decrypted, - }; - } } diff --git a/libs/common/src/vault/models/domain/folder.spec.ts b/libs/common/src/vault/models/domain/folder.spec.ts index 82a3aba22a2..2e5e7c74a3d 100644 --- a/libs/common/src/vault/models/domain/folder.spec.ts +++ b/libs/common/src/vault/models/domain/folder.spec.ts @@ -1,7 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; - import { makeEncString, makeSymmetricCryptoKey, mockEnc, mockFromJson } from "../../../../spec"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string"; @@ -72,15 +70,7 @@ describe("Folder", () => { beforeEach(() => { encryptService = mock(); - // Platform code is not migrated yet - encryptService.decryptToUtf8.mockImplementation( - (value: EncString, key: SymmetricCryptoKey, decryptTrace: string) => { - return Promise.resolve(value.data); - }, - ); - encryptService.decryptString.mockImplementation((value) => { - return Promise.resolve(value.data); - }); + encryptService.decryptString.mockResolvedValue("encName"); }); it("decrypts the name", async () => { diff --git a/libs/common/src/vault/models/domain/folder.ts b/libs/common/src/vault/models/domain/folder.ts index acefb6ce1c5..01f5b9599bd 100644 --- a/libs/common/src/vault/models/domain/folder.ts +++ b/libs/common/src/vault/models/domain/folder.ts @@ -47,11 +47,11 @@ export class Folder extends Domain { key: SymmetricCryptoKey, encryptService: EncryptService, ): Promise { - const decrypted = await this.decryptObjWithKey(["name"], key, encryptService, Folder); - - const view = new FolderView(decrypted); - view.name = decrypted.name; - return view; + const folderView = new FolderView(); + folderView.id = this.id; + folderView.revisionDate = this.revisionDate; + folderView.name = await encryptService.decryptString(this.name, key); + return folderView; } static fromJSON(obj: Jsonify) {