[bug] Toggle tokens appropriatly based on timeout action (#661)

common/src/abstractions/token.service.ts

@@ -14,7 +14,6 @@ export abstract class TokenService {
   getClientId: () => Promise<string>;
   setClientSecret: (clientSecret: string) => Promise<any>;
   getClientSecret: () => Promise<string>;
-  toggleTokens: () => Promise<any>;
   setTwoFactorToken: (tokenResponse: IdentityTokenResponse) => Promise<any>;
   getTwoFactorToken: () => Promise<string>;
   clearTwoFactorToken: () => Promise<any>;

common/src/services/state.service.ts

@@ -147,20 +147,23 @@ export class StateService<
   }
 
   async getAccessToken(options?: StorageOptions): Promise<string> {
-    return (
-      await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
-    )?.tokens?.accessToken;
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    return (await this.getAccount(options))?.tokens?.accessToken;
   }
 
   async setAccessToken(value: string, options?: StorageOptions): Promise<void> {
-    const account = await this.getAccount(
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    const account = await this.getAccount(options);
     account.tokens.accessToken = value;
-    await this.saveAccount(
-      account,
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    await this.saveAccount(account, options);
   }
 
   async getAddEditCipherInfo(options?: StorageOptions): Promise<any> {
@@ -195,37 +198,43 @@ export class StateService<
   }
 
   async getApiKeyClientId(options?: StorageOptions): Promise<string> {
-    return (
-      await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
-    )?.profile?.apiKeyClientId;
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    return (await this.getAccount(options))?.profile?.apiKeyClientId;
   }
 
   async setApiKeyClientId(value: string, options?: StorageOptions): Promise<void> {
-    const account = await this.getAccount(
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    const account = await this.getAccount(options);
     account.profile.apiKeyClientId = value;
-    await this.saveAccount(
-      account,
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    await this.saveAccount(account, options);
   }
 
   async getApiKeyClientSecret(options?: StorageOptions): Promise<string> {
-    return (
-      await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
-    )?.keys?.apiKeyClientSecret;
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    return (await this.getAccount(options))?.keys?.apiKeyClientSecret;
   }
 
   async setApiKeyClientSecret(value: string, options?: StorageOptions): Promise<void> {
-    const account = await this.getAccount(
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    const account = await this.getAccount(options);
     account.keys.apiKeyClientSecret = value;
-    await this.saveAccount(
-      account,
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    await this.saveAccount(account, options);
   }
 
   async getAutoConfirmFingerPrints(options?: StorageOptions): Promise<boolean> {
@@ -1866,20 +1875,23 @@ export class StateService<
   }
 
   async getRefreshToken(options?: StorageOptions): Promise<string> {
-    return (
-      await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))
-    )?.tokens?.refreshToken;
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    return (await this.getAccount(options))?.tokens?.refreshToken;
   }
 
   async setRefreshToken(value: string, options?: StorageOptions): Promise<void> {
-    const account = await this.getAccount(
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    const defaultOptions =
+      (await this.getVaultTimeoutAction({ userId: options?.userId })) === "logOut"
+        ? this.defaultInMemoryOptions
+        : await this.defaultOnDiskOptions();
+    options = this.reconcileOptions(options, defaultOptions);
+    const account = await this.getAccount(options);
     account.tokens.refreshToken = value;
-    await this.saveAccount(
-      account,
-      this.reconcileOptions(options, await this.defaultOnDiskOptions())
-    );
+    await this.saveAccount(account, options);
   }
 
   async getRememberedEmail(options?: StorageOptions): Promise<string> {
@@ -2225,9 +2237,11 @@ export class StateService<
   }
 
   protected async scaffoldNewAccountStorage(account: TAccount): Promise<void> {
-    await this.scaffoldNewAccountLocalStorage(account);
-    await this.scaffoldNewAccountSessionStorage(account);
-    await this.scaffoldNewAccountMemoryStorage(account);
+    // We don't want to manipulate the referenced in memory account
+    const deepClone = JSON.parse(JSON.stringify(account));
+    await this.scaffoldNewAccountLocalStorage(deepClone);
+    await this.scaffoldNewAccountSessionStorage(deepClone);
+    await this.scaffoldNewAccountMemoryStorage(deepClone);
   }
 
   // TODO: There is a tech debt item for splitting up these methods - only Web uses multiple storage locations in its storageService.
@@ -2246,6 +2260,12 @@ export class StateService<
       await this.storageService.remove(keys.tempAccountSettings);
     }
     account.settings.environmentUrls = environmentUrls;
+    if (account.settings.vaultTimeoutAction === "logOut") {
+      account.tokens.accessToken = null;
+      account.tokens.refreshToken = null;
+      account.profile.apiKeyClientId = null;
+      account.keys.apiKeyClientSecret = null;
+    }
     await this.storageService.save(
       account.profile.userId,
       account,
@@ -2422,7 +2442,7 @@ export class StateService<
 
   protected clearDecryptedDataForActiveUser() {
     const userId = this.state.activeUserId;
-    if (userId == null) {
+    if (userId == null || this.state?.accounts[userId]?.data == null) {
       return;
     }
     this.state.accounts[userId].data = new AccountData();

common/src/services/token.service.ts

@@ -22,9 +22,6 @@ export class TokenService implements TokenServiceAbstraction {
   }
 
   async setClientId(clientId: string): Promise<any> {
-    if ((await this.skipTokenStorage()) || clientId == null) {
-      return;
-    }
     return await this.stateService.setApiKeyClientId(clientId);
   }
 
@@ -33,9 +30,6 @@ export class TokenService implements TokenServiceAbstraction {
   }
 
   async setClientSecret(clientSecret: string): Promise<any> {
-    if ((await this.skipTokenStorage()) || clientSecret == null) {
-      return;
-    }
     return await this.stateService.setApiKeyClientSecret(clientSecret);
   }
 
@@ -52,9 +46,6 @@ export class TokenService implements TokenServiceAbstraction {
   }
 
   async setRefreshToken(refreshToken: string): Promise<any> {
-    if (await this.skipTokenStorage()) {
-      return;
-    }
     return await this.stateService.setRefreshToken(refreshToken);
   }
 
@@ -62,25 +53,6 @@ export class TokenService implements TokenServiceAbstraction {
     return await this.stateService.getRefreshToken();
   }
 
-  async toggleTokens(): Promise<any> {
-    const token = await this.getToken();
-    const refreshToken = await this.getRefreshToken();
-    const clientId = await this.getClientId();
-    const clientSecret = await this.getClientSecret();
-    const timeout = await this.stateService.getVaultTimeout();
-    const action = await this.stateService.getVaultTimeoutAction();
-
-    if ((timeout != null || timeout === 0) && action === "logOut") {
-      // if we have a vault timeout and the action is log out, reset tokens
-      await this.clearToken();
-    }
-
-    await this.setToken(token);
-    await this.setRefreshToken(refreshToken);
-    await this.setClientId(clientId);
-    await this.setClientSecret(clientSecret);
-  }
-
   async setTwoFactorToken(tokenResponse: IdentityTokenResponse): Promise<any> {
     return await this.stateService.setTwoFactorToken(tokenResponse.twoFactorToken);
   }
@@ -214,10 +186,4 @@ export class TokenService implements TokenServiceAbstraction {
 
     return Array.isArray(decoded.amr) && decoded.amr.includes("external");
   }
-
-  private async skipTokenStorage(): Promise<boolean> {
-    const timeout = await this.stateService.getVaultTimeout();
-    const action = await this.stateService.getVaultTimeoutAction();
-    return timeout != null && action === "logOut";
-  }
 }

common/src/services/vaultTimeout.service.ts

@@ -122,9 +122,28 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
 
   async setVaultTimeoutOptions(timeout: number, action: string): Promise<void> {
     await this.stateService.setVaultTimeout(timeout);
+
+    // We swap these tokens from being on disk for lock actions, and in memory for logout actions
+    // Get them here to set them to their new location after changing the timeout action and clearing if needed
+    const token = await this.tokenService.getToken();
+    const refreshToken = await this.tokenService.getRefreshToken();
+    const clientId = await this.tokenService.getClientId();
+    const clientSecret = await this.tokenService.getClientSecret();
+
+    const currentAction = await this.stateService.getVaultTimeoutAction();
+    if ((timeout != null || timeout === 0) && action === "logOut" && action !== currentAction) {
+      // if we have a vault timeout and the action is log out, reset tokens
+      await this.tokenService.clearToken();
+    }
+
     await this.stateService.setVaultTimeoutAction(action);
+
+    await this.tokenService.setToken(token);
+    await this.tokenService.setRefreshToken(refreshToken);
+    await this.tokenService.setClientId(clientId);
+    await this.tokenService.setClientSecret(clientSecret);
+
     await this.cryptoService.toggleKey();
-    await this.tokenService.toggleTokens();
   }
 
   async isPinLockSet(): Promise<[boolean, boolean]> {