[PM-5277] Migrate Sync Service to State Provider (#7680)

* [PM-5277] Introduce lastSync state via State Providers

* [PM-5277] Add migrator and tests

* [PM-5277] Use memory for web storage location

* [PM-5277] Remove lastSync methods from state service

* [PM-5277] Remove lastSync from AccountProfile

* [PM-5277] Use string instead of Date to fix serialization for chrome.storage API in Browser

* [PM-5277] Only set account if lastSync was deleted during migration

* [PM-5277] Fix spec file

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

@@ -139,10 +139,10 @@ import {
   VaultExportServiceAbstraction,
 } from "@bitwarden/exporter/vault-export";
 import {
-  ImportApiServiceAbstraction,
   ImportApiService,
-  ImportServiceAbstraction,
+  ImportApiServiceAbstraction,
   ImportService,
+  ImportServiceAbstraction,
 } from "@bitwarden/importer/core";
 
 import { BrowserOrganizationService } from "../admin-console/services/browser-organization.service";
@@ -634,6 +634,7 @@ export default class MainBackground {
       this.folderApiService,
       this.organizationService,
       this.sendApiService,
+      this.stateProvider,
       logoutCallback,
     );
     this.eventUploadService = new EventUploadService(
@@ -1009,7 +1010,7 @@ export default class MainBackground {
     await this.eventUploadService.uploadEvents(userId);
 
     await Promise.all([
-      this.syncService.setLastSync(new Date(0), userId),
+      this.syncService.setLastSync(new Date(0), userId as UserId),
       this.cryptoService.clearKeys(userId),
       this.settingsService.clear(userId),
       this.cipherService.clear(userId),

apps/cli/src/bw.ts

@@ -519,6 +519,7 @@ export class Main {
       this.folderApiService,
       this.organizationService,
       this.sendApiService,
+      this.stateProvider,
       async (expired: boolean) => await this.logout(),
     );
 

apps/desktop/src/app/app.component.ts

@@ -39,6 +39,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { SystemService } from "@bitwarden/common/platform/abstractions/system.service";
 import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password";
+import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
 import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
@@ -565,7 +566,7 @@ export class AppComponent implements OnInit, OnDestroy {
     let preLogoutActiveUserId;
     try {
       await this.eventUploadService.uploadEvents(userBeingLoggedOut);
-      await this.syncService.setLastSync(new Date(0), userBeingLoggedOut);
+      await this.syncService.setLastSync(new Date(0), userBeingLoggedOut as UserId);
       await this.cryptoService.clearKeys(userBeingLoggedOut);
       await this.settingsService.clear(userBeingLoggedOut);
       await this.cipherService.clear(userBeingLoggedOut);

apps/web/src/app/core/state/state.service.ts

@@ -93,14 +93,4 @@ export class StateService extends BaseStateService<GlobalState, Account> {
     options = this.reconcileOptions(options, await this.defaultInMemoryOptions());
     return await super.setEncryptedSends(value, options);
   }
-
-  override async getLastSync(options?: StorageOptions): Promise<string> {
-    options = this.reconcileOptions(options, await this.defaultInMemoryOptions());
-    return await super.getLastSync(options);
-  }
-
-  override async setLastSync(value: string, options?: StorageOptions): Promise<void> {
-    options = this.reconcileOptions(options, await this.defaultInMemoryOptions());
-    return await super.setLastSync(value, options);
-  }
 }

libs/angular/src/services/jslib-services.module.ts

@@ -123,10 +123,10 @@ import { ValidationService } from "@bitwarden/common/platform/services/validatio
 import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service";
 import {
   ActiveUserStateProvider,
+  DerivedStateProvider,
   GlobalStateProvider,
   SingleUserStateProvider,
   StateProvider,
-  DerivedStateProvider,
 } from "@bitwarden/common/platform/state";
 /* eslint-disable import/no-restricted-paths -- We need the implementations to inject, but generally these should not be accessed */
 import { DefaultActiveUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-active-user-state.provider";
@@ -490,6 +490,7 @@ import { ModalService } from "./modal.service";
         FolderApiServiceAbstraction,
         OrganizationServiceAbstraction,
         SendApiServiceAbstraction,
+        StateProvider,
         LOGOUT_CALLBACK,
       ],
     },

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

@@ -16,7 +16,7 @@ import { UsernameGeneratorOptions } from "../../tools/generator/username";
 import { SendData } from "../../tools/send/models/data/send.data";
 import { SendView } from "../../tools/send/models/view/send.view";
 import { UserId } from "../../types/guid";
-import { UserKey, MasterKey, DeviceKey } from "../../types/key";
+import { DeviceKey, MasterKey, UserKey } from "../../types/key";
 import { UriMatchType } from "../../vault/enums";
 import { CipherData } from "../../vault/models/data/cipher.data";
 import { CollectionData } from "../../vault/models/data/collection.data";
@@ -394,8 +394,6 @@ export abstract class StateService<T extends Account = Account> {
   setKeyHash: (value: string, options?: StorageOptions) => Promise<void>;
   getLastActive: (options?: StorageOptions) => Promise<number>;
   setLastActive: (value: number, options?: StorageOptions) => Promise<void>;
-  getLastSync: (options?: StorageOptions) => Promise<string>;
-  setLastSync: (value: string, options?: StorageOptions) => Promise<void>;
   getLocalData: (options?: StorageOptions) => Promise<{ [cipherId: string]: LocalData }>;
   setLocalData: (
     value: { [cipherId: string]: LocalData },

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

@@ -19,7 +19,7 @@ import { UsernameGeneratorOptions } from "../../../tools/generator/username/user
 import { SendData } from "../../../tools/send/models/data/send.data";
 import { SendView } from "../../../tools/send/models/view/send.view";
 import { DeepJsonify } from "../../../types/deep-jsonify";
-import { UserKey, MasterKey } from "../../../types/key";
+import { MasterKey, UserKey } from "../../../types/key";
 import { UriMatchType } from "../../../vault/enums";
 import { CipherData } from "../../../vault/models/data/cipher.data";
 import { CollectionData } from "../../../vault/models/data/collection.data";
@@ -197,7 +197,6 @@ export class AccountProfile {
   forceSetPasswordReason?: ForceSetPasswordReason;
   hasPremiumPersonally?: boolean;
   hasPremiumFromOrganization?: boolean;
-  lastSync?: string;
   userId?: string;
   usesKeyConnector?: boolean;
   keyHash?: string;

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

@@ -23,7 +23,7 @@ import { UsernameGeneratorOptions } from "../../tools/generator/username";
 import { SendData } from "../../tools/send/models/data/send.data";
 import { SendView } from "../../tools/send/models/view/send.view";
 import { UserId } from "../../types/guid";
-import { UserKey, MasterKey, DeviceKey } from "../../types/key";
+import { DeviceKey, MasterKey, UserKey } from "../../types/key";
 import { UriMatchType } from "../../vault/enums";
 import { CipherData } from "../../vault/models/data/cipher.data";
 import { CollectionData } from "../../vault/models/data/collection.data";
@@ -2121,23 +2121,6 @@ export class StateService<
     await this.storageService.save(keys.accountActivity, accountActivity, options);
   }
 
-  async getLastSync(options?: StorageOptions): Promise<string> {
-    return (
-      await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()))
-    )?.profile?.lastSync;
-  }
-
-  async setLastSync(value: string, options?: StorageOptions): Promise<void> {
-    const account = await this.getAccount(
-      this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()),
-    );
-    account.profile.lastSync = value;
-    await this.saveAccount(
-      account,
-      this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()),
-    );
-  }
-
   async getLocalData(options?: StorageOptions): Promise<{ [cipherId: string]: LocalData }> {
     return (
       await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()))

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

@@ -37,3 +37,5 @@ export const POLICIES_MEMORY = new StateDefinition("policies", "memory");
 export const PROVIDERS_DISK = new StateDefinition("providers", "disk");
 
 export const FOLDER_DISK = new StateDefinition("folder", "disk", { web: "memory" });
+
+export const SYNC_STATE = new StateDefinition("sync", "disk", { web: "memory" });

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

@@ -11,6 +11,7 @@ import { MoveEnvironmentStateToProviders } from "./migrations/12-move-environmen
 import { ProviderKeyMigrator } from "./migrations/13-move-provider-keys-to-state-providers";
 import { MoveBiometricClientKeyHalfToStateProviders } from "./migrations/14-move-biometric-client-key-half-state-to-providers";
 import { FolderMigrator } from "./migrations/15-move-folder-state-to-state-provider";
+import { LastSyncMigrator } from "./migrations/16-move-last-sync-to-state-provider";
 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";
@@ -21,7 +22,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting
 import { MinVersionMigrator } from "./migrations/min-version";
 
 export const MIN_VERSION = 2;
-export const CURRENT_VERSION = 15;
+export const CURRENT_VERSION = 16;
 export type MinVersion = typeof MIN_VERSION;
 
 export async function migrate(
@@ -52,7 +53,8 @@ export async function migrate(
     .with(MoveEnvironmentStateToProviders, 11, 12)
     .with(ProviderKeyMigrator, 12, 13)
     .with(MoveBiometricClientKeyHalfToStateProviders, 13, 14)
-    .with(FolderMigrator, 14, CURRENT_VERSION)
+    .with(FolderMigrator, 14, 15)
+    .with(LastSyncMigrator, 15, CURRENT_VERSION)
 
     .migrate(migrationHelper);
 }

libs/common/src/state-migrations/migrations/15-move-folder-state-to-state-provider.spec.ts

@@ -1,4 +1,4 @@
-import { MockProxy, any } from "jest-mock-extended";
+import { any, MockProxy } from "jest-mock-extended";
 
 import { MigrationHelper } from "../migration-helper";
 import { mockMigrationHelper } from "../migration-helper.spec";

libs/common/src/state-migrations/migrations/16-move-last-sync-to-state-provider.spec.ts

@@ -0,0 +1,112 @@
+import { any, MockProxy } from "jest-mock-extended";
+
+import { MigrationHelper } from "../migration-helper";
+import { mockMigrationHelper } from "../migration-helper.spec";
+
+import { LastSyncMigrator } from "./16-move-last-sync-to-state-provider";
+
+function exampleJSON() {
+  return {
+    global: {
+      otherStuff: "otherStuff1",
+    },
+    authenticatedAccounts: ["user-1", "user-2"],
+    "user-1": {
+      profile: {
+        lastSync: "2024-01-24T00:00:00.000Z",
+        otherStuff: "otherStuff4",
+      },
+      otherStuff: "otherStuff5",
+    },
+  };
+}
+
+function rollbackJSON() {
+  return {
+    "user_user-1_sync_lastSync": "2024-01-24T00:00:00.000Z",
+    "user_user-2_sync_lastSync": null as any,
+    global: {
+      otherStuff: "otherStuff1",
+    },
+    authenticatedAccounts: ["user-1", "user-2"],
+    "user-1": {
+      profile: {
+        lastSync: "2024-01-24T00:00:00.000Z",
+        otherStuff: "otherStuff4",
+      },
+      otherStuff: "otherStuff5",
+    },
+  };
+}
+
+describe("LastSyncMigrator", () => {
+  let helper: MockProxy<MigrationHelper>;
+  let sut: LastSyncMigrator;
+
+  const keyDefinitionLike = {
+    key: "lastSync",
+    stateDefinition: {
+      name: "sync",
+    },
+  };
+
+  describe("migrate", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(exampleJSON(), 15);
+      sut = new LastSyncMigrator(15, 16);
+    });
+
+    it("should remove lastSync from all accounts", async () => {
+      await sut.migrate(helper);
+      expect(helper.set).toHaveBeenCalledWith("user-1", {
+        profile: {
+          otherStuff: "otherStuff4",
+        },
+        otherStuff: "otherStuff5",
+      });
+    });
+
+    it("should set lastSync provider value for each account", async () => {
+      await sut.migrate(helper);
+
+      expect(helper.setToUser).toHaveBeenCalledWith(
+        "user-1",
+        keyDefinitionLike,
+        "2024-01-24T00:00:00.000Z",
+      );
+
+      expect(helper.setToUser).toHaveBeenCalledWith("user-2", keyDefinitionLike, null);
+    });
+  });
+
+  describe("rollback", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(rollbackJSON(), 16);
+      sut = new LastSyncMigrator(15, 16);
+    });
+
+    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 lastSync back to accounts", async () => {
+      await sut.rollback(helper);
+
+      expect(helper.set).toHaveBeenCalledWith("user-1", {
+        profile: {
+          lastSync: "2024-01-24T00:00:00.000Z",
+          otherStuff: "otherStuff4",
+        },
+        otherStuff: "otherStuff5",
+      });
+    });
+
+    it("should not try to restore values to missing accounts", async () => {
+      await sut.rollback(helper);
+
+      expect(helper.set).not.toHaveBeenCalledWith("user-2", any());
+    });
+  });
+});

libs/common/src/state-migrations/migrations/16-move-last-sync-to-state-provider.ts

@@ -0,0 +1,47 @@
+import { KeyDefinitionLike, MigrationHelper } from "../migration-helper";
+import { Migrator } from "../migrator";
+
+type ExpectedAccountType = {
+  profile?: {
+    lastSync?: string;
+  };
+};
+
+const LAST_SYNC_KEY: KeyDefinitionLike = {
+  key: "lastSync",
+  stateDefinition: {
+    name: "sync",
+  },
+};
+
+export class LastSyncMigrator extends Migrator<15, 16> {
+  async migrate(helper: MigrationHelper): Promise<void> {
+    const accounts = await helper.getAccounts<ExpectedAccountType>();
+    async function migrateAccount(userId: string, account: ExpectedAccountType): Promise<void> {
+      const value = account?.profile?.lastSync;
+      await helper.setToUser(userId, LAST_SYNC_KEY, value ?? null);
+      if (value != null) {
+        delete account.profile.lastSync;
+        await helper.set(userId, account);
+      }
+    }
+
+    await Promise.all([...accounts.map(({ userId, account }) => migrateAccount(userId, account))]);
+  }
+  async rollback(helper: MigrationHelper): Promise<void> {
+    const accounts = await helper.getAccounts<ExpectedAccountType>();
+
+    async function rollbackAccount(userId: string, account: ExpectedAccountType): Promise<void> {
+      const value = await helper.getFromUser(userId, LAST_SYNC_KEY);
+      if (account) {
+        account.profile = Object.assign(account.profile ?? {}, {
+          lastSync: value,
+        });
+        await helper.set(userId, account);
+      }
+      await helper.setToUser(userId, LAST_SYNC_KEY, null);
+    }
+
+    await Promise.all([...accounts.map(({ userId, account }) => rollbackAccount(userId, account))]);
+  }
+}

libs/common/src/vault/abstractions/sync/sync.service.abstraction.ts

@@ -1,14 +1,18 @@
+import { Observable } from "rxjs";
+
 import {
   SyncCipherNotification,
   SyncFolderNotification,
   SyncSendNotification,
 } from "../../../models/response/notification.response";
+import { UserId } from "../../../types/guid";
 
 export abstract class SyncService {
   syncInProgress: boolean;
+  lastSync$: Observable<Date | null>;
 
   getLastSync: () => Promise<Date>;
-  setLastSync: (date: Date, userId?: string) => Promise<any>;
+  setLastSync: (date: Date, userId?: UserId) => Promise<any>;
   fullSync: (forceSync: boolean, allowThrowOnError?: boolean) => Promise<boolean>;
   syncUpsertFolder: (notification: SyncFolderNotification, isEdit: boolean) => Promise<boolean>;
   syncDeleteFolder: (notification: SyncFolderNotification) => Promise<boolean>;

libs/common/src/vault/services/sync/sync.service.ts

@@ -1,3 +1,5 @@
+import { firstValueFrom, map } from "rxjs";
+
 import { ApiService } from "../../../abstractions/api.service";
 import { SettingsService } from "../../../abstractions/settings.service";
 import { InternalOrganizationServiceAbstraction } from "../../../admin-console/abstractions/organization/organization.service.abstraction";
@@ -23,10 +25,12 @@ import { MessagingService } from "../../../platform/abstractions/messaging.servi
 import { StateService } from "../../../platform/abstractions/state.service";
 import { sequentialize } from "../../../platform/misc/sequentialize";
 import { AccountDecryptionOptions } from "../../../platform/models/domain/account";
+import { KeyDefinition, StateProvider, SYNC_STATE } from "../../../platform/state";
 import { SendData } from "../../../tools/send/models/data/send.data";
 import { SendResponse } from "../../../tools/send/models/response/send.response";
 import { SendApiService } from "../../../tools/send/services/send-api.service.abstraction";
 import { InternalSendService } from "../../../tools/send/services/send.service.abstraction";
+import { UserId } from "../../../types/guid";
 import { CipherService } from "../../../vault/abstractions/cipher.service";
 import { FolderApiServiceAbstraction } from "../../../vault/abstractions/folder/folder-api.service.abstraction";
 import { InternalFolderService } from "../../../vault/abstractions/folder/folder.service.abstraction";
@@ -39,8 +43,22 @@ import { CollectionService } from "../../abstractions/collection.service";
 import { CollectionData } from "../../models/data/collection.data";
 import { CollectionDetailsResponse } from "../../models/response/collection.response";
 
+const LAST_SYNC_KEY = new KeyDefinition<string | null>(SYNC_STATE, "lastSync", {
+  deserializer: (value) => value,
+});
+
 export class SyncService implements SyncServiceAbstraction {
+  private lastSyncState = this.stateProvider.getActive(LAST_SYNC_KEY);
+
   syncInProgress = false;
+  lastSync$ = this.lastSyncState.state$.pipe(
+    map((value) => {
+      if (value == null) {
+        return null;
+      }
+      return new Date(value);
+    }),
+  );
 
   constructor(
     private apiService: ApiService,
@@ -59,24 +77,20 @@ export class SyncService implements SyncServiceAbstraction {
     private folderApiService: FolderApiServiceAbstraction,
     private organizationService: InternalOrganizationServiceAbstraction,
     private sendApiService: SendApiService,
+    private stateProvider: StateProvider,
     private logoutCallback: (expired: boolean) => Promise<void>,
   ) {}
 
-  async getLastSync(): Promise<Date> {
-    if ((await this.stateService.getUserId()) == null) {
-      return null;
+  async getLastSync(): Promise<Date | null> {
+    return await firstValueFrom(this.lastSync$);
   }
 
-    const lastSync = await this.stateService.getLastSync();
-    if (lastSync) {
-      return new Date(lastSync);
+  async setLastSync(date: Date, userId?: UserId): Promise<any> {
+    if (userId !== undefined) {
+      await this.stateProvider.getUser(userId, LAST_SYNC_KEY).update(() => date.toJSON());
+    } else {
+      await this.lastSyncState.update(() => date.toJSON());
     }
-
-    return null;
-  }
-
-  async setLastSync(date: Date, userId?: string): Promise<any> {
-    await this.stateService.setLastSync(date.toJSON(), { userId: userId });
   }
 
   @sequentialize(() => "fullSync")