[SM-1065] SmOnboarding state provider (#8037)

* Create state definition

* Create SmOnboardingTaskService

* Replace usage of stateService value with state new state provider

* Migrate old state values to state provider

* Fix injection of SmOnboardingTasksService

* Remove smOnboardingTasks from state

* Fix state provider imports

* Fix migration after merge from main

* Move null handling to SMOnboardingTasksService

bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts

@@ -11,12 +11,12 @@ import {
   distinctUntilChanged,
   take,
   share,
+  firstValueFrom,
 } from "rxjs";
 
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { DialogService } from "@bitwarden/components";
 
 import { ProjectListView } from "../models/view/project-list.view";
@@ -47,6 +47,8 @@ import {
 import { ServiceAccountService } from "../service-accounts/service-account.service";
 import { SecretsListComponent } from "../shared/secrets-list.component";
 
+import { SMOnboardingTasks, SMOnboardingTasksService } from "./sm-onboarding-tasks.service";
+
 type Tasks = {
   [organizationId: string]: OrganizationTasks;
 };
@@ -71,6 +73,7 @@ export class OverviewComponent implements OnInit, OnDestroy {
   protected showOnboarding = false;
   protected loading = true;
   protected organizationEnabled = false;
+  protected onboardingTasks$: Observable<SMOnboardingTasks>;
 
   protected view$: Observable<{
     allProjects: ProjectListView[];
@@ -87,12 +90,14 @@ export class OverviewComponent implements OnInit, OnDestroy {
     private serviceAccountService: ServiceAccountService,
     private dialogService: DialogService,
     private organizationService: OrganizationService,
-    private stateService: StateService,
     private platformUtilsService: PlatformUtilsService,
     private i18nService: I18nService,
+    private smOnboardingTasksService: SMOnboardingTasksService,
   ) {}
 
   ngOnInit() {
+    this.onboardingTasks$ = this.smOnboardingTasksService.smOnboardingTasks$;
+
     const orgId$ = this.route.params.pipe(
       map((p) => p.organizationId),
       distinctUntilChanged(),
@@ -184,7 +189,7 @@ export class OverviewComponent implements OnInit, OnDestroy {
     organizationId: string,
     orgTasks: OrganizationTasks,
   ): Promise<OrganizationTasks> {
-    const prevTasks = ((await this.stateService.getSMOnboardingTasks()) || {}) as Tasks;
+    const prevTasks = (await firstValueFrom(this.onboardingTasks$)) as Tasks;
     const newlyCompletedOrgTasks = Object.fromEntries(
       Object.entries(orgTasks).filter(([_k, v]) => v === true),
     );
@@ -196,12 +201,12 @@ export class OverviewComponent implements OnInit, OnDestroy {
       ...prevTasks[organizationId],
       ...newlyCompletedOrgTasks,
     };
-    // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+    await this.smOnboardingTasksService.setSmOnboardingTasks({
-    this.stateService.setSMOnboardingTasks({
       ...prevTasks,
       [organizationId]: nextOrgTasks,
     });
+
     return nextOrgTasks as OrganizationTasks;
   }
 

bitwarden_license/bit-web/src/app/secrets-manager/overview/sm-onboarding-tasks.service.ts

@@ -0,0 +1,34 @@
+import { Injectable } from "@angular/core";
+import { Observable, map } from "rxjs";
+
+import {
+  ActiveUserState,
+  KeyDefinition,
+  SM_ONBOARDING_DISK,
+  StateProvider,
+} from "@bitwarden/common/platform/state";
+
+export type SMOnboardingTasks = Record<string, Record<string, boolean>>;
+
+const SM_ONBOARDING_TASKS_KEY = new KeyDefinition<SMOnboardingTasks>(SM_ONBOARDING_DISK, "tasks", {
+  deserializer: (b) => b,
+});
+
+@Injectable({
+  providedIn: "root",
+})
+export class SMOnboardingTasksService {
+  private smOnboardingTasks: ActiveUserState<SMOnboardingTasks>;
+  smOnboardingTasks$: Observable<SMOnboardingTasks>;
+
+  constructor(private stateProvider: StateProvider) {
+    this.smOnboardingTasks = this.stateProvider.getActive(SM_ONBOARDING_TASKS_KEY);
+    this.smOnboardingTasks$ = this.smOnboardingTasks.state$.pipe(map((tasks) => tasks ?? {}));
+  }
+
+  async setSmOnboardingTasks(newState: SMOnboardingTasks): Promise<void> {
+    await this.smOnboardingTasks.update(() => {
+      return { ...newState };
+    });
+  }
+}

libs/common/src/platform/abstractions/state.service.ts

@@ -424,13 +424,6 @@ export abstract class StateService<T extends Account = Account> {
 
   getAvatarColor: (options?: StorageOptions) => Promise<string | null | undefined>;
   setAvatarColor: (value: string, options?: StorageOptions) => Promise<void>;
-  getSMOnboardingTasks: (
-    options?: StorageOptions,
-  ) => Promise<Record<string, Record<string, boolean>>>;
-  setSMOnboardingTasks: (
-    value: Record<string, Record<string, boolean>>,
-    options?: StorageOptions,
-  ) => Promise<void>;
   /**
    * fetches string value of URL user tried to navigate to while unauthenticated.
    * @param options Defines the storage options for the URL; Defaults to session Storage.

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

@@ -223,7 +223,6 @@ export class AccountSettings {
   serverConfig?: ServerConfigData;
   approveLoginRequests?: boolean;
   avatarColor?: string;
-  smOnboardingTasks?: Record<string, Record<string, boolean>>;
   trustDeviceChoiceForDecryption?: boolean;
 
   /** @deprecated July 2023, left for migration purposes*/

libs/common/src/platform/services/state.service.ts

@@ -2154,28 +2154,6 @@ export class StateService<
     );
   }
 
-  async getSMOnboardingTasks(
-    options?: StorageOptions,
-  ): Promise<Record<string, Record<string, boolean>>> {
-    return (
-      await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()))
-    )?.settings?.smOnboardingTasks;
-  }
-
-  async setSMOnboardingTasks(
-    value: Record<string, Record<string, boolean>>,
-    options?: StorageOptions,
-  ): Promise<void> {
-    const account = await this.getAccount(
-      this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()),
-    );
-    account.settings.smOnboardingTasks = value;
-    return await this.saveAccount(
-      account,
-      this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()),
-    );
-  }
-
   async getDeepLinkRedirectUrl(options?: StorageOptions): Promise<string> {
     return (
       await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))

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

@@ -66,3 +66,8 @@ export const VAULT_FILTER_DISK = new StateDefinition("vaultFilter", "disk", {
 export const NEW_WEB_LAYOUT_BANNER_DISK = new StateDefinition("newWebLayoutBanner", "disk", {
   web: "disk-local",
 });
+
+// Secrets Manager
+export const SM_ONBOARDING_DISK = new StateDefinition("smOnboarding", "disk", {
+  web: "disk-local",
+});

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

@@ -18,6 +18,7 @@ import { PrivateKeyMigrator } from "./migrations/20-move-private-key-to-state-pr
 import { CollectionMigrator } from "./migrations/21-move-collections-state-to-state-provider";
 import { CollapsedGroupingsMigrator } from "./migrations/22-move-collapsed-groupings-to-state-provider";
 import { MoveBiometricPromptsToStateProviders } from "./migrations/23-move-biometric-prompts-to-state-providers";
+import { SmOnboardingTasksMigrator } from "./migrations/24-move-sm-onboarding-key-to-state-providers";
 import { FixPremiumMigrator } from "./migrations/3-fix-premium";
 import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked";
 import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys";
@@ -28,7 +29,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting
 import { MinVersionMigrator } from "./migrations/min-version";
 
 export const MIN_VERSION = 2;
-export const CURRENT_VERSION = 23;
+export const CURRENT_VERSION = 24;
 export type MinVersion = typeof MIN_VERSION;
 
 export function createMigrationBuilder() {
@@ -54,7 +55,8 @@ export function createMigrationBuilder() {
     .with(PrivateKeyMigrator, 19, 20)
     .with(CollectionMigrator, 20, 21)
     .with(CollapsedGroupingsMigrator, 21, 22)
-    .with(MoveBiometricPromptsToStateProviders, 22, CURRENT_VERSION);
+    .with(MoveBiometricPromptsToStateProviders, 22, 23)
+    .with(SmOnboardingTasksMigrator, 23, CURRENT_VERSION);
 }
 
 export async function currentVersion(

libs/common/src/state-migrations/migrations/24-move-sm-onboarding-key-to-state-providers.spec.ts

@@ -0,0 +1,200 @@
+import { MockProxy, any } from "jest-mock-extended";
+
+import { MigrationHelper } from "../migration-helper";
+import { mockMigrationHelper } from "../migration-helper.spec";
+
+import { SmOnboardingTasksMigrator } from "./24-move-sm-onboarding-key-to-state-providers";
+
+function exampleJSON() {
+  return {
+    authenticatedAccounts: ["user-1", "user-2", "user-3"],
+    "user-1": {
+      settings: {
+        smOnboardingTasks: {
+          "0bd005de-c722-473b-a00c-b10101006fcd": {
+            createProject: true,
+            createSecret: true,
+            createServiceAccount: true,
+            importSecrets: true,
+          },
+          "2f0d26ec-493a-4ed7-9183-b10d013597c8": {
+            createProject: false,
+            createSecret: true,
+            createServiceAccount: false,
+            importSecrets: true,
+          },
+        },
+        someOtherProperty: "Some other value",
+      },
+      otherStuff: "otherStuff",
+    },
+    "user-2": {
+      settings: {
+        smOnboardingTasks: {
+          "000000-0000000-0000000-000000000": {
+            createProject: false,
+            createSecret: false,
+            createServiceAccount: false,
+            importSecrets: false,
+          },
+        },
+        someOtherProperty: "Some other value",
+      },
+      otherStuff: "otherStuff",
+    },
+  };
+}
+
+function rollbackJSON() {
+  return {
+    "user_user-1_smOnboarding_tasks": {
+      "0bd005de-c722-473b-a00c-b10101006fcd": {
+        createProject: true,
+        createSecret: true,
+        createServiceAccount: true,
+        importSecrets: true,
+      },
+      "2f0d26ec-493a-4ed7-9183-b10d013597c8": {
+        createProject: false,
+        createSecret: true,
+        createServiceAccount: false,
+        importSecrets: true,
+      },
+    },
+    "user_user-2_smOnboarding_tasks": {
+      "000000-0000000-0000000-000000000": {
+        createProject: false,
+        createSecret: false,
+        createServiceAccount: false,
+        importSecrets: false,
+      },
+    },
+    authenticatedAccounts: ["user-1", "user-2"],
+    "user-1": {
+      settings: {
+        someOtherProperty: "Some other value",
+      },
+      otherStuff: "otherStuff",
+    },
+    "user-2": {
+      settings: {
+        someOtherProperty: "Some other value",
+      },
+      otherStuff: "otherStuff",
+    },
+  };
+}
+
+describe("SmOnboardingTasksMigrator", () => {
+  let helper: MockProxy<MigrationHelper>;
+  let sut: SmOnboardingTasksMigrator;
+
+  const keyDefinitionLike = {
+    key: "tasks",
+    stateDefinition: { name: "smOnboarding" },
+  };
+
+  describe("migrate", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(exampleJSON(), 23);
+      sut = new SmOnboardingTasksMigrator(23, 24);
+    });
+
+    it("should remove smOnboardingTasks from all accounts", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).toHaveBeenCalledWith("user-1", {
+        settings: {
+          someOtherProperty: "Some other value",
+        },
+        otherStuff: "otherStuff",
+      });
+    });
+
+    it("should set smOnboardingTasks provider value for each account", async () => {
+      await sut.migrate(helper);
+
+      expect(helper.setToUser).toHaveBeenCalledWith("user-1", keyDefinitionLike, {
+        "0bd005de-c722-473b-a00c-b10101006fcd": {
+          createProject: true,
+          createSecret: true,
+          createServiceAccount: true,
+          importSecrets: true,
+        },
+        "2f0d26ec-493a-4ed7-9183-b10d013597c8": {
+          createProject: false,
+          createSecret: true,
+          createServiceAccount: false,
+          importSecrets: true,
+        },
+      });
+
+      expect(helper.setToUser).toHaveBeenCalledWith("user-2", keyDefinitionLike, {
+        "000000-0000000-0000000-000000000": {
+          createProject: false,
+          createSecret: false,
+          createServiceAccount: false,
+          importSecrets: false,
+        },
+      });
+    });
+  });
+
+  describe("rollback", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(rollbackJSON(), 23);
+      sut = new SmOnboardingTasksMigrator(23, 24);
+    });
+
+    it.each(["user-1", "user-2"])("should null out new values", async (userId) => {
+      await sut.rollback(helper);
+
+      expect(helper.setToUser).toHaveBeenCalledWith(userId, keyDefinitionLike, null);
+    });
+
+    it("should add smOnboardingTasks back to accounts", async () => {
+      await sut.rollback(helper);
+
+      expect(helper.set).toHaveBeenCalledWith("user-1", {
+        settings: {
+          smOnboardingTasks: {
+            "0bd005de-c722-473b-a00c-b10101006fcd": {
+              createProject: true,
+              createSecret: true,
+              createServiceAccount: true,
+              importSecrets: true,
+            },
+            "2f0d26ec-493a-4ed7-9183-b10d013597c8": {
+              createProject: false,
+              createSecret: true,
+              createServiceAccount: false,
+              importSecrets: true,
+            },
+          },
+          someOtherProperty: "Some other value",
+        },
+        otherStuff: "otherStuff",
+      });
+
+      expect(helper.set).toHaveBeenCalledWith("user-2", {
+        settings: {
+          smOnboardingTasks: {
+            "000000-0000000-0000000-000000000": {
+              createProject: false,
+              createSecret: false,
+              createServiceAccount: false,
+              importSecrets: false,
+            },
+          },
+          someOtherProperty: "Some other value",
+        },
+        otherStuff: "otherStuff",
+      });
+    });
+
+    it("should not try to restore values to missing accounts", async () => {
+      await sut.rollback(helper);
+
+      expect(helper.set).not.toHaveBeenCalledWith("user-3", any());
+    });
+  });
+});

libs/common/src/state-migrations/migrations/24-move-sm-onboarding-key-to-state-providers.ts

@@ -0,0 +1,53 @@
+import { KeyDefinitionLike, MigrationHelper } from "../migration-helper";
+import { Migrator } from "../migrator";
+
+type ExpectedAccountType = {
+  settings?: {
+    smOnboardingTasks?: Record<string, Record<string, boolean>>;
+  };
+};
+
+export const SM_ONBOARDING_TASKS: KeyDefinitionLike = {
+  key: "tasks",
+  stateDefinition: { name: "smOnboarding" },
+};
+
+export class SmOnboardingTasksMigrator extends Migrator<23, 24> {
+  async migrate(helper: MigrationHelper): Promise<void> {
+    const legacyAccounts = await helper.getAccounts<ExpectedAccountType>();
+
+    await Promise.all(
+      legacyAccounts.map(async ({ userId, account }) => {
+        // Move account data
+        if (account?.settings?.smOnboardingTasks != null) {
+          await helper.setToUser(userId, SM_ONBOARDING_TASKS, account.settings.smOnboardingTasks);
+
+          // Delete old account data
+          delete account.settings.smOnboardingTasks;
+          await helper.set(userId, account);
+        }
+      }),
+    );
+  }
+
+  async rollback(helper: MigrationHelper): Promise<void> {
+    async function rollbackUser(userId: string, account: ExpectedAccountType) {
+      const smOnboardingTasks = await helper.getFromUser<Record<string, Record<string, boolean>>>(
+        userId,
+        SM_ONBOARDING_TASKS,
+      );
+      if (smOnboardingTasks) {
+        account ??= {};
+        account.settings ??= {};
+
+        account.settings.smOnboardingTasks = smOnboardingTasks;
+        await helper.setToUser(userId, SM_ONBOARDING_TASKS, null);
+        await helper.set(userId, account);
+      }
+    }
+
+    const accounts = await helper.getAccounts<ExpectedAccountType>();
+
+    await Promise.all(accounts.map(({ userId, account }) => rollbackUser(userId, account)));
+  }
+}