PM-3585 Improve state migrations (#5009)

* WIP: safer state migrations Co-authored-by: Justin Baur <justindbaur@users.noreply.github.com> * Add min version check and remove old migrations Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com> * Add rollback and version checking * Add state version move migration * Expand tests and improve typing for Migrations * Remove StateMigration Service * Rewrite version 5 and 6 migrations * Add all but initial migration to supported migrations * Handle stateVersion location in migrator update versions * Move to unique migrations directory * Disallow imports outside of state-migrations * Lint and test fixes * Do not run migrations if we cannot determine state * Fix desktop background StateService build * Document Migration builder class * Add debug logging to migrations * Comment on migrator overrides * Use specific property names * `npm run prettier` 🤖 * Insert new migration * Set stateVersion when creating new globals object * PR comments * Fix migrate imports * Move migration building into `migrate` function * Export current version from migration definitions * Move file version concerns to migrator * Update migrate spec to reflect new version requirements * Fix import paths * Prefer unique state data * Remove unnecessary async * Prefer to not use `any` --------- Co-authored-by: Justin Baur <justindbaur@users.noreply.github.com> Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com>
2025-12-15 15:53:27 +00:00 · 2023-08-30 12:57:20 -05:00
parent b444eed0b5
commit 3340af8084
51 changed files with 1538 additions and 980 deletions
--- a/apps/browser/src/autofill/browser/cipher-context-menu-handler.ts
+++ b/apps/browser/src/autofill/browser/cipher-context-menu-handler.ts
@@ -66,9 +66,6 @@ export class CipherContextMenuHandler {
        clipboardWriteCallback: NOT_IMPLEMENTED,
        win: self,
      },
-      stateMigrationServiceOptions: {
-        stateFactory: stateFactory,
-      },
      stateServiceOptions: {
        stateFactory: stateFactory,
      },
--- a/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts
+++ b/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts
@@ -88,9 +88,6 @@ export class ContextMenuClickedHandler {
        clipboardWriteCallback: NOT_IMPLEMENTED,
        win: self,
      },
-      stateMigrationServiceOptions: {
-        stateFactory: stateFactory,
-      },
      stateServiceOptions: {
        stateFactory: stateFactory,
      },
--- a/apps/browser/src/autofill/browser/main-context-menu-handler.ts
+++ b/apps/browser/src/autofill/browser/main-context-menu-handler.ts
@@ -79,9 +79,6 @@ export class MainContextMenuHandler {
        logServiceOptions: {
          isDev: false,
        },
-        stateMigrationServiceOptions: {
-          stateFactory: stateFactory,
-        },
        stateServiceOptions: {
          stateFactory: stateFactory,
        },
--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@@ -59,7 +59,6 @@ import { EncryptServiceImplementation } from "@bitwarden/common/platform/service
 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 { StateMigrationService } from "@bitwarden/common/platform/services/state-migration.service";
 import { SystemService } from "@bitwarden/common/platform/services/system.service";
 import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service";
 import { AvatarUpdateService } from "@bitwarden/common/services/account/avatar-update.service";
@@ -177,7 +176,6 @@ export default class MainBackground {
  searchService: SearchServiceAbstraction;
  notificationsService: NotificationsServiceAbstraction;
  stateService: StateServiceAbstraction;
-  stateMigrationService: StateMigrationService;
  systemService: SystemServiceAbstraction;
  eventCollectionService: EventCollectionServiceAbstraction;
  eventUploadService: EventUploadServiceAbstraction;
@@ -262,17 +260,11 @@ export default class MainBackground {
            new KeyGenerationService(this.cryptoFunctionService)
          )
        : new MemoryStorageService();
-    this.stateMigrationService = new StateMigrationService(
-      this.storageService,
-      this.secureStorageService,
-      new StateFactory(GlobalState, Account)
-    );
    this.stateService = new BrowserStateService(
      this.storageService,
      this.secureStorageService,
      this.memoryStorageService,
      this.logService,
-      this.stateMigrationService,
      new StateFactory(GlobalState, Account)
    );
    this.platformUtilsService = new BrowserPlatformUtilsService(
--- a/apps/browser/src/platform/background/service-factories/state-migration-service.factory.ts
+++ b/apps/browser/src/platform/background/service-factories/state-migration-service.factory.ts
@@ -1,40 +0,0 @@
-import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
-import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state";
-import { StateMigrationService } from "@bitwarden/common/platform/services/state-migration.service";
-
-import { Account } from "../../../models/account";
-
-import { CachedServices, factory, FactoryOptions } from "./factory-options";
-import {
-  diskStorageServiceFactory,
-  DiskStorageServiceInitOptions,
-  secureStorageServiceFactory,
-  SecureStorageServiceInitOptions,
-} from "./storage-service.factory";
-
-type StateMigrationServiceFactoryOptions = FactoryOptions & {
-  stateMigrationServiceOptions: {
-    stateFactory: StateFactory<GlobalState, Account>;
-  };
-};
-
-export type StateMigrationServiceInitOptions = StateMigrationServiceFactoryOptions &
-  DiskStorageServiceInitOptions &
-  SecureStorageServiceInitOptions;
-
-export function stateMigrationServiceFactory(
-  cache: { stateMigrationService?: StateMigrationService } & CachedServices,
-  opts: StateMigrationServiceInitOptions
-): Promise<StateMigrationService> {
-  return factory(
-    cache,
-    "stateMigrationService",
-    opts,
-    async () =>
-      new StateMigrationService(
-        await diskStorageServiceFactory(cache, opts),
-        await secureStorageServiceFactory(cache, opts),
-        opts.stateMigrationServiceOptions.stateFactory
-      )
-  );
-}
--- a/apps/browser/src/platform/background/service-factories/state-service.factory.ts
+++ b/apps/browser/src/platform/background/service-factories/state-service.factory.ts
@@ -6,10 +6,6 @@ import { BrowserStateService } from "../../services/browser-state.service";

 import { CachedServices, factory, FactoryOptions } from "./factory-options";
 import { logServiceFactory, LogServiceInitOptions } from "./log-service.factory";
-import {
-  stateMigrationServiceFactory,
-  StateMigrationServiceInitOptions,
-} from "./state-migration-service.factory";
 import {
  diskStorageServiceFactory,
  secureStorageServiceFactory,
@@ -30,8 +26,7 @@ export type StateServiceInitOptions = StateServiceFactoryOptions &
  DiskStorageServiceInitOptions &
  SecureStorageServiceInitOptions &
  MemoryStorageServiceInitOptions &
-  LogServiceInitOptions &
-  StateMigrationServiceInitOptions;
+  LogServiceInitOptions;

 export async function stateServiceFactory(
  cache: { stateService?: BrowserStateService } & CachedServices,
@@ -47,7 +42,6 @@ export async function stateServiceFactory(
        await secureStorageServiceFactory(cache, opts),
        await memoryStorageServiceFactory(cache, opts),
        await logServiceFactory(cache, opts),
-        await stateMigrationServiceFactory(cache, opts),
        opts.stateServiceOptions.stateFactory,
        opts.stateServiceOptions.useAccountCache
      )
--- a/apps/browser/src/platform/listeners/on-command-listener.ts
+++ b/apps/browser/src/platform/listeners/on-command-listener.ts
@@ -47,9 +47,6 @@ const doAutoFillLogin = async (tab: chrome.tabs.Tab): Promise<void> => {
    stateServiceOptions: {
      stateFactory: new StateFactory(GlobalState, Account),
    },
-    stateMigrationServiceOptions: {
-      stateFactory: new StateFactory(GlobalState, Account),
-    },
    apiServiceOptions: {
      logoutCallback: () => Promise.resolve(),
    },
@@ -94,9 +91,6 @@ const doGeneratePasswordToClipboard = async (tab: chrome.tabs.Tab): Promise<void
      clipboardWriteCallback: () => Promise.resolve(),
      win: self,
    },
-    stateMigrationServiceOptions: {
-      stateFactory: stateFactory,
-    },
    stateServiceOptions: {
      stateFactory: stateFactory,
    },
--- a/apps/browser/src/platform/listeners/on-install-listener.ts
+++ b/apps/browser/src/platform/listeners/on-install-listener.ts
@@ -23,9 +23,6 @@ export async function onInstallListener(details: chrome.runtime.InstalledDetails
    stateServiceOptions: {
      stateFactory: new StateFactory(GlobalState, Account),
    },
-    stateMigrationServiceOptions: {
-      stateFactory: new StateFactory(GlobalState, Account),
-    },
  };
  const environmentService = await environmentServiceFactory(cache, opts);

--- a/apps/browser/src/platform/listeners/update-badge.ts
+++ b/apps/browser/src/platform/listeners/update-badge.ts
@@ -272,9 +272,6 @@ export class UpdateBadge {
      stateServiceOptions: {
        stateFactory: new StateFactory(GlobalState, Account),
      },
-      stateMigrationServiceOptions: {
-        stateFactory: new StateFactory(GlobalState, Account),
-      },
      apiServiceOptions: {
        logoutCallback: () => Promise.reject("not implemented"),
      },
--- a/apps/browser/src/platform/services/browser-state.service.spec.ts
+++ b/apps/browser/src/platform/services/browser-state.service.spec.ts
@@ -8,7 +8,6 @@ import {
 import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
 import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state";
 import { State } from "@bitwarden/common/platform/models/domain/state";
-import { StateMigrationService } from "@bitwarden/common/platform/services/state-migration.service";
 import { SendType } from "@bitwarden/common/tools/send/enums/send-type";
 import { SendView } from "@bitwarden/common/tools/send/models/view/send.view";

@@ -26,7 +25,6 @@ describe("Browser State Service", () => {
  let secureStorageService: MockProxy<AbstractStorageService>;
  let diskStorageService: MockProxy<AbstractStorageService>;
  let logService: MockProxy<LogService>;
-  let stateMigrationService: MockProxy<StateMigrationService>;
  let stateFactory: MockProxy<StateFactory<GlobalState, Account>>;
  let useAccountCache: boolean;

@@ -39,7 +37,6 @@ describe("Browser State Service", () => {
    secureStorageService = mock();
    diskStorageService = mock();
    logService = mock();
-    stateMigrationService = mock();
    stateFactory = mock();
    // turn off account cache for tests
    useAccountCache = false;
@@ -64,7 +61,6 @@ describe("Browser State Service", () => {
        secureStorageService,
        memoryStorageService,
        logService,
-        stateMigrationService,
        stateFactory,
        useAccountCache
      );
--- a/apps/browser/src/platform/services/browser-state.service.ts
+++ b/apps/browser/src/platform/services/browser-state.service.ts
@@ -1,7 +1,6 @@
 import { BehaviorSubject } from "rxjs";

 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
-import { StateMigrationService } from "@bitwarden/common/platform/abstractions/state-migration.service";
 import {
  AbstractStorageService,
  AbstractMemoryStorageService,
@@ -41,7 +40,6 @@ export class BrowserStateService
    secureStorageService: AbstractStorageService,
    memoryStorageService: AbstractMemoryStorageService,
    logService: LogService,
-    stateMigrationService: StateMigrationService,
    stateFactory: StateFactory<GlobalState, Account>,
    useAccountCache = true
  ) {
@@ -50,7 +48,6 @@ export class BrowserStateService
      secureStorageService,
      memoryStorageService,
      logService,
-      stateMigrationService,
      stateFactory,
      useAccountCache
    );
--- a/apps/browser/src/popup/services/services.module.ts
+++ b/apps/browser/src/popup/services/services.module.ts
@@ -47,7 +47,6 @@ import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platfor
 import { LogService as LogServiceAbstraction } from "@bitwarden/common/platform/abstractions/log.service";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
-import { StateMigrationService } from "@bitwarden/common/platform/abstractions/state-migration.service";
 import {
  StateService as BaseStateServiceAbstraction,
  StateService,
@@ -442,36 +441,23 @@ function getBgService<T>(service: keyof MainBackground) {
      provide: MEMORY_STORAGE,
      useFactory: getBgService<AbstractStorageService>("memoryStorageService"),
    },
-    {
-      provide: StateMigrationService,
-      useFactory: getBgService<StateMigrationService>("stateMigrationService"),
-      deps: [],
-    },
    {
      provide: StateServiceAbstraction,
      useFactory: (
        storageService: AbstractStorageService,
        secureStorageService: AbstractStorageService,
        memoryStorageService: AbstractMemoryStorageService,
-        logService: LogServiceAbstraction,
-        stateMigrationService: StateMigrationService
+        logService: LogServiceAbstraction
      ) => {
        return new BrowserStateService(
          storageService,
          secureStorageService,
          memoryStorageService,
          logService,
-          stateMigrationService,
          new StateFactory(GlobalState, Account)
        );
      },
-      deps: [
-        AbstractStorageService,
-        SECURE_STORAGE,
-        MEMORY_STORAGE,
-        LogServiceAbstraction,
-        StateMigrationService,
-      ],
+      deps: [AbstractStorageService, SECURE_STORAGE, MEMORY_STORAGE, LogServiceAbstraction],
    },
    {
      provide: UsernameGenerationServiceAbstraction,
--- a/apps/cli/src/bw.ts
+++ b/apps/cli/src/bw.ts
@@ -37,7 +37,6 @@ import { EnvironmentService } from "@bitwarden/common/platform/services/environm
 import { FileUploadService } from "@bitwarden/common/platform/services/file-upload/file-upload.service";
 import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service";
 import { NoopMessagingService } from "@bitwarden/common/platform/services/noop-messaging.service";
-import { StateMigrationService } from "@bitwarden/common/platform/services/state-migration.service";
 import { StateService } from "@bitwarden/common/platform/services/state.service";
 import { AuditService } from "@bitwarden/common/services/audit.service";
 import { OrganizationUserServiceImplementation } from "@bitwarden/common/services/organization-user/organization-user.service.implementation";
@@ -136,7 +135,6 @@ export class Main {
  keyConnectorService: KeyConnectorService;
  userVerificationService: UserVerificationService;
  stateService: StateService;
-  stateMigrationService: StateMigrationService;
  organizationService: OrganizationService;
  providerService: ProviderService;
  twoFactorService: TwoFactorService;
@@ -188,18 +186,11 @@ export class Main {

    this.memoryStorageService = new MemoryStorageService();

-    this.stateMigrationService = new StateMigrationService(
-      this.storageService,
-      this.secureStorageService,
-      new StateFactory(GlobalState, Account)
-    );
-
    this.stateService = new StateService(
      this.storageService,
      this.secureStorageService,
      this.memoryStorageService,
      this.logService,
-      this.stateMigrationService,
      new StateFactory(GlobalState, Account)
    );

--- a/apps/desktop/src/app/services/services.module.ts
+++ b/apps/desktop/src/app/services/services.module.ts
@@ -28,7 +28,6 @@ import {
 } from "@bitwarden/common/platform/abstractions/log.service";
 import { MessagingService as MessagingServiceAbstraction } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwarden/common/platform/abstractions/platform-utils.service";
-import { StateMigrationService as StateMigrationServiceAbstraction } from "@bitwarden/common/platform/abstractions/state-migration.service";
 import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service";
 import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service";
 import { SystemService as SystemServiceAbstraction } from "@bitwarden/common/platform/abstractions/system.service";
@@ -134,7 +133,6 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK");
        SECURE_STORAGE,
        MEMORY_STORAGE,
        LogService,
-        StateMigrationServiceAbstraction,
        STATE_FACTORY,
        STATE_SERVICE_USE_CACHE,
      ],
--- a/apps/desktop/src/main.ts
+++ b/apps/desktop/src/main.ts
@@ -90,7 +90,6 @@ export class Main {
      null,
      this.memoryStorageService,
      this.logService,
-      null,
      new StateFactory(GlobalState, Account),
      false // Do not use disk caching because this will get out of sync with the renderer service
    );
--- a/apps/web/src/app/core/core.module.ts
+++ b/apps/web/src/app/core/core.module.ts
@@ -17,7 +17,6 @@ import { FileDownloadService } from "@bitwarden/common/platform/abstractions/fil
 import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { MessagingService as MessagingServiceAbstraction } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwarden/common/platform/abstractions/platform-utils.service";
-import { StateMigrationService as StateMigrationServiceAbstraction } from "@bitwarden/common/platform/abstractions/state-migration.service";
 import { StateService as BaseStateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service";
 import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service";
 import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
@@ -27,7 +26,6 @@ import { PasswordRepromptService as PasswordRepromptServiceAbstraction } from "@
 import { PolicyListService } from "../admin-console/core/policy-list.service";
 import { HtmlStorageService } from "../core/html-storage.service";
 import { I18nService } from "../core/i18n.service";
-import { StateMigrationService } from "../core/state-migration.service";
 import { CollectionAdminService } from "../vault/core/collection-admin.service";
 import { PasswordRepromptService } from "../vault/core/password-reprompt.service";

@@ -84,11 +82,6 @@ import { WebPlatformUtilsService } from "./web-platform-utils.service";
    },
    { provide: MessagingServiceAbstraction, useClass: BroadcasterMessagingService },
    { provide: ModalServiceAbstraction, useClass: ModalService },
-    {
-      provide: StateMigrationServiceAbstraction,
-      useClass: StateMigrationService,
-      deps: [AbstractStorageService, SECURE_STORAGE, STATE_FACTORY],
-    },
    StateService,
    {
      provide: BaseStateServiceAbstraction,
--- a/apps/web/src/app/core/state-migration.service.ts
+++ b/apps/web/src/app/core/state-migration.service.ts
@@ -1,13 +0,0 @@
-import { StateMigrationService as BaseStateMigrationService } from "@bitwarden/common/platform/services/state-migration.service";
-
-import { Account } from "./state/account";
-import { GlobalState } from "./state/global-state";
-
-export class StateMigrationService extends BaseStateMigrationService<GlobalState, Account> {
-  protected async migrationStateFrom1To2(): Promise<void> {
-    await super.migrateStateFrom1To2();
-    const globals = (await this.get<GlobalState>("global")) ?? this.stateFactory.createGlobal(null);
-    globals.rememberEmail = (await this.get<boolean>("rememberEmail")) ?? globals.rememberEmail;
-    await this.set("global", globals);
-  }
-}
--- a/apps/web/src/app/core/state/state.service.ts
+++ b/apps/web/src/app/core/state/state.service.ts
@@ -7,7 +7,6 @@ import {
  STATE_SERVICE_USE_CACHE,
 } from "@bitwarden/angular/services/injection-tokens";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
-import { StateMigrationService } from "@bitwarden/common/platform/abstractions/state-migration.service";
 import {
  AbstractMemoryStorageService,
  AbstractStorageService,
@@ -30,7 +29,6 @@ export class StateService extends BaseStateService<GlobalState, Account> {
    @Inject(SECURE_STORAGE) secureStorageService: AbstractStorageService,
    @Inject(MEMORY_STORAGE) memoryStorageService: AbstractMemoryStorageService,
    logService: LogService,
-    stateMigrationService: StateMigrationService,
    @Inject(STATE_FACTORY) stateFactory: StateFactory<GlobalState, Account>,
    @Inject(STATE_SERVICE_USE_CACHE) useAccountCache = true
  ) {
@@ -39,7 +37,6 @@ export class StateService extends BaseStateService<GlobalState, Account> {
      secureStorageService,
      memoryStorageService,
      logService,
-      stateMigrationService,
      stateFactory,
      useAccountCache
    );