From 1cbc119ad8c0447046624634dcd09262bb3f9b46 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Thu, 24 Feb 2022 13:08:43 +1000 Subject: [PATCH] [AuthService refactor] Don't clear state if 2FA is invalid (#690) * Don't clear state if 2FA is invalid * Add session timeout to 2FA * Clear internal authService state if unhandled error --- angular/src/services/jslib-services.module.ts | 1 + common/src/services/auth.service.ts | 41 +++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/angular/src/services/jslib-services.module.ts b/angular/src/services/jslib-services.module.ts index 54765078ea..f389b86069 100644 --- a/angular/src/services/jslib-services.module.ts +++ b/angular/src/services/jslib-services.module.ts @@ -119,6 +119,7 @@ import { ValidationService } from "./validation.service"; EnvironmentServiceAbstraction, StateServiceAbstraction, TwoFactorServiceAbstraction, + I18nServiceAbstraction, ], }, { diff --git a/common/src/services/auth.service.ts b/common/src/services/auth.service.ts index 44bf539fe8..06ad640ceb 100644 --- a/common/src/services/auth.service.ts +++ b/common/src/services/auth.service.ts @@ -3,6 +3,7 @@ import { AppIdService } from "../abstractions/appId.service"; import { AuthService as AuthServiceAbstraction } from "../abstractions/auth.service"; import { CryptoService } from "../abstractions/crypto.service"; import { EnvironmentService } from "../abstractions/environment.service"; +import { I18nService } from "../abstractions/i18n.service"; import { KeyConnectorService } from "../abstractions/keyConnector.service"; import { LogService } from "../abstractions/log.service"; import { MessagingService } from "../abstractions/messaging.service"; @@ -24,6 +25,9 @@ import { import { SymmetricCryptoKey } from "../models/domain/symmetricCryptoKey"; import { TokenRequestTwoFactor } from "../models/request/identityToken/tokenRequest"; import { PreloginRequest } from "../models/request/preloginRequest"; +import { ErrorResponse } from "../models/response/errorResponse"; + +const sessionTimeoutLength = 2 * 60 * 1000; // 2 minutes export class AuthService implements AuthServiceAbstraction { get email(): string { @@ -37,6 +41,7 @@ export class AuthService implements AuthServiceAbstraction { } private logInStrategy: ApiLogInStrategy | PasswordLogInStrategy | SsoLogInStrategy; + private sessionTimeout: any; constructor( protected cryptoService: CryptoService, @@ -49,7 +54,8 @@ export class AuthService implements AuthServiceAbstraction { protected keyConnectorService: KeyConnectorService, protected environmentService: EnvironmentService, protected stateService: StateService, - protected twoFactorService: TwoFactorService + protected twoFactorService: TwoFactorService, + protected i18nService: I18nService ) {} async logIn( @@ -110,10 +116,24 @@ export class AuthService implements AuthServiceAbstraction { } async logInTwoFactor(twoFactor: TokenRequestTwoFactor): Promise { + if (this.logInStrategy == null) { + throw new Error(this.i18nService.t("sessionTimeout")); + } + try { - return await this.logInStrategy.logInTwoFactor(twoFactor); - } finally { - this.clearState(); + const result = await this.logInStrategy.logInTwoFactor(twoFactor); + + // Only clear state if 2FA token has been accepted, otherwise we need to be able to try again + if (!result.requiresTwoFactor) { + this.clearState(); + } + return result; + } catch (e) { + // API exceptions are okay, but if there are any unhandled client-side errors then clear state to be safe + if (!(e instanceof ErrorResponse)) { + this.clearState(); + } + throw e; } } @@ -154,9 +174,22 @@ export class AuthService implements AuthServiceAbstraction { private saveState(strategy: ApiLogInStrategy | PasswordLogInStrategy | SsoLogInStrategy) { this.logInStrategy = strategy; + this.startSessionTimeout(); } private clearState() { this.logInStrategy = null; + this.clearSessionTimeout(); + } + + private startSessionTimeout() { + this.clearSessionTimeout(); + this.sessionTimeout = setTimeout(() => this.clearState(), sessionTimeoutLength); + } + + private clearSessionTimeout() { + if (this.sessionTimeout != null) { + clearTimeout(this.sessionTimeout); + } } }