[TDL-11] Proposal - item decryption refactor (alternative proposal)

libs/common/spec/misc/encrypted.decorator.spec.ts

@@ -0,0 +1,18 @@
+import { encrypted, getEncryptedProperties } from "@bitwarden/common/misc/encrypted.decorator";
+
+class TestClass {
+  @encrypted
+  encryptedString: string;
+  @encrypted
+  anotherEncryptedString: string;
+
+  someOtherProperty: Date;
+}
+
+describe("encrypted decorator", () => {
+  it("adds property name to list", () => {
+    const testClass = new TestClass();
+    const result = getEncryptedProperties(testClass);
+    expect(result).toEqual(["encryptedString", "anotherEncryptedString"]);
+  });
+});

libs/common/spec/services/encrypt.service.spec.ts

@@ -10,6 +10,15 @@ import { EncryptService } from "@bitwarden/common/services/encrypt.service";
 
 import { makeStaticByteArray } from "../utils";
 
+import {
+  NestedArrayEncryptedObject,
+  NestedArrayEncryptedObjectView,
+  NestedEncryptedObject,
+  NestedEncryptedObjectView,
+  SimpleEncryptedObject,
+  SimpleEncryptedObjectView,
+} from "./encrypted-objects";
+
 describe("EncryptService", () => {
   const cryptoFunctionService = mock<CryptoFunctionService>();
   const logService = mock<LogService>();
@@ -185,4 +194,98 @@ describe("EncryptService", () => {
       expect(actual).toEqual(key);
     });
   });
+
+  describe("decryptItem", () => {
+    beforeEach(() => {
+      jest.clearAllMocks();
+    });
+
+    describe("decrypts", () => {
+      beforeEach(() => {
+        jest
+          .spyOn(encryptService, "decryptToUtf8")
+          .mockImplementation((encString, key) =>
+            Promise.resolve(encString.data.replace("_Encrypted", ""))
+          );
+      });
+
+      it("encStrings", async () => {
+        const target = new SimpleEncryptedObject(1);
+
+        const result = await encryptService.decryptItem(target, mock<SymmetricCryptoKey>());
+
+        expect(result).toMatchObject({
+          id: 1,
+          username: "myUsername1",
+          password: "myPassword1",
+        });
+        expect(result).toBeInstanceOf(SimpleEncryptedObjectView);
+      });
+
+      it("nested IDecryptables", async () => {
+        const target = new NestedEncryptedObject();
+
+        const result = await encryptService.decryptItem(target, mock<SymmetricCryptoKey>());
+
+        expect(result).toMatchObject({
+          nestedLogin1: {
+            id: 1,
+            username: "myUsername1",
+            password: "myPassword1",
+          },
+          nestedLogin2: {
+            id: 2,
+            username: "myUsername2",
+            password: "myPassword2",
+          },
+          collectionId: "myCollectionId",
+        });
+        expect(result).toBeInstanceOf(NestedEncryptedObjectView);
+        expect(result.nestedLogin1).toBeInstanceOf(SimpleEncryptedObjectView);
+        expect(result.nestedLogin2).toBeInstanceOf(SimpleEncryptedObjectView);
+      });
+
+      it("an array of nested IDecryptables", async () => {
+        const target = new NestedArrayEncryptedObject();
+
+        const result = await encryptService.decryptItem(target, mock<SymmetricCryptoKey>());
+
+        expect(result).toMatchObject({
+          logins: [
+            {
+              id: 1,
+              username: "myUsername1",
+              password: "myPassword1",
+            },
+            {
+              id: 2,
+              username: "myUsername2",
+              password: "myPassword2",
+            },
+          ],
+          collectionId: "myCollectionId",
+        });
+
+        expect(result).toBeInstanceOf(NestedArrayEncryptedObjectView);
+        expect(result.logins[0]).toBeInstanceOf(SimpleEncryptedObjectView);
+        expect(result.logins[1]).toBeInstanceOf(SimpleEncryptedObjectView);
+      });
+    });
+
+    it("handles decryption errors", async () => {
+      const target = new SimpleEncryptedObject(1);
+      jest.spyOn(encryptService, "decryptToUtf8").mockRejectedValue("some decryption error");
+
+      const result = await encryptService.decryptItem(target, mock<SymmetricCryptoKey>());
+
+      const decryptionError = "[error: cannot decrypt]";
+
+      expect(result).toMatchObject({
+        id: 1,
+        username: decryptionError,
+        password: decryptionError,
+      });
+      expect(result).toBeInstanceOf(SimpleEncryptedObjectView);
+    });
+  });
 });

libs/common/spec/services/encrypted-objects.ts

@@ -0,0 +1,73 @@
+import { Decryptable } from "../../src/interfaces/decryptable.interface";
+import { encrypted } from "../../src/misc/encrypted.decorator";
+import { EncString } from "../../src/models/domain/encString";
+
+export class SimpleEncryptedObjectView {
+  id: number;
+  username: string;
+  password: string;
+}
+
+/**
+ * An object with EncStrings and non-encrypted fields.
+ */
+export class SimpleEncryptedObject implements Decryptable<SimpleEncryptedObjectView> {
+  @encrypted username = new EncString("3.myUsername" + this.id + "_Encrypted");
+  @encrypted password = new EncString("3.myPassword" + this.id + "_Encrypted");
+
+  constructor(public id: number) {}
+
+  toView(decryptedProperties: any) {
+    return Object.assign(
+      new SimpleEncryptedObjectView(),
+      {
+        // Manually copy over unencrypted values
+        id: this.id,
+      },
+      decryptedProperties // Assign everything else from the decryptedProperties object
+    );
+  }
+}
+
+export class NestedEncryptedObjectView {}
+
+/**
+ * An object with nested encrypted objects (i.e. objects containing EncStrings)
+ */
+export class NestedEncryptedObject implements Decryptable<NestedEncryptedObjectView> {
+  @encrypted nestedLogin1 = new SimpleEncryptedObject(1);
+  @encrypted nestedLogin2 = new SimpleEncryptedObject(2);
+  collectionId = "myCollectionId";
+
+  toView(decryptedProperties: any) {
+    return Object.assign(
+      new NestedEncryptedObjectView(),
+      {
+        // Manually copy over unencrypted values
+        collectionId: this.collectionId,
+      },
+      decryptedProperties // Assign everything else from the decryptedProperties object
+    );
+  }
+}
+
+export class NestedArrayEncryptedObjectView {}
+
+/**
+ * An object with nested encrypted objects (i.e. objects containing EncStrings) in an array
+ */
+export class NestedArrayEncryptedObject implements Decryptable<NestedArrayEncryptedObjectView> {
+  @encrypted logins = [new SimpleEncryptedObject(1), new SimpleEncryptedObject(2)];
+  collectionId = "myCollectionId";
+
+  toView(decryptedProperties: any) {
+    return Object.assign(
+      new NestedArrayEncryptedObjectView(),
+      {
+        // Manually copy over unencrypted values
+        collectionId: this.collectionId,
+      },
+      decryptedProperties // Assign everything else from the decryptedProperties object
+    );
+  }
+}

libs/common/src/abstractions/abstractEncrypt.service.ts

@@ -1,4 +1,5 @@
 import { IEncrypted } from "../interfaces/IEncrypted";
+import { Decryptable } from "../interfaces/decryptable.interface";
 import { EncArrayBuffer } from "../models/domain/encArrayBuffer";
 import { EncString } from "../models/domain/encString";
 import { SymmetricCryptoKey } from "../models/domain/symmetricCryptoKey";
@@ -12,4 +13,5 @@ export abstract class AbstractEncryptService {
   abstract decryptToUtf8: (encString: EncString, key: SymmetricCryptoKey) => Promise<string>;
   abstract decryptToBytes: (encThing: IEncrypted, key: SymmetricCryptoKey) => Promise<ArrayBuffer>;
   abstract resolveLegacyKey: (key: SymmetricCryptoKey, encThing: IEncrypted) => SymmetricCryptoKey;
+  abstract decryptItem: <T>(item: Decryptable<T>, key: SymmetricCryptoKey) => Promise<T>;
 }

libs/common/src/interfaces/decryptable.interface.ts

@@ -0,0 +1,3 @@
+export interface Decryptable<T> {
+  toView: (decryptedProperties: any) => T;
+}

libs/common/src/misc/encrypted.decorator.ts

@@ -0,0 +1,16 @@
+import "reflect-metadata"; // TODO: this can only be imported in one place, so shouldn't be here
+
+const metadataKey = Symbol("encryptedPropertiesKey");
+
+export function encrypted(prototype: any, propertyKey: string) {
+  const encStringList: string[] = Reflect.getMetadata(metadataKey, prototype);
+  if (encStringList == null) {
+    Reflect.defineMetadata(metadataKey, [propertyKey], prototype);
+  } else {
+    encStringList.push(propertyKey);
+  }
+}
+
+export function getEncryptedProperties(target: any): string[] {
+  return Reflect.getMetadata(metadataKey, target);
+}

libs/common/src/models/domain/encString.ts

@@ -9,11 +9,15 @@ import { SymmetricCryptoKey } from "./symmetricCryptoKey";
 export class EncString implements IEncrypted {
   encryptedString?: string;
   encryptionType?: EncryptionType;
-  decryptedValue?: string;
   data?: string;
   iv?: string;
   mac?: string;
 
+  /**
+   * @deprecated: EncStrings no longer store their decrypted values
+   */
+  private decryptedValue?: string;
+
   constructor(
     encryptedStringOrType: string | EncryptionType,
     data?: string,

libs/common/src/services/encrypt.service.ts

@@ -3,11 +3,14 @@ import { CryptoFunctionService } from "../abstractions/cryptoFunction.service";
 import { LogService } from "../abstractions/log.service";
 import { EncryptionType } from "../enums/encryptionType";
 import { IEncrypted } from "../interfaces/IEncrypted";
+import { Decryptable } from "../interfaces/decryptable.interface";
+import { getEncryptedProperties } from "../misc/encrypted.decorator";
 import { Utils } from "../misc/utils";
 import { EncArrayBuffer } from "../models/domain/encArrayBuffer";
 import { EncString } from "../models/domain/encString";
 import { EncryptedObject } from "../models/domain/encryptedObject";
 import { SymmetricCryptoKey } from "../models/domain/symmetricCryptoKey";
+import { View } from "../models/view/view";
 
 export class EncryptService implements AbstractEncryptService {
   constructor(
@@ -164,6 +167,61 @@ export class EncryptService implements AbstractEncryptService {
     return obj;
   }
 
+  async decryptItem<T>(item: Decryptable<T>, key: SymmetricCryptoKey): Promise<T> {
+    if (item == null) {
+      throw new Error("Cannot decrypt a null item");
+    }
+
+    const promises: Promise<any>[] = [];
+    const decryptedValues: Record<string, string | View | Array<View>> = {};
+
+    const encryptedProperties = getEncryptedProperties(item);
+    encryptedProperties?.forEach((prop) => {
+      const propValue = (item as any)[prop];
+      if (propValue == null) {
+        return;
+      }
+
+      // Case 1: the property is an EncString
+      if (propValue instanceof EncString) {
+        promises.push(
+          this.decryptToUtf8(propValue, key)
+            .then((decryptedValue) => (decryptedValues[prop] = decryptedValue))
+            .catch((e) => {
+              this.logService.error(e);
+              decryptedValues[prop] = "[error: cannot decrypt]";
+            })
+        );
+        return;
+      }
+
+      // Case 2: the property is an array of nested decryptables
+      // NB: arrays of EncStrings are not supported at this time (no current use cases)
+      if (propValue instanceof Array) {
+        decryptedValues[prop] = [];
+
+        propValue.forEach((subItem) =>
+          promises.push(
+            this.decryptItem(subItem, key).then((decryptedValue) =>
+              (decryptedValues[prop] as Array<View>).push(decryptedValue)
+            )
+          )
+        );
+      } else {
+        // Case 3: the property is a single nested decryptable
+        promises.push(
+          this.decryptItem(propValue, key).then(
+            (decryptedValue) => (decryptedValues[prop] = decryptedValue)
+          )
+        );
+      }
+    });
+
+    await Promise.all(promises);
+
+    return item.toView(decryptedValues);
+  }
+
   private logMacFailed(msg: string) {
     if (this.logMacFailures) {
       this.logService.error(msg);