[bug] Assign client specific account settings during migration (#653)

* [bug] Assign client specific account settings during migration

* [refactor] Write State type arguements in consistent order

* [style] Ran prettier

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

@@ -360,7 +360,7 @@ import { StateFactory } from "jslib-common/factories/stateFactory";
         new StateMigrationService(
           storageService,
           secureStorageService,
-          new GlobalStateFactory(GlobalState)
+          new StateFactory(GlobalState, Account)
         ),
       deps: [StorageServiceAbstraction, "SECURE_STORAGE"],
     },

common/src/factories/stateFactory.ts

@@ -3,7 +3,10 @@ import { GlobalState } from "../models/domain/globalState";
 import { AccountFactory } from "./accountFactory";
 import { GlobalStateFactory } from "./globalStateFactory";
 
-export class StateFactory<TAccount extends Account, TGlobal extends GlobalState> {
+export class StateFactory<
+  TGlobal extends GlobalState = GlobalState,
+  TAccount extends Account = Account
+> {
   private globalStateFactory: GlobalStateFactory<TGlobal>;
   private accountFactory: AccountFactory<TAccount>;
 

common/src/models/domain/state.ts

@@ -2,8 +2,8 @@ import { Account } from "./account";
 import { GlobalState } from "./globalState";
 
 export class State<
-  TAccount extends Account = Account,
+  TGlobalState extends GlobalState = GlobalState,
-  TGlobalState extends GlobalState = GlobalState
+  TAccount extends Account = Account
 > {
   accounts: { [userId: string]: TAccount } = {};
   globals: TGlobalState;

common/src/services/state.service.ts

@@ -55,14 +55,14 @@ const partialKeys = {
 };
 
 export class StateService<
-  TAccount extends Account = Account,
+  TGlobalState extends GlobalState = GlobalState,
-  TGlobalState extends GlobalState = GlobalState
+  TAccount extends Account = Account
 > implements StateServiceAbstraction<TAccount>
 {
   accounts = new BehaviorSubject<{ [userId: string]: TAccount }>({});
   activeAccount = new BehaviorSubject<string>(null);
 
-  protected state: State<TAccount, TGlobalState> = new State<TAccount, TGlobalState>(
+  protected state: State<TGlobalState, TAccount> = new State<TGlobalState, TAccount>(
     this.createGlobals()
   );
 
@@ -73,7 +73,7 @@ export class StateService<
     protected secureStorageService: StorageService,
     protected logService: LogService,
     protected stateMigrationService: StateMigrationService,
-    protected stateFactory: StateFactory<TAccount, TGlobalState>
+    protected stateFactory: StateFactory<TGlobalState, TAccount>
   ) {}
 
   async init(): Promise<void> {
@@ -2233,7 +2233,6 @@ export class StateService<
       account.settings = await this.storageService.get<any>(keys.tempAccountSettings);
       await this.storageService.remove(keys.tempAccountSettings);
     }
-    Object.assign(account.settings, this.createAccount().settings);
     account.settings.environmentUrls = environmentUrls;
     await this.storageService.save(
       account.profile.userId,

common/src/services/stateMigration.service.ts

@@ -21,6 +21,8 @@ import { ThemeType } from "../enums/themeType";
 import { EnvironmentUrls } from "../models/domain/environmentUrls";
 
 import { GlobalStateFactory } from "../factories/globalStateFactory";
+import { StateFactory } from "../factories/stateFactory";
+import { Account, AccountSettings } from "../models/domain/account";
 
 // Originally (before January 2022) storage was handled as a flat key/value pair store.
 // With the move to a typed object for state storage these keys should no longer be in use anywhere outside of this migration.
@@ -128,11 +130,14 @@ const partialKeys = {
   masterKey: "_masterkey",
 };
 
-export class StateMigrationService<TGlobalState extends GlobalState = GlobalState> {
+export class StateMigrationService<
+  TGlobalState extends GlobalState = GlobalState,
+  TAccount extends Account = Account
+> {
   constructor(
     protected storageService: StorageService,
     protected secureStorageService: StorageService,
-    protected globalStateFactory: GlobalStateFactory<TGlobalState>
+    protected stateFactory: StateFactory<TGlobalState, TAccount>
   ) {}
 
   async needsMigration(): Promise<boolean> {
@@ -177,7 +182,8 @@ export class StateMigrationService<TGlobalState extends GlobalState = GlobalStat
     // 1. Check for an existing storage value from the old storage structure OR
     // 2. Check for a value already set by processes that run before migration OR
     // 3. Assign the default value
-    const globals = (await this.get<GlobalState>(keys.global)) ?? this.globalStateFactory.create();
+    const globals =
+      (await this.get<GlobalState>(keys.global)) ?? this.stateFactory.createGlobal(null);
     globals.stateVersion = StateVersion.Two;
     globals.environmentUrls =
       (await this.get<EnvironmentUrls>(v1Keys.environmentUrls)) ?? globals.environmentUrls;
@@ -220,46 +226,91 @@ export class StateMigrationService<TGlobalState extends GlobalState = GlobalStat
     const userId =
       (await this.get<string>(v1Keys.userId)) ?? (await this.get<string>(v1Keys.entityId));
 
-    // (userId == null) = no logged in user (so no known userId) and we need to temporarily store account specific settings in state to migrate on first auth
+    const defaultAccount = this.stateFactory.createAccount(null);
-    // (userId != null) = we have a currently authed user (so known userId) with encrypted data and other key settings we can move, no need to temporarily store account settings
+    const accountSettings: AccountSettings = {
-    if (userId == null) {
+      autoConfirmFingerPrints:
-      await this.set(keys.tempAccountSettings, {
+        (await this.get<boolean>(v1Keys.autoConfirmFingerprints)) ??
-        autoConfirmFingerPrints: await this.get<boolean>(v1Keys.autoConfirmFingerprints),
+        defaultAccount.settings.autoConfirmFingerPrints,
-        autoFillOnPageLoadDefault: await this.get<boolean>(v1Keys.autoFillOnPageLoadDefault),
+      autoFillOnPageLoadDefault:
+        (await this.get<boolean>(v1Keys.autoFillOnPageLoadDefault)) ??
+        defaultAccount.settings.autoFillOnPageLoadDefault,
       biometricLocked: null,
-        biometricUnlock: await this.get<boolean>(v1Keys.biometricUnlock),
+      biometricUnlock:
-        clearClipboard: await this.get<number>(v1Keys.clearClipboard),
+        (await this.get<boolean>(v1Keys.biometricUnlock)) ??
-        defaultUriMatch: await this.get<any>(v1Keys.defaultUriMatch),
+        defaultAccount.settings.biometricUnlock,
-        disableAddLoginNotification: await this.get<boolean>(v1Keys.disableAddLoginNotification),
+      clearClipboard:
-        disableAutoBiometricsPrompt: await this.get<boolean>(v1Keys.disableAutoBiometricsPrompt),
+        (await this.get<number>(v1Keys.clearClipboard)) ?? defaultAccount.settings.clearClipboard,
-        disableAutoTotpCopy: await this.get<boolean>(v1Keys.disableAutoTotpCopy),
+      defaultUriMatch:
-        disableBadgeCounter: await this.get<boolean>(v1Keys.disableBadgeCounter),
+        (await this.get<any>(v1Keys.defaultUriMatch)) ?? defaultAccount.settings.defaultUriMatch,
-        disableChangedPasswordNotification: await this.get<boolean>(
+      disableAddLoginNotification:
-          v1Keys.disableChangedPasswordNotification
+        (await this.get<boolean>(v1Keys.disableAddLoginNotification)) ??
-        ),
+        defaultAccount.settings.disableAddLoginNotification,
-        disableContextMenuItem: await this.get<boolean>(v1Keys.disableContextMenuItem),
+      disableAutoBiometricsPrompt:
-        disableGa: await this.get<boolean>(v1Keys.disableGa),
+        (await this.get<boolean>(v1Keys.disableAutoBiometricsPrompt)) ??
-        dontShowCardsCurrentTab: await this.get<boolean>(v1Keys.dontShowCardsCurrentTab),
+        defaultAccount.settings.disableAutoBiometricsPrompt,
-        dontShowIdentitiesCurrentTab: await this.get<boolean>(v1Keys.dontShowIdentitiesCurrentTab),
+      disableAutoTotpCopy:
-        enableAlwaysOnTop: await this.get<boolean>(v1Keys.enableAlwaysOnTop),
+        (await this.get<boolean>(v1Keys.disableAutoTotpCopy)) ??
-        enableAutoFillOnPageLoad: await this.get<boolean>(v1Keys.enableAutoFillOnPageLoad),
+        defaultAccount.settings.disableAutoTotpCopy,
-        enableBiometric: await this.get<boolean>(v1Keys.enableBiometric),
+      disableBadgeCounter:
-        enableFullWidth: await this.get<boolean>(v1Keys.enableFullWidth),
+        (await this.get<boolean>(v1Keys.disableBadgeCounter)) ??
-        enableGravitars: await this.get<boolean>(v1Keys.enableGravatars),
+        defaultAccount.settings.disableBadgeCounter,
-        environmentUrls: globals.environmentUrls,
+      disableChangedPasswordNotification:
-        equivalentDomains: await this.get<any>(v1Keys.equivalentDomains),
+        (await this.get<boolean>(v1Keys.disableChangedPasswordNotification)) ??
-        minimizeOnCopyToClipboard: await this.get<boolean>(v1Keys.minimizeOnCopyToClipboard),
+        defaultAccount.settings.disableChangedPasswordNotification,
-        neverDomains: await this.get<any>(v1Keys.neverDomains),
+      disableContextMenuItem:
-        passwordGenerationOptions: await this.get<any>(v1Keys.passwordGenerationOptions),
+        (await this.get<boolean>(v1Keys.disableContextMenuItem)) ??
+        defaultAccount.settings.disableContextMenuItem,
+      disableGa: (await this.get<boolean>(v1Keys.disableGa)) ?? defaultAccount.settings.disableGa,
+      dontShowCardsCurrentTab:
+        (await this.get<boolean>(v1Keys.dontShowCardsCurrentTab)) ??
+        defaultAccount.settings.dontShowCardsCurrentTab,
+      dontShowIdentitiesCurrentTab:
+        (await this.get<boolean>(v1Keys.dontShowIdentitiesCurrentTab)) ??
+        defaultAccount.settings.dontShowIdentitiesCurrentTab,
+      enableAlwaysOnTop:
+        (await this.get<boolean>(v1Keys.enableAlwaysOnTop)) ??
+        defaultAccount.settings.enableAlwaysOnTop,
+      enableAutoFillOnPageLoad:
+        (await this.get<boolean>(v1Keys.enableAutoFillOnPageLoad)) ??
+        defaultAccount.settings.enableAutoFillOnPageLoad,
+      enableBiometric:
+        (await this.get<boolean>(v1Keys.enableBiometric)) ??
+        defaultAccount.settings.enableBiometric,
+      enableFullWidth:
+        (await this.get<boolean>(v1Keys.enableFullWidth)) ??
+        defaultAccount.settings.enableFullWidth,
+      enableGravitars:
+        (await this.get<boolean>(v1Keys.enableGravatars)) ??
+        defaultAccount.settings.enableGravitars,
+      environmentUrls: globals.environmentUrls ?? defaultAccount.settings.environmentUrls,
+      equivalentDomains:
+        (await this.get<any>(v1Keys.equivalentDomains)) ??
+        defaultAccount.settings.equivalentDomains,
+      minimizeOnCopyToClipboard:
+        (await this.get<boolean>(v1Keys.minimizeOnCopyToClipboard)) ??
+        defaultAccount.settings.minimizeOnCopyToClipboard,
+      neverDomains:
+        (await this.get<any>(v1Keys.neverDomains)) ?? defaultAccount.settings.neverDomains,
+      passwordGenerationOptions:
+        (await this.get<any>(v1Keys.passwordGenerationOptions)) ??
+        defaultAccount.settings.passwordGenerationOptions,
       pinProtected: {
         decrypted: null,
         encrypted: await this.get<string>(v1Keys.pinProtected),
       },
       protectedPin: await this.get<string>(v1Keys.protectedPin),
-        settings: null,
+      settings: userId == null ? null : await this.get<any>(v1KeyPrefixes.settings + userId),
-        vaultTimeout: await this.get<number>(v1Keys.vaultTimeout),
+      vaultTimeout:
-        vaultTimeoutAction: await this.get<string>(v1Keys.vaultTimeoutAction),
+        (await this.get<number>(v1Keys.vaultTimeout)) ?? defaultAccount.settings.vaultTimeout,
-      });
+      vaultTimeoutAction:
+        (await this.get<string>(v1Keys.vaultTimeoutAction)) ??
+        defaultAccount.settings.vaultTimeoutAction,
+    };
+
+    // (userId == null) = no logged in user (so no known userId) and we need to temporarily store account specific settings in state to migrate on first auth
+    // (userId != null) = we have a currently authed user (so known userId) with encrypted data and other key settings we can move, no need to temporarily store account settings
+    if (userId == null) {
+      await this.set(keys.tempAccountSettings, accountSettings);
       await this.set(keys.global, globals);
       await this.set(keys.authenticatedAccounts, []);
       await this.set(keys.activeUserId, null);
@@ -350,43 +401,7 @@ export class StateMigrationService<TGlobalState extends GlobalState = GlobalStat
         userId: userId,
         usesKeyConnector: null,
       },
-      settings: {
+      settings: accountSettings,
-        autoConfirmFingerPrints: await this.get<boolean>(v1Keys.autoConfirmFingerprints),
-        autoFillOnPageLoadDefault: await this.get<boolean>(v1Keys.autoFillOnPageLoadDefault),
-        biometricLocked: null,
-        biometricUnlock: await this.get<boolean>(v1Keys.biometricUnlock),
-        clearClipboard: await this.get<number>(v1Keys.clearClipboard),
-        defaultUriMatch: await this.get<any>(v1Keys.defaultUriMatch),
-        disableAddLoginNotification: await this.get<boolean>(v1Keys.disableAddLoginNotification),
-        disableAutoBiometricsPrompt: await this.get<boolean>(v1Keys.disableAutoBiometricsPrompt),
-        disableAutoTotpCopy: await this.get<boolean>(v1Keys.disableAutoTotpCopy),
-        disableBadgeCounter: await this.get<boolean>(v1Keys.disableBadgeCounter),
-        disableChangedPasswordNotification: await this.get<boolean>(
-          v1Keys.disableChangedPasswordNotification
-        ),
-        disableContextMenuItem: await this.get<boolean>(v1Keys.disableContextMenuItem),
-        disableGa: await this.get<boolean>(v1Keys.disableGa),
-        dontShowCardsCurrentTab: await this.get<boolean>(v1Keys.dontShowCardsCurrentTab),
-        dontShowIdentitiesCurrentTab: await this.get<boolean>(v1Keys.dontShowIdentitiesCurrentTab),
-        enableAlwaysOnTop: await this.get<boolean>(v1Keys.enableAlwaysOnTop),
-        enableAutoFillOnPageLoad: await this.get<boolean>(v1Keys.enableAutoFillOnPageLoad),
-        enableBiometric: await this.get<boolean>(v1Keys.enableBiometric),
-        enableFullWidth: await this.get<boolean>(v1Keys.enableFullWidth),
-        enableGravitars: await this.get<boolean>(v1Keys.enableGravatars),
-        environmentUrls: globals.environmentUrls,
-        equivalentDomains: await this.get<any>(v1Keys.equivalentDomains),
-        minimizeOnCopyToClipboard: await this.get<boolean>(v1Keys.minimizeOnCopyToClipboard),
-        neverDomains: await this.get<any>(v1Keys.neverDomains),
-        passwordGenerationOptions: await this.get<any>(v1Keys.passwordGenerationOptions),
-        pinProtected: {
-          decrypted: null,
-          encrypted: await this.get<string>(v1Keys.pinProtected),
-        },
-        protectedPin: await this.get<string>(v1Keys.protectedPin),
-        settings: await this.get<any>(v1KeyPrefixes.settings + userId),
-        vaultTimeout: await this.get<number>(v1Keys.vaultTimeout),
-        vaultTimeoutAction: await this.get<string>(v1Keys.vaultTimeoutAction),
-      },
       tokens: {
         accessToken: await this.get<string>(v1Keys.accessToken),
         decodedToken: null,