JSON stringify memory items (#7731)

* JSON stringify memory items

stringification is required so they can be reliably sent through messaging

* Simplify null handling

apps/browser/src/background/main.background.ts

@@ -51,6 +51,7 @@ import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwar
 import {
   AbstractMemoryStorageService,
   AbstractStorageService,
+  ObservableStorageService,
 } from "@bitwarden/common/platform/abstractions/storage.service";
 import { SystemService as SystemServiceAbstraction } from "@bitwarden/common/platform/abstractions/system.service";
 import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
@@ -62,6 +63,7 @@ import { ContainerService } from "@bitwarden/common/platform/services/container.
 import { EncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/encrypt.service.implementation";
 import { MultithreadEncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/multithread-encrypt.service.implementation";
 import { FileUploadService } from "@bitwarden/common/platform/services/file-upload/file-upload.service";
+import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service";
 import { SystemService } from "@bitwarden/common/platform/services/system.service";
 import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service";
 import {
@@ -188,6 +190,7 @@ export default class MainBackground {
   storageService: AbstractStorageService;
   secureStorageService: AbstractStorageService;
   memoryStorageService: AbstractMemoryStorageService;
+  memoryStorageForStateProviders: AbstractMemoryStorageService & ObservableStorageService;
   i18nService: I18nServiceAbstraction;
   platformUtilsService: PlatformUtilsServiceAbstraction;
   logService: LogServiceAbstraction;
@@ -309,6 +312,13 @@ export default class MainBackground {
     this.storageService = new BrowserLocalStorageService();
     this.secureStorageService = new BrowserLocalStorageService();
     this.memoryStorageService =
+      BrowserApi.manifestVersion === 3
+        ? new LocalBackedSessionStorageService(
+            new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false),
+            new KeyGenerationService(this.cryptoFunctionService),
+          )
+        : new MemoryStorageService();
+    this.memoryStorageForStateProviders =
       BrowserApi.manifestVersion === 3
         ? new LocalBackedSessionStorageService(
             new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false),
@@ -316,7 +326,7 @@ export default class MainBackground {
           )
         : new BackgroundMemoryStorageService();
     this.globalStateProvider = new DefaultGlobalStateProvider(
-      this.memoryStorageService as BackgroundMemoryStorageService,
+      this.memoryStorageForStateProviders,
       this.storageService as BrowserLocalStorageService,
     );
     this.encryptService = flagEnabled("multithreadDecryption")
@@ -328,7 +338,7 @@ export default class MainBackground {
       : new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, true);
 
     this.singleUserStateProvider = new DefaultSingleUserStateProvider(
-      this.memoryStorageService as BackgroundMemoryStorageService,
+      this.memoryStorageForStateProviders,
       this.storageService as BrowserLocalStorageService,
     );
     this.accountService = new AccountServiceImplementation(
@@ -338,11 +348,11 @@ export default class MainBackground {
     );
     this.activeUserStateProvider = new DefaultActiveUserStateProvider(
       this.accountService,
-      this.memoryStorageService as BackgroundMemoryStorageService,
+      this.memoryStorageForStateProviders,
       this.storageService as BrowserLocalStorageService,
     );
     this.derivedStateProvider = new BackgroundDerivedStateProvider(
-      this.memoryStorageService as BackgroundMemoryStorageService,
+      this.memoryStorageForStateProviders,
     );
     this.stateProvider = new DefaultStateProvider(
       this.activeUserStateProvider,

apps/browser/src/platform/storage/background-memory-storage.service.ts

@@ -1,4 +1,5 @@
-import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service";
+// eslint-disable-next-line import/no-restricted-paths -- Implementation for memory storage specifically for browser backgrounds
+import { MemoryStorageService } from "@bitwarden/common/platform/state/storage/memory-storage.service";
 
 import { BrowserApi } from "../browser/browser-api";
 
@@ -27,7 +28,7 @@ export class BackgroundMemoryStorageService extends MemoryStorageService {
       // Initialize the new memory storage service with existing data
       this.sendMessageTo(port, {
         action: "initialization",
-        data: Array.from(this.store.keys()),
+        data: Array.from(Object.keys(this.store)),
       });
     });
     this.updates$.subscribe((update) => {

apps/cli/src/bw.ts

@@ -60,6 +60,7 @@ import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/im
 import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider";
 import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider";
 import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider";
+import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service";
 /* eslint-enable import/no-restricted-paths */
 import { AuditService } from "@bitwarden/common/services/audit.service";
 import { EventCollectionService } from "@bitwarden/common/services/event/event-collection.service";
@@ -125,6 +126,7 @@ export class Main {
   storageService: LowdbStorageService;
   secureStorageService: NodeEnvSecureStorageService;
   memoryStorageService: MemoryStorageService;
+  memoryStorageForStateProviders: MemoryStorageServiceForStateProviders;
   i18nService: I18nService;
   platformUtilsService: CliPlatformUtilsService;
   cryptoService: CryptoService;
@@ -227,14 +229,15 @@ export class Main {
     );
 
     this.memoryStorageService = new MemoryStorageService();
+    this.memoryStorageForStateProviders = new MemoryStorageServiceForStateProviders();
 
     this.globalStateProvider = new DefaultGlobalStateProvider(
-      this.memoryStorageService,
+      this.memoryStorageForStateProviders,
       this.storageService,
     );
 
     this.singleUserStateProvider = new DefaultSingleUserStateProvider(
-      this.memoryStorageService,
+      this.memoryStorageForStateProviders,
       this.storageService,
     );
 
@@ -248,11 +251,13 @@ export class Main {
 
     this.activeUserStateProvider = new DefaultActiveUserStateProvider(
       this.accountService,
-      this.memoryStorageService,
+      this.memoryStorageForStateProviders,
       this.storageService,
     );
 
-    this.derivedStateProvider = new DefaultDerivedStateProvider(this.memoryStorageService);
+    this.derivedStateProvider = new DefaultDerivedStateProvider(
+      this.memoryStorageForStateProviders,
+    );
 
     this.stateProvider = new DefaultStateProvider(
       this.activeUserStateProvider,

apps/desktop/src/app/services/services.module.ts

@@ -39,6 +39,8 @@ import { GlobalState } from "@bitwarden/common/platform/models/domain/global-sta
 import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service";
 import { SystemService } from "@bitwarden/common/platform/services/system.service";
 import { StateProvider } from "@bitwarden/common/platform/state";
+// eslint-disable-next-line import/no-restricted-paths -- Implementation for memory storage
+import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service";
 import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password";
 import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { DialogService } from "@bitwarden/components";
@@ -107,7 +109,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK");
     { provide: AbstractStorageService, useClass: ElectronRendererStorageService },
     { provide: SECURE_STORAGE, useClass: ElectronRendererSecureStorageService },
     { provide: MEMORY_STORAGE, useClass: MemoryStorageService },
-    { provide: OBSERVABLE_MEMORY_STORAGE, useExisting: MEMORY_STORAGE },
+    { provide: OBSERVABLE_MEMORY_STORAGE, useClass: MemoryStorageServiceForStateProviders },
     { provide: OBSERVABLE_DISK_STORAGE, useExisting: AbstractStorageService },
     {
       provide: SystemServiceAbstraction,

apps/desktop/src/main.ts

@@ -14,6 +14,7 @@ import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/im
 import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider";
 import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider";
 import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider";
+import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service";
 /*/ eslint-enable import/no-restricted-paths */
 
 import { MenuMain } from "./main/menu/menu.main";
@@ -38,6 +39,7 @@ export class Main {
   i18nService: I18nMainService;
   storageService: ElectronStorageService;
   memoryStorageService: MemoryStorageService;
+  memoryStorageForStateProviders: MemoryStorageServiceForStateProviders;
   messagingService: ElectronMainMessagingService;
   stateService: ElectronStateService;
   environmentService: EnvironmentService;
@@ -95,8 +97,9 @@ export class Main {
     storageDefaults["global.vaultTimeoutAction"] = "lock";
     this.storageService = new ElectronStorageService(app.getPath("userData"), storageDefaults);
     this.memoryStorageService = new MemoryStorageService();
+    this.memoryStorageForStateProviders = new MemoryStorageServiceForStateProviders();
     const globalStateProvider = new DefaultGlobalStateProvider(
-      this.memoryStorageService,
+      this.memoryStorageForStateProviders,
       this.storageService,
     );
 
@@ -109,12 +112,12 @@ export class Main {
     const stateProvider = new DefaultStateProvider(
       new DefaultActiveUserStateProvider(
         accountService,
-        this.memoryStorageService,
+        this.memoryStorageForStateProviders,
         this.storageService,
       ),
-      new DefaultSingleUserStateProvider(this.memoryStorageService, this.storageService),
+      new DefaultSingleUserStateProvider(this.memoryStorageForStateProviders, this.storageService),
       globalStateProvider,
-      new DefaultDerivedStateProvider(this.memoryStorageService),
+      new DefaultDerivedStateProvider(this.memoryStorageForStateProviders),
     );
 
     this.environmentService = new EnvironmentService(stateProvider, accountService);

apps/web/src/app/core/core.module.ts

@@ -30,6 +30,8 @@ import {
   GlobalStateProvider,
   SingleUserStateProvider,
 } from "@bitwarden/common/platform/state";
+// eslint-disable-next-line import/no-restricted-paths -- Implementation for memory storage
+import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service";
 
 import { PolicyListService } from "../admin-console/core/policy-list.service";
 import { HtmlStorageService } from "../core/html-storage.service";
@@ -87,7 +89,7 @@ import { WebPlatformUtilsService } from "./web-platform-utils.service";
       provide: MEMORY_STORAGE,
       useClass: MemoryStorageService,
     },
-    { provide: OBSERVABLE_MEMORY_STORAGE, useExisting: MEMORY_STORAGE },
+    { provide: OBSERVABLE_MEMORY_STORAGE, useClass: MemoryStorageServiceForStateProviders },
     {
       provide: OBSERVABLE_DISK_STORAGE,
       useFactory: () => new WindowStorageService(window.sessionStorage),

libs/common/spec/fake-storage.service.ts

@@ -3,11 +3,12 @@ import { Subject } from "rxjs";
 
 import {
   AbstractStorageService,
+  ObservableStorageService,
   StorageUpdate,
 } from "../src/platform/abstractions/storage.service";
 import { StorageOptions } from "../src/platform/models/domain/storage-options";
 
-export class FakeStorageService implements AbstractStorageService {
+export class FakeStorageService implements AbstractStorageService, ObservableStorageService {
   private store: Record<string, unknown>;
   private updatesSubject = new Subject<StorageUpdate>();
   private _valuesRequireDeserialization = false;

libs/common/src/platform/state/storage/memory-storage.service.spec.ts

@@ -0,0 +1,53 @@
+import { MemoryStorageService } from "./memory-storage.service";
+
+describe("MemoryStorageService", () => {
+  let sut: MemoryStorageService;
+  const key = "key";
+  const value = { test: "value" };
+
+  beforeEach(() => {
+    sut = new MemoryStorageService();
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  describe("get", () => {
+    it("should return null if the key does not exist", async () => {
+      const result = await sut.get(key);
+      expect(result).toBeNull();
+    });
+
+    it("should return the value if the key exists", async () => {
+      await sut.save(key, value);
+      const result = await sut.get(key);
+      expect(result).toEqual(value);
+    });
+
+    it("should json parse stored values", async () => {
+      sut["store"][key] = JSON.stringify({ test: "value" });
+      const result = await sut.get(key);
+
+      expect(result).toEqual({ test: "value" });
+    });
+  });
+
+  describe("save", () => {
+    it("should store the value as json string", async () => {
+      const value = { test: "value" };
+      await sut.save(key, value);
+
+      expect(sut["store"][key]).toEqual(JSON.stringify(value));
+    });
+  });
+
+  describe("remove", () => {
+    it("should remove a value from store", async () => {
+      await sut.save(key, value);
+      await sut.remove(key);
+
+      expect(Object.keys(sut["store"])).not.toContain(key);
+    });
+  });
+});

libs/common/src/platform/state/storage/memory-storage.service.ts

@@ -0,0 +1,56 @@
+import { Subject } from "rxjs";
+
+import {
+  AbstractMemoryStorageService,
+  ObservableStorageService,
+  StorageUpdate,
+} from "../../abstractions/storage.service";
+
+export class MemoryStorageService
+  extends AbstractMemoryStorageService
+  implements ObservableStorageService
+{
+  protected store: Record<string, string> = {};
+  private updatesSubject = new Subject<StorageUpdate>();
+
+  get valuesRequireDeserialization(): boolean {
+    return true;
+  }
+  get updates$() {
+    return this.updatesSubject.asObservable();
+  }
+
+  get<T>(key: string): Promise<T> {
+    const json = this.store[key];
+    if (json) {
+      const obj = JSON.parse(json as string);
+      return Promise.resolve(obj as T);
+    }
+    return Promise.resolve(null);
+  }
+
+  async has(key: string): Promise<boolean> {
+    return (await this.get(key)) != null;
+  }
+
+  save<T>(key: string, obj: T): Promise<void> {
+    if (obj == null) {
+      return this.remove(key);
+    }
+    // TODO: Remove once foreground/background contexts are separated in browser
+    // Needed to ensure ownership of all memory by the context running the storage service
+    this.store[key] = JSON.stringify(obj);
+    this.updatesSubject.next({ key, updateType: "save" });
+    return Promise.resolve();
+  }
+
+  remove(key: string): Promise<void> {
+    delete this.store[key];
+    this.updatesSubject.next({ key, updateType: "remove" });
+    return Promise.resolve();
+  }
+
+  getBypassCache<T>(key: string): Promise<T> {
+    return this.get<T>(key);
+  }
+}