[PM-6654] new app id service for angular (#8229)

* Improve state documentation

* Add namespace for application id

* Spec out behavior of app id service

* Use state providers for app ids

* Migrate app Id

* Add reactive interface

apps/browser/src/popup/services/services.module.ts

@@ -44,7 +44,6 @@ import {
   UserNotificationSettingsService,
   UserNotificationSettingsServiceAbstraction,
 } from "@bitwarden/common/autofill/services/user-notification-settings.service";
-import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
 import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction";
 import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
 import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
@@ -349,7 +348,6 @@ function getBgService<T>(service: keyof MainBackground) {
       useClass: BrowserLocalStorageService,
       deps: [],
     },
-    { provide: AppIdService, useFactory: getBgService<AppIdService>("appIdService"), deps: [] },
     {
       provide: AutofillService,
       useFactory: getBgService<AutofillService>("autofillService"),

libs/common/src/platform/abstractions/app-id.service.ts

@@ -1,4 +1,8 @@
+import { Observable } from "rxjs";
+
 export abstract class AppIdService {
+  appId$: Observable<string>;
+  anonymousAppId$: Observable<string>;
   getAppId: () => Promise<string>;
   getAnonymousAppId: () => Promise<string>;
 }

libs/common/src/platform/misc/utils.ts

@@ -253,11 +253,10 @@ export class Utils {
     });
   }
 
+  static guidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
+
   static isGuid(id: string) {
-    return RegExp(
-      /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/,
-      "i",
-    ).test(id);
+    return RegExp(Utils.guidRegex, "i").test(id);
   }
 
   static getHostname(uriString: string): string {

libs/common/src/platform/services/app-id.service.spec.ts

@@ -0,0 +1,101 @@
+import { FakeGlobalStateProvider } from "../../../spec";
+import { Utils } from "../misc/utils";
+
+import { ANONYMOUS_APP_ID_KEY, APP_ID_KEY, AppIdService } from "./app-id.service";
+
+describe("AppIdService", () => {
+  const globalStateProvider = new FakeGlobalStateProvider();
+  const appIdState = globalStateProvider.getFake(APP_ID_KEY);
+  const anonymousAppIdState = globalStateProvider.getFake(ANONYMOUS_APP_ID_KEY);
+  let sut: AppIdService;
+
+  beforeEach(() => {
+    sut = new AppIdService(globalStateProvider);
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  describe("getAppId", () => {
+    it("returns the existing appId when it exists", async () => {
+      appIdState.stateSubject.next("existingAppId");
+
+      const appId = await sut.getAppId();
+
+      expect(appId).toBe("existingAppId");
+    });
+
+    it.each([null, undefined])(
+      "uses the util function to create a new id when it AppId does not exist",
+      async (value) => {
+        appIdState.stateSubject.next(value);
+        const spy = jest.spyOn(Utils, "newGuid");
+
+        await sut.getAppId();
+
+        expect(spy).toHaveBeenCalledTimes(1);
+      },
+    );
+
+    it.each([null, undefined])("returns a new appId when it does not exist", async (value) => {
+      appIdState.stateSubject.next(value);
+
+      const appId = await sut.getAppId();
+
+      expect(appId).toMatch(Utils.guidRegex);
+    });
+
+    it.each([null, undefined])(
+      "stores the new guid when it an existing one is not found",
+      async (value) => {
+        appIdState.stateSubject.next(value);
+
+        const appId = await sut.getAppId();
+
+        expect(appIdState.nextMock).toHaveBeenCalledWith(appId);
+      },
+    );
+  });
+
+  describe("getAnonymousAppId", () => {
+    it("returns the existing appId when it exists", async () => {
+      anonymousAppIdState.stateSubject.next("existingAppId");
+
+      const appId = await sut.getAnonymousAppId();
+
+      expect(appId).toBe("existingAppId");
+    });
+
+    it.each([null, undefined])(
+      "uses the util function to create a new id when it AppId does not exist",
+      async (value) => {
+        anonymousAppIdState.stateSubject.next(value);
+        const spy = jest.spyOn(Utils, "newGuid");
+
+        await sut.getAnonymousAppId();
+
+        expect(spy).toHaveBeenCalledTimes(1);
+      },
+    );
+
+    it.each([null, undefined])("returns a new appId when it does not exist", async (value) => {
+      anonymousAppIdState.stateSubject.next(value);
+
+      const appId = await sut.getAnonymousAppId();
+
+      expect(appId).toMatch(Utils.guidRegex);
+    });
+
+    it.each([null, undefined])(
+      "stores the new guid when it an existing one is not found",
+      async (value) => {
+        anonymousAppIdState.stateSubject.next(value);
+
+        const appId = await sut.getAnonymousAppId();
+
+        expect(anonymousAppIdState.nextMock).toHaveBeenCalledWith(appId);
+      },
+    );
+  });
+});

libs/common/src/platform/services/app-id.service.ts

@@ -1,31 +1,46 @@
+import { Observable, filter, firstValueFrom, tap } from "rxjs";
+
 import { AppIdService as AppIdServiceAbstraction } from "../abstractions/app-id.service";
-import { AbstractStorageService } from "../abstractions/storage.service";
-import { HtmlStorageLocation } from "../enums";
 import { Utils } from "../misc/utils";
+import { APPLICATION_ID_DISK, GlobalStateProvider, KeyDefinition } from "../state";
+
+export const APP_ID_KEY = new KeyDefinition(APPLICATION_ID_DISK, "appId", {
+  deserializer: (value: string) => value,
+});
+export const ANONYMOUS_APP_ID_KEY = new KeyDefinition(APPLICATION_ID_DISK, "anonymousAppId", {
+  deserializer: (value: string) => value,
+});
 
 export class AppIdService implements AppIdServiceAbstraction {
-  constructor(private storageService: AbstractStorageService) {}
+  appId$: Observable<string>;
+  anonymousAppId$: Observable<string>;
 
-  getAppId(): Promise<string> {
-    return this.makeAndGetAppId("appId");
+  constructor(globalStateProvider: GlobalStateProvider) {
+    const appIdState = globalStateProvider.get(APP_ID_KEY);
+    const anonymousAppIdState = globalStateProvider.get(ANONYMOUS_APP_ID_KEY);
+    this.appId$ = appIdState.state$.pipe(
+      tap(async (appId) => {
+        if (!appId) {
+          await appIdState.update(() => Utils.newGuid());
+        }
+      }),
+      filter((appId) => !!appId),
+    );
+    this.anonymousAppId$ = anonymousAppIdState.state$.pipe(
+      tap(async (appId) => {
+        if (!appId) {
+          await anonymousAppIdState.update(() => Utils.newGuid());
+        }
+      }),
+      filter((appId) => !!appId),
+    );
   }
 
-  getAnonymousAppId(): Promise<string> {
-    return this.makeAndGetAppId("anonymousAppId");
+  async getAppId(): Promise<string> {
+    return await firstValueFrom(this.appId$);
   }
 
-  private async makeAndGetAppId(key: string) {
-    const existingId = await this.storageService.get<string>(key, {
-      htmlStorageLocation: HtmlStorageLocation.Local,
-    });
-    if (existingId != null) {
-      return existingId;
-    }
-
-    const guid = Utils.newGuid();
-    await this.storageService.save(key, guid, {
-      htmlStorageLocation: HtmlStorageLocation.Local,
-    });
-    return guid;
+  async getAnonymousAppId(): Promise<string> {
+    return await firstValueFrom(this.anonymousAppId$);
   }
 }

libs/common/src/platform/state/global-state.ts

@@ -15,6 +15,7 @@ export interface GlobalState<T> {
    * @param options.combineLatestWith An observable that you want to combine with the current state for callbacks. Defaults to null
    * @param options.msTimeout A timeout for how long you are willing to wait for a `combineLatestWith` option to complete. Defaults to 1000ms. Only applies if `combineLatestWith` is set.
    * @returns A promise that must be awaited before your next action to ensure the update has been written to state.
+   * Resolves to the new state. If `shouldUpdate` returns false, the promise will resolve to the current state.
    */
   update: <TCombine>(
     configureState: (state: T, dependency: TCombine) => T,

libs/common/src/platform/state/state-definitions.ts

@@ -52,6 +52,9 @@ export const NEW_WEB_LAYOUT_BANNER_DISK = new StateDefinition("newWebLayoutBanne
 
 // Platform
 
+export const APPLICATION_ID_DISK = new StateDefinition("applicationId", "disk", {
+  web: "disk-local",
+});
 export const BIOMETRIC_SETTINGS_DISK = new StateDefinition("biometricSettings", "disk");
 export const CLEAR_EVENT_DISK = new StateDefinition("clearEvent", "disk");
 export const CRYPTO_DISK = new StateDefinition("crypto", "disk");

libs/common/src/platform/state/user-state.ts

@@ -32,7 +32,8 @@ export interface ActiveUserState<T> extends UserState<T> {
    * @param options.combineLatestWith An observable that you want to combine with the current state for callbacks. Defaults to null
    * @param options.msTimeout A timeout for how long you are willing to wait for a `combineLatestWith` option to complete. Defaults to 1000ms. Only applies if `combineLatestWith` is set.
 
-   * @returns The new state
+   * @returns A promise that must be awaited before your next action to ensure the update has been written to state.
+   * Resolves to the new state. If `shouldUpdate` returns false, the promise will resolve to the current state.
    */
   readonly update: <TCombine>(
     configureState: (state: T, dependencies: TCombine) => T,
@@ -50,7 +51,8 @@ export interface SingleUserState<T> extends UserState<T> {
    * @param options.combineLatestWith An observable that you want to combine with the current state for callbacks. Defaults to null
    * @param options.msTimeout A timeout for how long you are willing to wait for a `combineLatestWith` option to complete. Defaults to 1000ms. Only applies if `combineLatestWith` is set.
 
-   * @returns The new state
+   * @returns A promise that must be awaited before your next action to ensure the update has been written to state.
+   * Resolves to the new state. If `shouldUpdate` returns false, the promise will resolve to the current state.
    */
   readonly update: <TCombine>(
     configureState: (state: T, dependencies: TCombine) => T,

libs/common/src/state-migrations/migrate.ts

@@ -28,6 +28,7 @@ import { FixPremiumMigrator } from "./migrations/3-fix-premium";
 import { PolicyMigrator } from "./migrations/30-move-policy-state-to-state-provider";
 import { EnableContextMenuMigrator } from "./migrations/31-move-enable-context-menu-to-autofill-settings-state-provider";
 import { PreferredLanguageMigrator } from "./migrations/32-move-preferred-language";
+import { AppIdMigrator } from "./migrations/33-move-app-id-to-state-providers";
 import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked";
 import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys";
 import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key";
@@ -37,7 +38,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting
 import { MinVersionMigrator } from "./migrations/min-version";
 
 export const MIN_VERSION = 2;
-export const CURRENT_VERSION = 32;
+export const CURRENT_VERSION = 33;
 export type MinVersion = typeof MIN_VERSION;
 
 export function createMigrationBuilder() {
@@ -72,7 +73,8 @@ export function createMigrationBuilder() {
     .with(UserNotificationSettingsKeyMigrator, 28, 29)
     .with(PolicyMigrator, 29, 30)
     .with(EnableContextMenuMigrator, 30, 31)
-    .with(PreferredLanguageMigrator, 31, CURRENT_VERSION);
+    .with(PreferredLanguageMigrator, 31, 32)
+    .with(AppIdMigrator, 32, CURRENT_VERSION);
 }
 
 export async function currentVersion(

libs/common/src/state-migrations/migrations/33-move-app-id-to-state-providers.spec.ts

@@ -0,0 +1,213 @@
+import { MockProxy, any } from "jest-mock-extended";
+
+import { MigrationHelper } from "../migration-helper";
+import { mockMigrationHelper } from "../migration-helper.spec";
+
+import {
+  ANONYMOUS_APP_ID_KEY,
+  APP_ID_KEY,
+  AppIdMigrator,
+} from "./33-move-app-id-to-state-providers";
+
+function exampleJSON() {
+  return {
+    appId: "appId",
+    anonymousAppId: "anonymousAppId",
+    otherStuff: "otherStuff1",
+  };
+}
+
+function missingAppIdJSON() {
+  return {
+    anonymousAppId: "anonymousAppId",
+    otherStuff: "otherStuff1",
+  };
+}
+
+function missingAnonymousAppIdJSON() {
+  return {
+    appId: "appId",
+    otherStuff: "otherStuff1",
+  };
+}
+
+function missingBothJSON() {
+  return {
+    otherStuff: "otherStuff1",
+  };
+}
+
+function rollbackJSON() {
+  return {
+    global_applicationId_appId: "appId",
+    global_applicationId_anonymousAppId: "anonymousAppId",
+    otherStuff: "otherStuff1",
+  };
+}
+
+describe("AppIdMigrator", () => {
+  let helper: MockProxy<MigrationHelper>;
+  let sut: AppIdMigrator;
+
+  describe("migrate with both ids", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(exampleJSON(), 32);
+      sut = new AppIdMigrator(32, 33);
+    });
+
+    it("removes appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).toHaveBeenCalledWith("appId", null);
+    });
+
+    it("removes anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).toHaveBeenCalledWith("anonymousAppId", null);
+    });
+
+    it("sets appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).toHaveBeenCalledWith(APP_ID_KEY, "appId");
+    });
+
+    it("sets anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).toHaveBeenCalledWith(ANONYMOUS_APP_ID_KEY, "anonymousAppId");
+    });
+  });
+
+  describe("migrate with missing appId", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(missingAppIdJSON(), 32);
+      sut = new AppIdMigrator(32, 33);
+    });
+
+    it("does not set appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).not.toHaveBeenCalledWith(APP_ID_KEY, any());
+    });
+
+    it("removes anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).toHaveBeenCalledWith("anonymousAppId", null);
+    });
+
+    it("does not set appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).not.toHaveBeenCalledWith(APP_ID_KEY, any());
+    });
+
+    it("sets anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).toHaveBeenCalledWith(ANONYMOUS_APP_ID_KEY, "anonymousAppId");
+    });
+  });
+
+  describe("migrate with missing anonymousAppId", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(missingAnonymousAppIdJSON(), 32);
+      sut = new AppIdMigrator(32, 33);
+    });
+
+    it("sets appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).toHaveBeenCalledWith(APP_ID_KEY, "appId");
+    });
+
+    it("does not set anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).not.toHaveBeenCalledWith(ANONYMOUS_APP_ID_KEY, any());
+    });
+
+    it("removes appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).toHaveBeenCalledWith("appId", null);
+    });
+
+    it("does not remove anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).not.toHaveBeenCalledWith("anonymousAppId", any());
+    });
+  });
+
+  describe("migrate with missing appId and anonymousAppId", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(missingBothJSON(), 32);
+      sut = new AppIdMigrator(32, 33);
+    });
+
+    it("does not set appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).not.toHaveBeenCalledWith(APP_ID_KEY, any());
+    });
+
+    it("does not set anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.setToGlobal).not.toHaveBeenCalledWith(ANONYMOUS_APP_ID_KEY, any());
+    });
+
+    it("does not remove appId", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).not.toHaveBeenCalledWith("appId", any());
+    });
+
+    it("does not remove anonymousAppId", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).not.toHaveBeenCalledWith("anonymousAppId", any());
+    });
+  });
+
+  describe("rollback with both Ids", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(rollbackJSON(), 33);
+      sut = new AppIdMigrator(32, 33);
+    });
+
+    it("removes appId", async () => {
+      await sut.rollback(helper);
+      expect(helper.setToGlobal).toHaveBeenCalledWith(APP_ID_KEY, null);
+    });
+
+    it("sets appId", async () => {
+      await sut.rollback(helper);
+      expect(helper.set).toHaveBeenCalledWith("appId", "appId");
+    });
+
+    it("removes anonymousAppId", async () => {
+      await sut.rollback(helper);
+      expect(helper.setToGlobal).toHaveBeenCalledWith(ANONYMOUS_APP_ID_KEY, null);
+    });
+
+    it("sets anonymousAppId", async () => {
+      await sut.rollback(helper);
+      expect(helper.set).toHaveBeenCalledWith("anonymousAppId", "anonymousAppId");
+    });
+  });
+
+  describe("rollback missing both Ids", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(missingBothJSON(), 33);
+      sut = new AppIdMigrator(32, 33);
+    });
+
+    it("does not set appId for providers", async () => {
+      await sut.rollback(helper);
+      expect(helper.setToGlobal).not.toHaveBeenCalledWith(APP_ID_KEY, any());
+    });
+
+    it("does not set anonymousAppId for providers", async () => {
+      await sut.rollback(helper);
+      expect(helper.setToGlobal).not.toHaveBeenCalledWith(ANONYMOUS_APP_ID_KEY, any());
+    });
+
+    it("does not revert appId", async () => {
+      await sut.rollback(helper);
+      expect(helper.set).not.toHaveBeenCalledWith("appId", any());
+    });
+
+    it("does not revert anonymousAppId", async () => {
+      await sut.rollback(helper);
+      expect(helper.set).not.toHaveBeenCalledWith("anonymousAppId", any());
+    });
+  });
+});

libs/common/src/state-migrations/migrations/33-move-app-id-to-state-providers.ts

@@ -0,0 +1,46 @@
+import { KeyDefinitionLike, MigrationHelper } from "../migration-helper";
+import { Migrator } from "../migrator";
+
+export const APP_ID_STORAGE_KEY = "appId";
+export const ANONYMOUS_APP_ID_STORAGE_KEY = "anonymousAppId";
+
+export const APP_ID_KEY: KeyDefinitionLike = {
+  key: APP_ID_STORAGE_KEY,
+  stateDefinition: { name: "applicationId" },
+};
+
+export const ANONYMOUS_APP_ID_KEY: KeyDefinitionLike = {
+  key: ANONYMOUS_APP_ID_STORAGE_KEY,
+  stateDefinition: { name: "applicationId" },
+};
+
+export class AppIdMigrator extends Migrator<32, 33> {
+  async migrate(helper: MigrationHelper): Promise<void> {
+    const appId = await helper.get<string>(APP_ID_STORAGE_KEY);
+    const anonymousAppId = await helper.get<string>(ANONYMOUS_APP_ID_STORAGE_KEY);
+
+    if (appId != null) {
+      await helper.setToGlobal(APP_ID_KEY, appId);
+      await helper.set(APP_ID_STORAGE_KEY, null);
+    }
+
+    if (anonymousAppId != null) {
+      await helper.setToGlobal(ANONYMOUS_APP_ID_KEY, anonymousAppId);
+      await helper.set(ANONYMOUS_APP_ID_STORAGE_KEY, null);
+    }
+  }
+
+  async rollback(helper: MigrationHelper): Promise<void> {
+    const appId = await helper.getFromGlobal<string>(APP_ID_KEY);
+    const anonymousAppId = await helper.getFromGlobal<string>(ANONYMOUS_APP_ID_KEY);
+
+    if (appId != null) {
+      await helper.set(APP_ID_STORAGE_KEY, appId);
+      await helper.setToGlobal(APP_ID_KEY, null);
+    }
+    if (anonymousAppId != null) {
+      await helper.set(ANONYMOUS_APP_ID_STORAGE_KEY, anonymousAppId);
+      await helper.setToGlobal(ANONYMOUS_APP_ID_KEY, null);
+    }
+  }
+}