Ps 1291/apply to from json pattern to state (#3425)

* Clean up dangling behaviorSubject

* Handle null in utils

* fix null check

* Await promises, even in async functions

* Add to/fromJSON methods to State and Accounts

This is needed since all storage in manifest v3 is key-value-pair-based
and session storage of most data is actually serialized into an
encrypted string.

* Simplify AccountKeys json parsing

* Fix account key (de)serialization

* Remove unused DecodedToken state

* Correct filename typo

* Simplify keys `toJSON` tests

* Explain AccountKeys `toJSON` return type

* Remove unnecessary `any`s

* Remove unique ArrayBuffer serialization

* Initialize items in MemoryStorageService

* Revert "Fix account key (de)serialization"

This reverts commit b1dffb5c2c, which was breaking serializations

* Move fromJSON to owning object

* Add DeepJsonify type

* Use Records for storage

* Add new Account Settings to serialized data

* Fix failing serialization tests

* Extract complex type conversion to helper methods

* Remove unnecessary decorator

* Return null from json deserializers

* Remove unnecessary decorators

* Remove obsolete test

* Use type-fest `Jsonify` formatting rules for external library

* Update jsonify comment

Co-authored-by: @eliykat

* Remove erroneous comment

* Fix unintended deep-jsonify changes

* Fix prettierignore

* Fix formatting of deep-jsonify.ts

Co-authored-by: Thomas Rittson <trittson@bitwarden.com>
Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>

libs/common/src/models/data/server-config.data.spec.ts

@@ -0,0 +1,55 @@
+import {
+  EnvironmentServerConfigData,
+  ServerConfigData,
+  ThirdPartyServerConfigData,
+} from "./server-config.data";
+
+describe("ServerConfigData", () => {
+  describe("fromJSON", () => {
+    it("should create a ServerConfigData from a JSON object", () => {
+      const serverConfigData = ServerConfigData.fromJSON({
+        version: "1.0.0",
+        gitHash: "1234567890",
+        server: {
+          name: "test",
+          url: "https://test.com",
+        },
+        environment: {
+          vault: "https://vault.com",
+          api: "https://api.com",
+          identity: "https://identity.com",
+          notifications: "https://notifications.com",
+          sso: "https://sso.com",
+        },
+        utcDate: "2020-01-01T00:00:00.000Z",
+      });
+
+      expect(serverConfigData.version).toEqual("1.0.0");
+      expect(serverConfigData.gitHash).toEqual("1234567890");
+      expect(serverConfigData.server.name).toEqual("test");
+      expect(serverConfigData.server.url).toEqual("https://test.com");
+      expect(serverConfigData.environment.vault).toEqual("https://vault.com");
+      expect(serverConfigData.environment.api).toEqual("https://api.com");
+      expect(serverConfigData.environment.identity).toEqual("https://identity.com");
+      expect(serverConfigData.environment.notifications).toEqual("https://notifications.com");
+      expect(serverConfigData.environment.sso).toEqual("https://sso.com");
+      expect(serverConfigData.utcDate).toEqual("2020-01-01T00:00:00.000Z");
+    });
+
+    it("should be an instance of ServerConfigData", () => {
+      const serverConfigData = ServerConfigData.fromJSON({} as any);
+
+      expect(serverConfigData).toBeInstanceOf(ServerConfigData);
+    });
+
+    it("should deserialize sub objects", () => {
+      const serverConfigData = ServerConfigData.fromJSON({
+        server: {},
+        environment: {},
+      } as any);
+
+      expect(serverConfigData.server).toBeInstanceOf(ThirdPartyServerConfigData);
+      expect(serverConfigData.environment).toBeInstanceOf(EnvironmentServerConfigData);
+    });
+  });
+});

libs/common/src/models/data/server-config.data.ts

@@ -1,3 +1,5 @@
+import { Jsonify } from "type-fest";
+
 import {
   ServerConfigResponse,
   ThirdPartyServerConfigResponse,
@@ -11,27 +13,38 @@ export class ServerConfigData {
   environment?: EnvironmentServerConfigData;
   utcDate: string;
 
-  constructor(serverConfigReponse: ServerConfigResponse) {
-    this.version = serverConfigReponse?.version;
-    this.gitHash = serverConfigReponse?.gitHash;
-    this.server = serverConfigReponse?.server
-      ? new ThirdPartyServerConfigData(serverConfigReponse.server)
+  constructor(serverConfigResponse: Partial<ServerConfigResponse>) {
+    this.version = serverConfigResponse?.version;
+    this.gitHash = serverConfigResponse?.gitHash;
+    this.server = serverConfigResponse?.server
+      ? new ThirdPartyServerConfigData(serverConfigResponse.server)
       : null;
     this.utcDate = new Date().toISOString();
-    this.environment = serverConfigReponse?.environment
-      ? new EnvironmentServerConfigData(serverConfigReponse.environment)
+    this.environment = serverConfigResponse?.environment
+      ? new EnvironmentServerConfigData(serverConfigResponse.environment)
       : null;
   }
+
+  static fromJSON(obj: Jsonify<ServerConfigData>): ServerConfigData {
+    return Object.assign(new ServerConfigData({}), obj, {
+      server: obj?.server ? ThirdPartyServerConfigData.fromJSON(obj.server) : null,
+      environment: obj?.environment ? EnvironmentServerConfigData.fromJSON(obj.environment) : null,
+    });
+  }
 }
 
 export class ThirdPartyServerConfigData {
   name: string;
   url: string;
 
-  constructor(response: ThirdPartyServerConfigResponse) {
+  constructor(response: Partial<ThirdPartyServerConfigResponse>) {
     this.name = response.name;
     this.url = response.url;
   }
+
+  static fromJSON(obj: Jsonify<ThirdPartyServerConfigData>): ThirdPartyServerConfigData {
+    return Object.assign(new ThirdPartyServerConfigData({}), obj);
+  }
 }
 
 export class EnvironmentServerConfigData {
@@ -41,11 +54,15 @@ export class EnvironmentServerConfigData {
   notifications: string;
   sso: string;
 
-  constructor(response: EnvironmentServerConfigResponse) {
+  constructor(response: Partial<EnvironmentServerConfigResponse>) {
     this.vault = response.vault;
     this.api = response.api;
     this.identity = response.identity;
     this.notifications = response.notifications;
     this.sso = response.sso;
   }
+
+  static fromJSON(obj: Jsonify<EnvironmentServerConfigData>): EnvironmentServerConfigData {
+    return Object.assign(new EnvironmentServerConfigData({}), obj);
+  }
 }

libs/common/src/models/domain/account-keys.spec.ts

@@ -0,0 +1,62 @@
+import { Utils } from "@bitwarden/common/misc/utils";
+
+import { makeStaticByteArray } from "../../../spec/utils";
+
+import { AccountKeys, EncryptionPair } from "./account";
+import { SymmetricCryptoKey } from "./symmetricCryptoKey";
+
+describe("AccountKeys", () => {
+  describe("toJSON", () => {
+    it("should serialize itself", () => {
+      const keys = new AccountKeys();
+      const buffer = makeStaticByteArray(64).buffer;
+      keys.publicKey = buffer;
+
+      const bufferSpy = jest.spyOn(Utils, "fromBufferToByteString");
+      keys.toJSON();
+      expect(bufferSpy).toHaveBeenCalledWith(buffer);
+    });
+
+    it("should serialize public key as a string", () => {
+      const keys = new AccountKeys();
+      keys.publicKey = Utils.fromByteStringToArray("hello").buffer;
+      const json = JSON.stringify(keys);
+      expect(json).toContain('"publicKey":"hello"');
+    });
+  });
+
+  describe("fromJSON", () => {
+    it("should deserialize public key to a buffer", () => {
+      const keys = AccountKeys.fromJSON({
+        publicKey: "hello",
+      });
+      expect(keys.publicKey).toEqual(Utils.fromByteStringToArray("hello").buffer);
+    });
+
+    it("should deserialize cryptoMasterKey", () => {
+      const spy = jest.spyOn(SymmetricCryptoKey, "fromJSON");
+      AccountKeys.fromJSON({} as any);
+      expect(spy).toHaveBeenCalled();
+    });
+
+    it("should deserialize organizationKeys", () => {
+      const spy = jest.spyOn(SymmetricCryptoKey, "fromJSON");
+      AccountKeys.fromJSON({ organizationKeys: [{ orgId: "keyJSON" }] } as any);
+      expect(spy).toHaveBeenCalled();
+    });
+
+    it("should deserialize providerKeys", () => {
+      const spy = jest.spyOn(SymmetricCryptoKey, "fromJSON");
+      AccountKeys.fromJSON({ providerKeys: [{ providerId: "keyJSON" }] } as any);
+      expect(spy).toHaveBeenCalled();
+    });
+
+    it("should deserialize privateKey", () => {
+      const spy = jest.spyOn(EncryptionPair, "fromJSON");
+      AccountKeys.fromJSON({
+        privateKey: { encrypted: "encrypted", decrypted: "decrypted" },
+      } as any);
+      expect(spy).toHaveBeenCalled();
+    });
+  });
+});

libs/common/src/models/domain/account-profile.spec.ts

@@ -0,0 +1,9 @@
+import { AccountProfile } from "./account";
+
+describe("AccountProfile", () => {
+  describe("fromJSON", () => {
+    it("should deserialize to an instance of itself", () => {
+      expect(AccountProfile.fromJSON({})).toBeInstanceOf(AccountProfile);
+    });
+  });
+});

libs/common/src/models/domain/account-settings.spec.ts

@@ -0,0 +1,24 @@
+import { AccountSettings, EncryptionPair } from "./account";
+import { EncString } from "./encString";
+
+describe("AccountSettings", () => {
+  describe("fromJSON", () => {
+    it("should deserialize to an instance of itself", () => {
+      expect(AccountSettings.fromJSON(JSON.parse("{}"))).toBeInstanceOf(AccountSettings);
+    });
+
+    it("should deserialize pinProtected", () => {
+      const accountSettings = new AccountSettings();
+      accountSettings.pinProtected = EncryptionPair.fromJSON<string, EncString>({
+        encrypted: "encrypted",
+        decrypted: "3.data",
+      });
+      const jsonObj = JSON.parse(JSON.stringify(accountSettings));
+      const actual = AccountSettings.fromJSON(jsonObj);
+
+      expect(actual.pinProtected).toBeInstanceOf(EncryptionPair);
+      expect(actual.pinProtected.encrypted).toEqual("encrypted");
+      expect(actual.pinProtected.decrypted.encryptedString).toEqual("3.data");
+    });
+  });
+});

libs/common/src/models/domain/account-tokens.spec.ts

@@ -0,0 +1,9 @@
+import { AccountTokens } from "./account";
+
+describe("AccountTokens", () => {
+  describe("fromJSON", () => {
+    it("should deserialize to an instance of itself", () => {
+      expect(AccountTokens.fromJSON({})).toBeInstanceOf(AccountTokens);
+    });
+  });
+});

libs/common/src/models/domain/account.spec.ts

@@ -0,0 +1,23 @@
+import { Account, AccountKeys, AccountProfile, AccountSettings, AccountTokens } from "./account";
+
+describe("Account", () => {
+  describe("fromJSON", () => {
+    it("should deserialize to an instance of itself", () => {
+      expect(Account.fromJSON({})).toBeInstanceOf(Account);
+    });
+
+    it("should call all the sub-fromJSONs", () => {
+      const keysSpy = jest.spyOn(AccountKeys, "fromJSON");
+      const profileSpy = jest.spyOn(AccountProfile, "fromJSON");
+      const settingsSpy = jest.spyOn(AccountSettings, "fromJSON");
+      const tokensSpy = jest.spyOn(AccountTokens, "fromJSON");
+
+      Account.fromJSON({});
+
+      expect(keysSpy).toHaveBeenCalled();
+      expect(profileSpy).toHaveBeenCalled();
+      expect(settingsSpy).toHaveBeenCalled();
+      expect(tokensSpy).toHaveBeenCalled();
+    });
+  });
+});

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

@@ -1,3 +1,8 @@
+import { Except, Jsonify } from "type-fest";
+
+import { Utils } from "@bitwarden/common/misc/utils";
+import { DeepJsonify } from "@bitwarden/common/types/deep-jsonify";
+
 import { AuthenticationStatus } from "../../enums/authenticationStatus";
 import { KdfType } from "../../enums/kdfType";
 import { UriMatchType } from "../../enums/uriMatchType";
@@ -24,7 +29,39 @@ import { SymmetricCryptoKey } from "./symmetricCryptoKey";
 export class EncryptionPair<TEncrypted, TDecrypted> {
   encrypted?: TEncrypted;
   decrypted?: TDecrypted;
-  decryptedSerialized?: string;
+
+  toJSON() {
+    return {
+      encrypted: this.encrypted,
+      decrypted:
+        this.decrypted instanceof ArrayBuffer
+          ? Utils.fromBufferToByteString(this.decrypted)
+          : this.decrypted,
+    };
+  }
+
+  static fromJSON<TEncrypted, TDecrypted>(
+    obj: Jsonify<EncryptionPair<Jsonify<TEncrypted>, Jsonify<TDecrypted>>>,
+    decryptedFromJson?: (decObj: Jsonify<TDecrypted> | string) => TDecrypted,
+    encryptedFromJson?: (encObj: Jsonify<TEncrypted>) => TEncrypted
+  ) {
+    if (obj == null) {
+      return null;
+    }
+
+    const pair = new EncryptionPair<TEncrypted, TDecrypted>();
+    if (obj?.encrypted != null) {
+      pair.encrypted = encryptedFromJson
+        ? encryptedFromJson(obj.encrypted)
+        : (obj.encrypted as TEncrypted);
+    }
+    if (obj?.decrypted != null) {
+      pair.decrypted = decryptedFromJson
+        ? decryptedFromJson(obj.decrypted)
+        : (obj.decrypted as TDecrypted);
+    }
+    return pair;
+  }
 }
 
 export class DataEncryptionPair<TEncrypted, TDecrypted> {
@@ -73,19 +110,66 @@ export class AccountKeys {
   >();
   organizationKeys?: EncryptionPair<
     { [orgId: string]: EncryptedOrganizationKeyData },
-    Map<string, SymmetricCryptoKey>
+    Record<string, SymmetricCryptoKey>
   > = new EncryptionPair<
     { [orgId: string]: EncryptedOrganizationKeyData },
-    Map<string, SymmetricCryptoKey>
+    Record<string, SymmetricCryptoKey>
   >();
-  providerKeys?: EncryptionPair<any, Map<string, SymmetricCryptoKey>> = new EncryptionPair<
+  providerKeys?: EncryptionPair<any, Record<string, SymmetricCryptoKey>> = new EncryptionPair<
     any,
-    Map<string, SymmetricCryptoKey>
+    Record<string, SymmetricCryptoKey>
   >();
   privateKey?: EncryptionPair<string, ArrayBuffer> = new EncryptionPair<string, ArrayBuffer>();
   publicKey?: ArrayBuffer;
-  publicKeySerialized?: string;
   apiKeyClientSecret?: string;
+
+  toJSON() {
+    return Object.assign(this as Except<AccountKeys, "publicKey">, {
+      publicKey: Utils.fromBufferToByteString(this.publicKey),
+    });
+  }
+
+  static fromJSON(obj: DeepJsonify<AccountKeys>): AccountKeys {
+    if (obj == null) {
+      return null;
+    }
+
+    return Object.assign(
+      new AccountKeys(),
+      { cryptoMasterKey: SymmetricCryptoKey.fromJSON(obj?.cryptoMasterKey) },
+      {
+        cryptoSymmetricKey: EncryptionPair.fromJSON(
+          obj?.cryptoSymmetricKey,
+          SymmetricCryptoKey.fromJSON
+        ),
+      },
+      { organizationKeys: AccountKeys.initRecordEncryptionPairsFromJSON(obj?.organizationKeys) },
+      { providerKeys: AccountKeys.initRecordEncryptionPairsFromJSON(obj?.providerKeys) },
+      {
+        privateKey: EncryptionPair.fromJSON<string, ArrayBuffer>(
+          obj?.privateKey,
+          (decObj: string) => Utils.fromByteStringToArray(decObj).buffer
+        ),
+      },
+      {
+        publicKey: Utils.fromByteStringToArray(obj?.publicKey)?.buffer,
+      }
+    );
+  }
+
+  static initRecordEncryptionPairsFromJSON(obj: any) {
+    return EncryptionPair.fromJSON(obj, (decObj: any) => {
+      if (obj == null) {
+        return null;
+      }
+
+      const record: Record<string, SymmetricCryptoKey> = {};
+      for (const id in decObj) {
+        record[id] = SymmetricCryptoKey.fromJSON(decObj[id]);
+      }
+      return record;
+    });
+  }
 }
 
 export class AccountProfile {
@@ -106,6 +190,14 @@ export class AccountProfile {
   keyHash?: string;
   kdfIterations?: number;
   kdfType?: KdfType;
+
+  static fromJSON(obj: Jsonify<AccountProfile>): AccountProfile {
+    if (obj == null) {
+      return null;
+    }
+
+    return Object.assign(new AccountProfile(), obj);
+  }
 }
 
 export class AccountSettings {
@@ -142,6 +234,21 @@ export class AccountSettings {
   vaultTimeout?: number;
   vaultTimeoutAction?: string = "lock";
   serverConfig?: ServerConfigData;
+
+  static fromJSON(obj: Jsonify<AccountSettings>): AccountSettings {
+    if (obj == null) {
+      return null;
+    }
+
+    return Object.assign(new AccountSettings(), obj, {
+      environmentUrls: EnvironmentUrls.fromJSON(obj?.environmentUrls),
+      pinProtected: EncryptionPair.fromJSON<string, EncString>(
+        obj?.pinProtected,
+        EncString.fromJSON
+      ),
+      serverConfig: ServerConfigData.fromJSON(obj?.serverConfig),
+    });
+  }
 }
 
 export type AccountSettingsSettings = {
@@ -150,9 +257,16 @@ export type AccountSettingsSettings = {
 
 export class AccountTokens {
   accessToken?: string;
-  decodedToken?: any;
   refreshToken?: string;
   securityStamp?: string;
+
+  static fromJSON(obj: Jsonify<AccountTokens>): AccountTokens {
+    if (obj == null) {
+      return null;
+    }
+
+    return Object.assign(new AccountTokens(), obj);
+  }
 }
 
 export class Account {
@@ -186,4 +300,17 @@ export class Account {
       },
     });
   }
+
+  static fromJSON(json: Jsonify<Account>): Account {
+    if (json == null) {
+      return null;
+    }
+
+    return Object.assign(new Account({}), json, {
+      keys: AccountKeys.fromJSON(json?.keys),
+      profile: AccountProfile.fromJSON(json?.profile),
+      settings: AccountSettings.fromJSON(json?.settings),
+      tokens: AccountTokens.fromJSON(json?.tokens),
+    });
+  }
 }

libs/common/src/models/domain/encryption-pair.spec.ts

@@ -0,0 +1,34 @@
+import { Utils } from "@bitwarden/common/misc/utils";
+
+import { EncryptionPair } from "./account";
+
+describe("EncryptionPair", () => {
+  describe("toJSON", () => {
+    it("should populate decryptedSerialized for buffer arrays", () => {
+      const pair = new EncryptionPair<string, ArrayBuffer>();
+      pair.decrypted = Utils.fromByteStringToArray("hello").buffer;
+      const json = pair.toJSON();
+      expect(json.decrypted).toEqual("hello");
+    });
+
+    it("should serialize encrypted and decrypted", () => {
+      const pair = new EncryptionPair<string, string>();
+      pair.encrypted = "hello";
+      pair.decrypted = "world";
+      const json = pair.toJSON();
+      expect(json.encrypted).toEqual("hello");
+      expect(json.decrypted).toEqual("world");
+    });
+  });
+
+  describe("fromJSON", () => {
+    it("should deserialize encrypted and decrypted", () => {
+      const pair = EncryptionPair.fromJSON({
+        encrypted: "hello",
+        decrypted: "world",
+      });
+      expect(pair.encrypted).toEqual("hello");
+      expect(pair.decrypted).toEqual("world");
+    });
+  });
+});

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

@@ -1,3 +1,5 @@
+import { Jsonify } from "type-fest";
+
 export class EnvironmentUrls {
   base: string = null;
   api: string = null;
@@ -7,4 +9,8 @@ export class EnvironmentUrls {
   events: string = null;
   webVault: string = null;
   keyConnector: string = null;
+
+  static fromJSON(obj: Jsonify<EnvironmentUrls>): EnvironmentUrls {
+    return Object.assign(new EnvironmentUrls(), obj);
+  }
 }

libs/common/src/models/domain/state.spec.ts

@@ -0,0 +1,28 @@
+import { Account } from "./account";
+import { State } from "./state";
+
+describe("state", () => {
+  describe("fromJSON", () => {
+    it("should deserialize to an instance of itself", () => {
+      expect(State.fromJSON({})).toBeInstanceOf(State);
+    });
+
+    it("should always assign an object to accounts", () => {
+      const state = State.fromJSON({});
+      expect(state.accounts).not.toBeNull();
+      expect(state.accounts).toEqual({});
+    });
+
+    it("should build an account map", () => {
+      const accountsSpy = jest.spyOn(Account, "fromJSON");
+      const state = State.fromJSON({
+        accounts: {
+          userId: {},
+        },
+      });
+
+      expect(state.accounts["userId"]).toBeInstanceOf(Account);
+      expect(accountsSpy).toHaveBeenCalled();
+    });
+  });
+});

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

@@ -1,3 +1,5 @@
+import { Jsonify } from "type-fest";
+
 import { Account } from "./account";
 import { GlobalState } from "./globalState";
 
@@ -14,4 +16,30 @@ export class State<
   constructor(globals: TGlobalState) {
     this.globals = globals;
   }
+
+  // TODO, make Jsonify<State,TGlobalState,TAccount> work. It currently doesn't because Globals doesn't implement Jsonify.
+  static fromJSON<TGlobalState extends GlobalState, TAccount extends Account>(
+    obj: any
+  ): State<TGlobalState, TAccount> {
+    if (obj == null) {
+      return null;
+    }
+
+    return Object.assign(new State(null), obj, {
+      accounts: State.buildAccountMapFromJSON(obj?.accounts),
+    });
+  }
+
+  private static buildAccountMapFromJSON(
+    jsonAccounts: Jsonify<{ [userId: string]: Jsonify<Account> }>
+  ) {
+    if (!jsonAccounts) {
+      return {};
+    }
+    const accounts: { [userId: string]: Account } = {};
+    for (const userId in jsonAccounts) {
+      accounts[userId] = Account.fromJSON(jsonAccounts[userId]);
+    }
+    return accounts;
+  }
 }

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

@@ -1,3 +1,5 @@
+import { Jsonify } from "type-fest";
+
 import { HtmlStorageLocation } from "../../enums/htmlStorageLocation";
 import { StorageLocation } from "../../enums/storageLocation";
 
@@ -8,3 +10,5 @@ export type StorageOptions = {
   htmlStorageLocation?: HtmlStorageLocation;
   keySuffix?: string;
 };
+
+export type MemoryStorageOptions<T> = StorageOptions & { deserializer?: (obj: Jsonify<T>) => T };