[chore] Update jslib & state services to match (#212)

* [chore] Update jslib & state services to match

* [bug] Save userId when migrating state

This is used to check for authentication, so if not present on boot of the app authenticated users will still have to log in again

* [bug] Save added accounts with userId

Currently we are passing in an account object, resulting in a null key. We should be passing in a userId

* [bug] Ensure configs and settings are not cleared on logout

We need to persist directoryConfigruations on logout so that logging out and back in doesn't require folks to need to reconfig their settings

* Remove unneeded LoginSyncService

* Run prettier

* [style] Remove commented lines

Co-authored-by: Thomas Rittson <trittson@bitwarden.com>

src/app/services/services.module.ts

@@ -43,6 +43,10 @@ import { AuthService } from "../../services/auth.service";
 import { StateService } from "../../services/state.service";
 import { StateMigrationService } from "../../services/stateMigration.service";
 
+import { Account } from "../../models/account";
+
+import { AccountFactory } from "jslib-common/models/domain/account";
+
 function refreshTokenCallback(injector: Injector) {
   return () => {
     const stateService = injector.get(StateServiceAbstraction);
@@ -197,7 +201,20 @@ export function initFactory(
     },
     {
       provide: StateServiceAbstraction,
-      useClass: StateService,
+      useFactory: (
+        storageService: StorageServiceAbstraction,
+        secureStorageService: StorageServiceAbstraction,
+        logService: LogServiceAbstraction,
+        stateMigrationService: StateMigrationServiceAbstraction
+      ) =>
+        new StateService(
+          storageService,
+          secureStorageService,
+          logService,
+          stateMigrationService,
+          true, // TODO: It seems like we aren't applying this from settings anywhere. Is toggling secure storage working?
+          new AccountFactory(Account)
+        ),
       deps: [
         StorageServiceAbstraction,
         "SECURE_STORAGE",

src/bwdc.ts

@@ -34,13 +34,15 @@ import { ProviderService } from "jslib-common/services/provider.service";
 import { SearchService } from "jslib-common/services/search.service";
 import { SendService } from "jslib-common/services/send.service";
 import { SettingsService } from "jslib-common/services/settings.service";
-import { SyncService as LoginSyncService } from "jslib-common/services/sync.service";
 import { TokenService } from "jslib-common/services/token.service";
 
 import { StorageService as StorageServiceAbstraction } from "jslib-common/abstractions/storage.service";
 
 import { Program } from "./program";
-import { refreshToken } from "./services/api.service";
+
+import { AccountFactory } from "jslib-common/models/domain/account";
+
+import { Account } from "./models/account";
 
 // tslint:disable-next-line
 const packageJson = require("./package.json");
@@ -72,7 +74,6 @@ export class Main {
   syncService: SyncService;
   passwordGenerationService: PasswordGenerationService;
   policyService: PolicyService;
-  loginSyncService: LoginSyncService;
   keyConnectorService: KeyConnectorService;
   program: Program;
   stateService: StateService;
@@ -130,7 +131,8 @@ export class Main {
       this.secureStorageService,
       this.logService,
       this.stateMigrationService,
-      process.env.BITWARDENCLI_CONNECTOR_PLAINTEXT_SECRETS !== "true"
+      process.env.BITWARDENCLI_CONNECTOR_PLAINTEXT_SECRETS !== "true",
+      new AccountFactory(Account)
     );
 
     this.cryptoService = new CryptoService(
@@ -249,24 +251,6 @@ export class Main {
 
     this.providerService = new ProviderService(this.stateService);
 
-    this.loginSyncService = new LoginSyncService(
-      this.apiService,
-      this.settingsService,
-      this.folderService,
-      this.cipherService,
-      this.cryptoService,
-      this.collectionService,
-      this.messagingService,
-      this.policyService,
-      this.sendService,
-      this.logService,
-      this.keyConnectorService,
-      this.stateService,
-      this.organizationService,
-      this.providerService,
-      async (expired: boolean) => this.messagingService.send("logout", { expired: expired })
-    );
-
     this.program = new Program(this);
   }
 

src/main.ts

@@ -15,6 +15,10 @@ import { WindowMain } from "jslib-electron/window.main";
 
 import { StateService } from "./services/state.service";
 
+import { AccountFactory } from "jslib-common/models/domain/account";
+
+import { Account } from "./models/account";
+
 export class Main {
   logService: ElectronLogService;
   i18nService: I18nService;
@@ -54,7 +58,14 @@ export class Main {
     this.logService = new ElectronLogService(null, app.getPath("userData"));
     this.i18nService = new I18nService("en", "./locales/");
     this.storageService = new ElectronStorageService(app.getPath("userData"));
-    this.stateService = new StateService(this.storageService, null, this.logService, null);
+    this.stateService = new StateService(
+      this.storageService,
+      null,
+      this.logService,
+      null,
+      true,
+      new AccountFactory(Account)
+    );
 
     this.windowMain = new WindowMain(
       this.stateService,

src/models/account.ts

@@ -16,8 +16,8 @@ export class Account extends BaseAccount {
 
   constructor(init: Partial<Account>) {
     super(init);
-    this.directoryConfigurations = init.directoryConfigurations ?? new DirectoryConfigurations();
+    this.directoryConfigurations = init?.directoryConfigurations ?? new DirectoryConfigurations();
-    this.directorySettings = init.directorySettings ?? new DirectorySettings();
+    this.directorySettings = init?.directorySettings ?? new DirectorySettings();
   }
 }
 

src/program.ts

@@ -106,9 +106,7 @@ export class Program extends BaseProgram {
           this.main.stateService,
           this.main.cryptoService,
           this.main.policyService,
-          "connector",
+          "connector"
-          this.main.loginSyncService,
-          this.main.keyConnectorService
         );
 
         if (!Utils.isNullOrWhitespace(clientId)) {

src/services/state.service.ts

@@ -1,6 +1,6 @@
 import { StateService as BaseStateService } from "jslib-common/services/state.service";
 
-import { State } from "jslib-common/models/domain/state";
+import { AccountFactory } from "jslib-common/models/domain/account";
 import { StorageOptions } from "jslib-common/models/domain/storageOptions";
 
 import { Account } from "src/models/account";
@@ -10,13 +10,15 @@ import { IConfiguration } from "src/models/IConfiguration";
 import { LdapConfiguration } from "src/models/ldapConfiguration";
 import { OktaConfiguration } from "src/models/oktaConfiguration";
 import { OneLoginConfiguration } from "src/models/oneLoginConfiguration";
+import { SyncConfiguration } from "src/models/syncConfiguration";
 
 import { LogService } from "jslib-common/abstractions/log.service";
+import { StateMigrationService } from "jslib-common/abstractions/stateMigration.service";
 import { StorageService } from "jslib-common/abstractions/storage.service";
+
 import { StateService as StateServiceAbstraction } from "src/abstractions/state.service";
+
 import { DirectoryType } from "src/enums/directoryType";
-import { SyncConfiguration } from "src/models/syncConfiguration";
-import { StateMigrationService } from "./stateMigration.service";
 
 const SecureStorageKeys = {
   ldap: "ldapPassword",
@@ -31,6 +33,12 @@ const SecureStorageKeys = {
   lastSyncHash: "lastSyncHash",
 };
 
+const keys = {
+  tempAccountSettings: "tempAccountSettings",
+  tempDirectoryConfigs: "tempDirectoryConfigs",
+  tempDirectorySettings: "tempDirectorySettings",
+};
+
 const StoredSecurely = "[STORED SECURELY]";
 
 export class StateService extends BaseStateService<Account> implements StateServiceAbstraction {
@@ -39,9 +47,10 @@ export class StateService extends BaseStateService<Account> implements StateServ
     protected secureStorageService: StorageService,
     protected logService: LogService,
     protected stateMigrationService: StateMigrationService,
-    private useSecureStorageForSecrets = true
+    private useSecureStorageForSecrets = true,
+    protected accountFactory: AccountFactory<Account>
   ) {
-    super(storageService, secureStorageService, logService, stateMigrationService);
+    super(storageService, secureStorageService, logService, stateMigrationService, accountFactory);
   }
 
   async getDirectory<T extends IConfiguration>(type: DirectoryType): Promise<T> {
@@ -520,19 +529,33 @@ export class StateService extends BaseStateService<Account> implements StateServ
   }
 
   protected async scaffoldNewAccountDiskStorage(account: Account): Promise<void> {
-    const storedState =
+    const storedAccount = await this.getAccount(
-      (await this.storageService.get<State<Account>>(
+      this.reconcileOptions(
-        "state",
+        { userId: account.profile.userId },
         await this.defaultOnDiskLocalOptions()
-      )) ?? new State<Account>();
+      )
-    const storedAccount = storedState.accounts[account.profile.userId];
+    );
     if (storedAccount != null) {
       account.settings = storedAccount.settings;
       account.directorySettings = storedAccount.directorySettings;
       account.directoryConfigurations = storedAccount.directoryConfigurations;
+    } else if (await this.hasTemporaryStorage()) {
+      // If migrating to state V2 with an no actively authed account we store temporary data to be copied on auth - this will only be run once.
+      account.settings = await this.storageService.get<any>(keys.tempAccountSettings);
+      account.directorySettings = await this.storageService.get<any>(keys.tempDirectorySettings);
+      account.directoryConfigurations = await this.storageService.get<any>(
+        keys.tempDirectoryConfigs
+      );
+      await this.storageService.remove(keys.tempAccountSettings);
+      await this.storageService.remove(keys.tempDirectorySettings);
+      await this.storageService.remove(keys.tempDirectoryConfigs);
     }
-    storedState.accounts[account.profile.userId] = account;
+
-    await this.saveStateToStorage(storedState, await this.defaultOnDiskLocalOptions());
+    await this.storageService.save(
+      account.profile.userId,
+      account,
+      await this.defaultOnDiskLocalOptions()
+    );
   }
 
   protected async pushAccounts(): Promise<void> {
@@ -542,4 +565,21 @@ export class StateService extends BaseStateService<Account> implements StateServ
     }
     this.accounts.next(this.state.accounts);
   }
+
+  protected async hasTemporaryStorage(): Promise<boolean> {
+    return (
+      (await this.storageService.has(keys.tempAccountSettings)) ||
+      (await this.storageService.has(keys.tempDirectorySettings)) ||
+      (await this.storageService.has(keys.tempDirectoryConfigs))
+    );
+  }
+
+  protected resetAccount(account: Account) {
+    const persistentAccountInformation = {
+      settings: account.settings,
+      directorySettings: account.directorySettings,
+      directoryConfigurations: account.directoryConfigurations,
+    };
+    return Object.assign(this.createAccount(), persistentAccountInformation);
+  }
 }

src/services/stateMigration.service.ts

@@ -1,6 +1,3 @@
-import { HtmlStorageLocation } from "jslib-common/enums/htmlStorageLocation";
-import { State } from "jslib-common/models/domain/state";
-
 import { StateMigrationService as BaseStateMigrationService } from "jslib-common/services/stateMigration.service";
 
 import { StateVersion } from "jslib-common/enums/stateVersion";
@@ -30,7 +27,6 @@ const SecureStorageKeys: { [key: string]: any } = {
 };
 
 const Keys: { [key: string]: any } = {
-  state: "state",
   entityId: "entityId",
   directoryType: "directoryType",
   organizationId: "organizationId",
@@ -39,6 +35,8 @@ const Keys: { [key: string]: any } = {
   lastSyncHash: "lastSyncHash",
   syncingDir: "syncingDir",
   syncConfig: "syncConfig",
+  tempDirectoryConfigs: "tempDirectoryConfigs",
+  tempDirectorySettings: "tempDirectorySettings",
 };
 
 const ClientKeys: { [key: string]: any } = {
@@ -50,8 +48,7 @@ const ClientKeys: { [key: string]: any } = {
 
 export class StateMigrationService extends BaseStateMigrationService {
   async migrate(): Promise<void> {
-    let currentStateVersion =
+    let currentStateVersion = await this.getCurrentStateVersion();
-      (await this.storageService.get<State<Account>>("state"))?.globals?.stateVersion ?? StateVersion.One;
     while (currentStateVersion < StateVersion.Latest) {
       switch (currentStateVersion) {
         case StateVersion.One:
@@ -80,73 +77,73 @@ export class StateMigrationService extends BaseStateMigrationService {
   }
 
   protected async migrateStateFrom1To2(useSecureStorageForSecrets: boolean = true): Promise<void> {
+    // Grabbing a couple of key settings before they get cleared by the base migration
+    const userId = await this.get<string>(Keys.entityId);
+    const clientId = await this.get<string>(ClientKeys.clientId);
+    const clientSecret = await this.get<string>(ClientKeys.clientSecret);
+
     await super.migrateStateFrom1To2();
-    const state = await this.storageService.get<State<Account>>(Keys.state);
-    const userId = await this.storageService.get<string>(Keys.entityId);
 
-    if (userId != null) {
+    // Setup reusable method for clearing keys since we will want to do that regardless of if there is an active authenticated session
-      state.accounts[userId] = new Account({
+    const clearDirectoryConnectorV1Keys = async () => {
-        directorySettings: {
+      for (const key in Keys) {
-          directoryType: await this.storageService.get<DirectoryType>(Keys.directoryType),
+        if (key == null) {
-          organizationId: await this.storageService.get<string>(Keys.organizationId),
+          continue;
-          lastUserSync: await this.storageService.get<Date>(Keys.lastUserSync),
+        }
-          lastGroupSync: await this.storageService.get<Date>(Keys.lastGroupSync),
+        for (const directoryType in DirectoryType) {
-          lastSyncHash: await this.storageService.get<string>(Keys.lastSyncHash),
+          if (directoryType == null) {
-          syncingDir: await this.storageService.get<boolean>(Keys.syncingDir),
+            continue;
-          sync: await this.storageService.get<SyncConfiguration>(Keys.syncConfig),
+          }
-        },
+          await this.set(SecureStorageKeys.directoryConfigPrefix + directoryType, null);
-        profile: {
-          entityId: await this.storageService.get<string>(Keys.entityId),
-        },
-        directoryConfigurations: new DirectoryConfigurations(),
-        clientKeys: {
-          clientId: await this.storageService.get<string>(ClientKeys.clientId),
-          clientSecret: await this.storageService.get<string>(ClientKeys.clientSecret),
-        },
-      });
-    }
-
-    for (const key in DirectoryType) {
-      if (await this.storageService.has(SecureStorageKeys.directoryConfigPrefix + key)) {
-        switch (+key) {
-          case DirectoryType.Ldap:
-            state.accounts[userId].directoryConfigurations.ldap =
-              await this.storageService.get<LdapConfiguration>(
-                SecureStorageKeys.directoryConfigPrefix + key
-              );
-            break;
-          case DirectoryType.GSuite:
-            state.accounts[userId].directoryConfigurations.gsuite =
-              await this.storageService.get<GSuiteConfiguration>(
-                SecureStorageKeys.directoryConfigPrefix + key
-              );
-            break;
-          case DirectoryType.AzureActiveDirectory:
-            state.accounts[userId].directoryConfigurations.azure =
-              await this.storageService.get<AzureConfiguration>(
-                SecureStorageKeys.directoryConfigPrefix + key
-              );
-            break;
-          case DirectoryType.Okta:
-            state.accounts[userId].directoryConfigurations.okta =
-              await this.storageService.get<OktaConfiguration>(
-                SecureStorageKeys.directoryConfigPrefix + key
-              );
-            break;
-          case DirectoryType.OneLogin:
-            state.accounts[userId].directoryConfigurations.oneLogin =
-              await this.storageService.get<OneLoginConfiguration>(
-                SecureStorageKeys.directoryConfigPrefix + key
-              );
-            break;
         }
-        await this.storageService.remove(SecureStorageKeys.directoryConfigPrefix + key);
       }
+    };
+
+    // Initilize typed objects from key/value pairs in storage to either be saved temporarily until an account is authed or applied to the active account
+    const getDirectoryConfig = async <T>(type: DirectoryType) =>
+      await this.get<T>(SecureStorageKeys.directoryConfigPrefix + type);
+    const directoryConfigs: DirectoryConfigurations = {
+      ldap: await getDirectoryConfig<LdapConfiguration>(DirectoryType.Ldap),
+      gsuite: await getDirectoryConfig<GSuiteConfiguration>(DirectoryType.GSuite),
+      azure: await getDirectoryConfig<AzureConfiguration>(DirectoryType.AzureActiveDirectory),
+      okta: await getDirectoryConfig<OktaConfiguration>(DirectoryType.Okta),
+      oneLogin: await getDirectoryConfig<OneLoginConfiguration>(DirectoryType.OneLogin),
+    };
+
+    const directorySettings: DirectorySettings = {
+      directoryType: await this.get<DirectoryType>(Keys.directoryType),
+      organizationId: await this.get<string>(Keys.organizationId),
+      lastUserSync: await this.get<Date>(Keys.lastUserSync),
+      lastGroupSync: await this.get<Date>(Keys.lastGroupSync),
+      lastSyncHash: await this.get<string>(Keys.lastSyncHash),
+      syncingDir: await this.get<boolean>(Keys.syncingDir),
+      sync: await this.get<SyncConfiguration>(Keys.syncConfig),
+    };
+
+    // (userId == null) = no authed account, stored data temporarily to be applied and cleared on next auth
+    // (userId != null) = authed account known, applied stored data to it and do not save temp data
+    if (userId == null) {
+      await this.set(Keys.tempDirectoryConfigs, directoryConfigs);
+      await this.set(Keys.tempDirectorySettings, directorySettings);
+      await clearDirectoryConnectorV1Keys();
+      return;
     }
 
-    state.globals.environmentUrls = await this.storageService.get("environmentUrls");
+    const account = await this.get<Account>(userId);
+    account.directoryConfigurations = directoryConfigs;
+    account.directorySettings = directorySettings;
+    account.profile = {
+      userId: userId,
+      entityId: userId,
+      apiKeyClientId: clientId,
+    };
+    account.clientKeys = {
+      clientId: clientId,
+      clientSecret: clientSecret,
+    };
 
-    await this.storageService.save("state", state);
+    await this.set(userId, account);
+    await clearDirectoryConnectorV1Keys();
 
     if (useSecureStorageForSecrets) {
       for (const key in SecureStorageKeys) {