From 69800d01ab8889f6604e033efdc0db9a73986aa2 Mon Sep 17 00:00:00 2001 From: Alec Rippberger <127791530+alec-livefront@users.noreply.github.com> Date: Fri, 6 Dec 2024 13:42:32 -0600 Subject: [PATCH] revert: [PR-13659] remove 2FA timeout logging and fix attempts This reverts two previous commits: - PM-13659 - 2FA Timeout Log All the things (#12275) - Auth/PM-13659 - 2FA Timeout - Attempted Fix (#12263) --- .../service-container/service-container.ts | 7 --- .../auth/components/two-factor.component.ts | 10 ---- libs/angular/src/services/injection-tokens.ts | 6 +-- .../src/services/jslib-services.module.ts | 9 +--- .../abstractions/login-strategy.service.ts | 2 - .../login-strategy.service.ts | 53 +++---------------- 6 files changed, 9 insertions(+), 78 deletions(-) diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index ce944db78f..21e8f9f208 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -17,7 +17,6 @@ import { PinService, PinServiceAbstraction, UserDecryptionOptionsService, - Executor, } from "@bitwarden/auth/common"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; import { EventUploadService as EventUploadServiceAbstraction } from "@bitwarden/common/abstractions/event/event-upload.service"; @@ -615,11 +614,6 @@ export class ServiceContainer { this.configService, ); - // Execute any authn session timeout logic without any wrapping logic. - // An executor is required to ensure the logic is executed in an Angular context when it - // it is available. - const authnSessionTimeoutExecutor: Executor = (fn) => fn(); - this.loginStrategyService = new LoginStrategyService( this.accountService, this.masterPasswordService, @@ -646,7 +640,6 @@ export class ServiceContainer { this.vaultTimeoutSettingsService, this.kdfConfigService, this.taskSchedulerService, - authnSessionTimeoutExecutor, ); // FIXME: CLI does not support autofill diff --git a/libs/angular/src/auth/components/two-factor.component.ts b/libs/angular/src/auth/components/two-factor.component.ts index f484ccd1e8..33269e28e9 100644 --- a/libs/angular/src/auth/components/two-factor.component.ts +++ b/libs/angular/src/auth/components/two-factor.component.ts @@ -102,20 +102,10 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI super(environmentService, i18nService, platformUtilsService, toastService); this.webAuthnSupported = this.platformUtilsService.supportsWebAuthn(win); - this.logService.info( - "Subscribing to timeout on LoginStrategyService with service id: " + - this.loginStrategyService.id, - ); - // Add subscription to twoFactorTimeout$ and navigate to twoFactorTimeoutRoute if expired this.loginStrategyService.twoFactorTimeout$ .pipe(takeUntilDestroyed()) .subscribe(async (expired) => { - this.logService.info( - "Received emission from LoginStrategyService.twoFactorTimeout$ with service id: " + - this.loginStrategyService.id, - ); - if (!expired) { return; } diff --git a/libs/angular/src/services/injection-tokens.ts b/libs/angular/src/services/injection-tokens.ts index 69a1ed3f61..86c5642a0c 100644 --- a/libs/angular/src/services/injection-tokens.ts +++ b/libs/angular/src/services/injection-tokens.ts @@ -1,7 +1,7 @@ import { InjectionToken } from "@angular/core"; import { Observable, Subject } from "rxjs"; -import { Executor, LogoutReason } from "@bitwarden/auth/common"; +import { LogoutReason } from "@bitwarden/auth/common"; import { ClientType } from "@bitwarden/common/enums"; import { RegionConfig } from "@bitwarden/common/platform/abstractions/environment.service"; import { @@ -68,7 +68,3 @@ export const REFRESH_ACCESS_TOKEN_ERROR_CALLBACK = new SafeInjectionToken<() => export const ENV_ADDITIONAL_REGIONS = new SafeInjectionToken( "ENV_ADDITIONAL_REGIONS", ); - -export const AUTHN_SESSION_TIMEOUT_EXECUTOR = new SafeInjectionToken( - "AuthnSessionTimeoutExecutor", -); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 8637e26a2b..a43f1fa07a 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1,4 +1,4 @@ -import { ErrorHandler, LOCALE_ID, NgModule, NgZone } from "@angular/core"; +import { ErrorHandler, LOCALE_ID, NgModule } from "@angular/core"; import { Subject } from "rxjs"; import { @@ -319,7 +319,6 @@ import { CLIENT_TYPE, REFRESH_ACCESS_TOKEN_ERROR_CALLBACK, ENV_ADDITIONAL_REGIONS, - AUTHN_SESSION_TIMEOUT_EXECUTOR, } from "./injection-tokens"; import { ModalService } from "./modal.service"; @@ -412,11 +411,6 @@ const safeProviders: SafeProvider[] = [ TokenServiceAbstraction, ], }), - safeProvider({ - provide: AUTHN_SESSION_TIMEOUT_EXECUTOR, - useFactory: (ngZone: NgZone) => (fn: () => void) => ngZone.run(fn), - deps: [NgZone], - }), safeProvider({ provide: LoginStrategyServiceAbstraction, useClass: LoginStrategyService, @@ -446,7 +440,6 @@ const safeProviders: SafeProvider[] = [ VaultTimeoutSettingsServiceAbstraction, KdfConfigService, TaskSchedulerService, - AUTHN_SESSION_TIMEOUT_EXECUTOR, ], }), safeProvider({ diff --git a/libs/auth/src/common/abstractions/login-strategy.service.ts b/libs/auth/src/common/abstractions/login-strategy.service.ts index 89e8491b27..e86cd6b0b0 100644 --- a/libs/auth/src/common/abstractions/login-strategy.service.ts +++ b/libs/auth/src/common/abstractions/login-strategy.service.ts @@ -14,8 +14,6 @@ import { } from "../models/domain/login-credentials"; export abstract class LoginStrategyServiceAbstraction { - id: string; - /** * The current strategy being used to authenticate. * Emits null if the session has timed out. diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts index 0c857cf7cc..99e3c057e1 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts @@ -7,7 +7,6 @@ import { shareReplay, Subscription, BehaviorSubject, - tap, } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -32,7 +31,6 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; import { TaskSchedulerService, ScheduledTaskNames } from "@bitwarden/common/platform/scheduling"; import { GlobalState, GlobalStateProvider } from "@bitwarden/common/platform/state"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/src/auth/abstractions/device-trust.service.abstraction"; @@ -73,8 +71,6 @@ import { const sessionTimeoutLength = 5 * 60 * 1000; // 5 minutes -export type Executor = (fn: () => void) => void; - export class LoginStrategyService implements LoginStrategyServiceAbstraction { private sessionTimeoutSubscription: Subscription; private currentAuthnTypeState: GlobalState; @@ -83,36 +79,7 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { private authRequestPushNotificationState: GlobalState; private twoFactorTimeoutSubject = new BehaviorSubject(false); - twoFactorTimeout$: Observable = this.twoFactorTimeoutSubject.asObservable().pipe( - // line 87 is the tap? - tap({ - next: (value) => { - this.logService.info( - `LoginStrategyService.twoFactorTimeout$ with service id: ${this.id} emmitted value: ${value}`, - ); - }, - error: (error: unknown) => { - this.logService.error( - `LoginStrategyService.twoFactorTimeout$ with service id: ${this.id} errored with error: ${JSON.stringify(error)}`, - ); - }, - finalize: () => { - this.logService.info( - `LoginStrategyService.twoFactorTimeout$ with service id: ${this.id} finalized`, - ); - }, - complete: () => { - this.logService.info( - `LoginStrategyService.twoFactorTimeout$ with service id: ${this.id} completed`, - ); - }, - subscribe: () => { - this.logService.info( - `LoginStrategyService.twoFactorTimeout$ with service id: ${this.id} subscribed`, - ); - }, - }), - ); + twoFactorTimeout$: Observable = this.twoFactorTimeoutSubject.asObservable(); private loginStrategy$: Observable< | UserApiLoginStrategy @@ -125,8 +92,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { currentAuthType$: Observable; - id: string = Utils.newGuid(); - constructor( protected accountService: AccountService, protected masterPasswordService: InternalMasterPasswordServiceAbstraction, @@ -153,7 +118,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { protected vaultTimeoutSettingsService: VaultTimeoutSettingsService, protected kdfConfigService: KdfConfigService, protected taskSchedulerService: TaskSchedulerService, - private authnSessionTimeoutExecutor: Executor = (fn) => fn(), // Default to no-op ) { this.currentAuthnTypeState = this.stateProvider.get(CURRENT_LOGIN_STRATEGY_KEY); this.loginStrategyCacheState = this.stateProvider.get(CACHE_KEY); @@ -164,15 +128,12 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { this.taskSchedulerService.registerTaskHandler( ScheduledTaskNames.loginStrategySessionTimeout, async () => { - this.logService.info("Timeout executing for LoginStrategyService with id: " + this.id); - this.authnSessionTimeoutExecutor(async () => { - this.twoFactorTimeoutSubject.next(true); - try { - await this.clearCache(); - } catch (e) { - this.logService.error("Failed to clear cache during session timeout", e); - } - }); + this.twoFactorTimeoutSubject.next(true); + try { + await this.clearCache(); + } catch (e) { + this.logService.error("Failed to clear cache during session timeout", e); + } }, );