From b7bb16c18a4bcd47988397bcf089fe6d895256b9 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 9 Feb 2022 17:01:43 -0500 Subject: [PATCH] [bug] Toggle tokens appropriatly based on timeout action (#661) --- common/src/abstractions/token.service.ts | 1 - common/src/services/state.service.ts | 108 ++++++++++++-------- common/src/services/token.service.ts | 34 ------ common/src/services/vaultTimeout.service.ts | 21 +++- 4 files changed, 84 insertions(+), 80 deletions(-) diff --git a/common/src/abstractions/token.service.ts b/common/src/abstractions/token.service.ts index 89aec53601..f04704942a 100644 --- a/common/src/abstractions/token.service.ts +++ b/common/src/abstractions/token.service.ts @@ -14,7 +14,6 @@ export abstract class TokenService { getClientId: () => Promise; setClientSecret: (clientSecret: string) => Promise; getClientSecret: () => Promise; - toggleTokens: () => Promise; setTwoFactorToken: (tokenResponse: IdentityTokenResponse) => Promise; getTwoFactorToken: () => Promise; clearTwoFactorToken: () => Promise; diff --git a/common/src/services/state.service.ts b/common/src/services/state.service.ts index 5c5be3b209..6dc69cf047 100644 --- a/common/src/services/state.service.ts +++ b/common/src/services/state.service.ts @@ -147,20 +147,23 @@ export class StateService< } async getAccessToken(options?: StorageOptions): Promise { - 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 { - 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 { @@ -195,37 +198,43 @@ export class StateService< } async getApiKeyClientId(options?: StorageOptions): Promise { - 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 { - 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 { - 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 { - 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 { @@ -1866,20 +1875,23 @@ export class StateService< } async getRefreshToken(options?: StorageOptions): Promise { - 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 { - 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 { @@ -2225,9 +2237,11 @@ export class StateService< } protected async scaffoldNewAccountStorage(account: TAccount): Promise { - 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(); diff --git a/common/src/services/token.service.ts b/common/src/services/token.service.ts index 052d67ae15..f1e328b073 100644 --- a/common/src/services/token.service.ts +++ b/common/src/services/token.service.ts @@ -22,9 +22,6 @@ export class TokenService implements TokenServiceAbstraction { } async setClientId(clientId: string): Promise { - 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 { - 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 { - 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 { - 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 { 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 { - const timeout = await this.stateService.getVaultTimeout(); - const action = await this.stateService.getVaultTimeoutAction(); - return timeout != null && action === "logOut"; - } } diff --git a/common/src/services/vaultTimeout.service.ts b/common/src/services/vaultTimeout.service.ts index e0fa81158d..682d1d7a56 100644 --- a/common/src/services/vaultTimeout.service.ts +++ b/common/src/services/vaultTimeout.service.ts @@ -122,9 +122,28 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { async setVaultTimeoutOptions(timeout: number, action: string): Promise { 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]> {