From ad58fc6086a6b9721b6da86573f25f512db41763 Mon Sep 17 00:00:00 2001 From: Alec Rippberger Date: Tue, 4 Mar 2025 16:54:47 -0600 Subject: [PATCH] Refactor service to use ViewCacheService --- .../popup/two-factor-options-v1.component.ts | 10 +-- .../auth/popup/two-factor-v1.component.html | 34 ++++++++-- .../src/auth/popup/two-factor-v1.component.ts | 22 ++++++- ...two-factor-auth-email-component.service.ts | 8 ++- ...extension-two-factor-form-cache.service.ts | 63 ++++++++++++++----- .../src/popup/services/services.module.ts | 14 ++--- ...o-factor-form-cache.service.abstraction.ts | 16 ++++- .../two-factor-auth-email.component.ts | 31 +++++---- .../two-factor-auth.component.ts | 4 +- 9 files changed, 151 insertions(+), 51 deletions(-) diff --git a/apps/browser/src/auth/popup/two-factor-options-v1.component.ts b/apps/browser/src/auth/popup/two-factor-options-v1.component.ts index e2c85b095e3..33c7126ee78 100644 --- a/apps/browser/src/auth/popup/two-factor-options-v1.component.ts +++ b/apps/browser/src/auth/popup/two-factor-options-v1.component.ts @@ -2,7 +2,7 @@ import { Component } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { TwoFactorOptionsComponentV1 as BaseTwoFactorOptionsComponent } from "@bitwarden/angular/auth/components/two-factor-options-v1.component"; -import { TwoFactorFormCacheServiceAbstraction } from "@bitwarden/auth/angular"; +import { TwoFactorFormCacheService } from "@bitwarden/auth/angular"; import { TwoFactorProviderDetails, TwoFactorService, @@ -23,7 +23,7 @@ export class TwoFactorOptionsComponentV1 extends BaseTwoFactorOptionsComponent { platformUtilsService: PlatformUtilsService, environmentService: EnvironmentService, private activatedRoute: ActivatedRoute, - private twoFactorFormCacheService: TwoFactorFormCacheServiceAbstraction, + private twoFactorFormCacheService: TwoFactorFormCacheService, ) { super(twoFactorService, router, i18nService, platformUtilsService, window, environmentService); } @@ -36,10 +36,10 @@ export class TwoFactorOptionsComponentV1 extends BaseTwoFactorOptionsComponent { await super.choose(p); await this.twoFactorService.setSelectedProvider(p.type); - const persistedData = await this.twoFactorFormCacheService.getFormData(); + // Clear the token when changing provider types await this.twoFactorFormCacheService.saveFormData({ - token: persistedData?.token || undefined, - remember: persistedData?.remember ?? undefined, + token: undefined, + remember: undefined, selectedProviderType: p.type, emailSent: false, }); diff --git a/apps/browser/src/auth/popup/two-factor-v1.component.html b/apps/browser/src/auth/popup/two-factor-v1.component.html index 126b0ea5a99..49071f3c44e 100644 --- a/apps/browser/src/auth/popup/two-factor-v1.component.html +++ b/apps/browser/src/auth/popup/two-factor-v1.component.html @@ -46,6 +46,7 @@ type="text" name="Code" [(ngModel)]="token" + (ngModelChange)="saveFormData()" required appAutofocus inputmode="tel" @@ -54,7 +55,13 @@
- +
@@ -74,6 +81,7 @@ type="password" name="Code" [(ngModel)]="token" + (ngModelChange)="saveFormData()" required appAutofocus appInputVerbatim @@ -81,7 +89,13 @@
- +
@@ -95,7 +109,13 @@
- +
@@ -132,7 +152,13 @@
- +
diff --git a/apps/browser/src/auth/popup/two-factor-v1.component.ts b/apps/browser/src/auth/popup/two-factor-v1.component.ts index 28abfada9c7..cb9ca0ed863 100644 --- a/apps/browser/src/auth/popup/two-factor-v1.component.ts +++ b/apps/browser/src/auth/popup/two-factor-v1.component.ts @@ -7,18 +7,19 @@ import { filter, first, takeUntil } from "rxjs/operators"; import { TwoFactorComponentV1 as BaseTwoFactorComponent } from "@bitwarden/angular/auth/components/two-factor-v1.component"; import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; +import { TwoFactorFormCacheService } from "@bitwarden/auth/angular"; import { LoginStrategyServiceAbstraction, LoginEmailServiceAbstraction, UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; -import { TwoFactorFormCacheServiceAbstraction } from "@bitwarden/auth/angular"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; @@ -68,7 +69,7 @@ export class TwoFactorComponentV1 extends BaseTwoFactorComponent implements OnIn toastService: ToastService, @Inject(WINDOW) protected win: Window, private browserMessagingApi: ZonedMessageListenerService, - private twoFactorFormCacheService: TwoFactorFormCacheServiceAbstraction, + private twoFactorFormCacheService: TwoFactorFormCacheService, ) { super( loginStrategyService, @@ -156,7 +157,8 @@ export class TwoFactorComponentV1 extends BaseTwoFactorComponent implements OnIn if ( this.selectedProviderType === TwoFactorProviderType.Email && - BrowserPopupUtils.inPopup(window) + BrowserPopupUtils.inPopup(window) && + !(await this.configService.getFeatureFlag(FeatureFlag.PM9115_TwoFactorFormPersistence)) ) { const confirmed = await this.dialogService.openSimpleDialog({ title: { key: "warning" }, @@ -302,4 +304,18 @@ export class TwoFactorComponentV1 extends BaseTwoFactorComponent implements OnIn // Clear cached data on successful login await this.twoFactorFormCacheService.clearFormData(); } + + /** + * Save the current form state when inputs change + */ + async saveFormData() { + if (this.twoFactorFormCacheService) { + await this.twoFactorFormCacheService.saveFormData({ + token: this.token, + remember: this.remember, + selectedProviderType: this.selectedProviderType, + emailSent: this.selectedProviderType === TwoFactorProviderType.Email, + }); + } + } } diff --git a/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.ts b/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.ts index 5d8d269412e..23febdef654 100644 --- a/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.ts +++ b/apps/browser/src/auth/services/extension-two-factor-auth-email-component.service.ts @@ -2,6 +2,8 @@ import { DefaultTwoFactorAuthEmailComponentService, TwoFactorAuthEmailComponentService, } from "@bitwarden/auth/angular"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { DialogService } from "@bitwarden/components"; import { openTwoFactorAuthEmailPopout } from "../../auth/popup/utils/auth-popout-window"; @@ -15,12 +17,16 @@ export class ExtensionTwoFactorAuthEmailComponentService constructor( private dialogService: DialogService, private window: Window, + private configService: ConfigService, ) { super(); } async openPopoutIfApprovedForEmail2fa(): Promise { - if (BrowserPopupUtils.inPopup(this.window)) { + const isTwoFactorFormPersistenceEnabled = await this.configService.getFeatureFlag( + FeatureFlag.PM9115_TwoFactorFormPersistence, + ); + if (BrowserPopupUtils.inPopup(this.window) && isTwoFactorFormPersistenceEnabled) { const confirmed = await this.dialogService.openSimpleDialog({ title: { key: "warning" }, content: { key: "popup2faCloseMessage" }, diff --git a/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts b/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts index dfc2b2a3f00..132191c5ad7 100644 --- a/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts +++ b/apps/browser/src/auth/services/extension-two-factor-form-cache.service.ts @@ -1,28 +1,53 @@ +import { Injectable, WritableSignal } from "@angular/core"; import { Observable, from, of, switchMap } from "rxjs"; -import { TwoFactorFormCacheServiceAbstraction } from "@bitwarden/auth/angular"; +import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; +import { TwoFactorFormCacheService, TwoFactorFormData } from "@bitwarden/auth/angular"; import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; -/** - * Interface for two-factor form data - */ -interface TwoFactorFormData { +const TWO_FACTOR_FORM_CACHE_KEY = "two-factor-form-cache"; + +// Utilize function overloading to create a type-safe deserializer to match the exact expected signature +function deserializeFormData(jsonValue: null): null; +function deserializeFormData(jsonValue: { token?: string; remember?: boolean; selectedProviderType?: TwoFactorProviderType; emailSent?: boolean; +}): TwoFactorFormData; +function deserializeFormData(jsonValue: any): TwoFactorFormData | null { + if (!jsonValue) { + return null; + } + + return { + token: jsonValue.token, + remember: jsonValue.remember, + selectedProviderType: jsonValue.selectedProviderType, + emailSent: jsonValue.emailSent, + }; } -const STORAGE_KEY = "twoFactorFormData"; +/** + * Service for caching two-factor form data + */ +@Injectable() +export class ExtensionTwoFactorFormCacheService extends TwoFactorFormCacheService { + private formDataCache: WritableSignal; -export class ExtensionTwoFactorFormCacheService implements TwoFactorFormCacheServiceAbstraction { constructor( - private storageService: AbstractStorageService, + private viewCacheService: ViewCacheService, private configService: ConfigService, - ) {} + ) { + super(); + this.formDataCache = this.viewCacheService.signal({ + key: TWO_FACTOR_FORM_CACHE_KEY, + initialValue: null, + deserializer: deserializeFormData, + }); + } isEnabled$(): Observable { return from(this.configService.getFeatureFlag(FeatureFlag.PM9115_TwoFactorFormPersistence)); @@ -38,28 +63,38 @@ export class ExtensionTwoFactorFormCacheService implements TwoFactorFormCacheSer if (!enabled) { return of(null); } - return from(this.storageService.get(STORAGE_KEY)); + return of(this.formDataCache()); }), ); } + /** + * Save form data to cache + */ async saveFormData(data: TwoFactorFormData): Promise { if (!(await this.isEnabled())) { return; } - await this.storageService.save(STORAGE_KEY, data); + // Set the new form data in the cache + this.formDataCache.set({ ...data }); } + /** + * Retrieve form data from cache + */ async getFormData(): Promise { if (!(await this.isEnabled())) { return null; } - return await this.storageService.get(STORAGE_KEY); + return this.formDataCache(); } + /** + * Clear form data from cache + */ async clearFormData(): Promise { - await this.storageService.remove(STORAGE_KEY); + this.formDataCache.set(null); } } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index b7980d83aa6..20ea0befe17 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -31,7 +31,8 @@ import { TwoFactorAuthDuoComponentService, TwoFactorAuthWebAuthnComponentService, SsoComponentService, - TwoFactorFormCacheServiceAbstraction } from "@bitwarden/auth/angular"; + TwoFactorFormCacheService, +} from "@bitwarden/auth/angular"; import { LockService, LoginEmailService, @@ -553,7 +554,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: TwoFactorAuthEmailComponentService, useClass: ExtensionTwoFactorAuthEmailComponentService, - deps: [DialogService, WINDOW], + deps: [DialogService, WINDOW, ConfigService], }), safeProvider({ provide: TwoFactorAuthWebAuthnComponentService, @@ -561,9 +562,9 @@ const safeProviders: SafeProvider[] = [ deps: [PlatformUtilsService], }), safeProvider({ - provide: TwoFactorFormCacheServiceAbstraction, + provide: TwoFactorFormCacheService, useClass: ExtensionTwoFactorFormCacheService, - deps: [AbstractStorageService, ConfigService], + deps: [PopupViewCacheService, ConfigService], }), safeProvider({ provide: TwoFactorAuthDuoComponentService, @@ -658,11 +659,6 @@ const safeProviders: SafeProvider[] = [ useClass: ExtensionLoginDecryptionOptionsService, deps: [MessagingServiceAbstraction, Router], }), - safeProvider({ - provide: TwoFactorFormCacheServiceAbstraction, - useClass: ExtensionTwoFactorFormCacheService, - deps: [AbstractStorageService, ConfigService], - }), ]; @NgModule({ diff --git a/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts b/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts index c7b8515111a..4fa386dd944 100644 --- a/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts +++ b/libs/auth/src/angular/two-factor-auth/abstractions/two-factor-form-cache.service.abstraction.ts @@ -1,9 +1,11 @@ +import { Observable } from "rxjs"; + import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; /** * Interface for two-factor form data */ -interface TwoFactorFormData { +export interface TwoFactorFormData { token?: string; remember?: boolean; selectedProviderType?: TwoFactorProviderType; @@ -13,12 +15,17 @@ interface TwoFactorFormData { /** * Abstract service for two-factor form caching */ -export abstract class TwoFactorFormCacheServiceAbstraction { +export abstract class TwoFactorFormCacheService { /** * Check if the form persistence feature is enabled */ abstract isEnabled(): Promise; + /** + * Observable that emits the current enabled state + */ + abstract isEnabled$(): Observable; + /** * Save form data to persistent storage */ @@ -29,6 +36,11 @@ export abstract class TwoFactorFormCacheServiceAbstraction { */ abstract getFormData(): Promise; + /** + * Observable that emits the current form data + */ + abstract formData$(): Observable; + /** * Clear form data from persistent storage */ diff --git a/libs/auth/src/angular/two-factor-auth/child-components/two-factor-auth-email/two-factor-auth-email.component.ts b/libs/auth/src/angular/two-factor-auth/child-components/two-factor-auth-email/two-factor-auth-email.component.ts index 7448a68e707..5b90c88b7e5 100644 --- a/libs/auth/src/angular/two-factor-auth/child-components/two-factor-auth-email/two-factor-auth-email.component.ts +++ b/libs/auth/src/angular/two-factor-auth/child-components/two-factor-auth-email/two-factor-auth-email.component.ts @@ -22,7 +22,8 @@ import { ToastService, } from "@bitwarden/components"; -import { TwoFactorFormCacheServiceAbstraction } from "../../abstractions/two-factor-form-cache.service.abstraction"; +import { TwoFactorFormCacheService } from "../../abstractions/two-factor-form-cache.service.abstraction"; + import { TwoFactorAuthEmailComponentService } from "./two-factor-auth-email-component.service"; @Component({ @@ -41,7 +42,6 @@ import { TwoFactorAuthEmailComponentService } from "./two-factor-auth-email-comp AsyncActionsModule, FormsModule, ], - providers: [], }) export class TwoFactorAuthEmailComponent implements OnInit { @Input({ required: true }) tokenFormControl: FormControl | undefined = undefined; @@ -60,7 +60,7 @@ export class TwoFactorAuthEmailComponent implements OnInit { protected appIdService: AppIdService, private toastService: ToastService, private twoFactorAuthEmailComponentService: TwoFactorAuthEmailComponentService, - private twoFactorFormCacheService: TwoFactorFormCacheServiceAbstraction, + private twoFactorFormCacheService: TwoFactorFormCacheService, ) {} async ngOnInit(): Promise { @@ -81,12 +81,17 @@ export class TwoFactorAuthEmailComponent implements OnInit { this.twoFactorEmail = email2faProviderData.Email; // Check if email has already been sent according to the cache - let cachedData; + let emailAlreadySent = false; if (this.twoFactorFormCacheService) { - cachedData = await this.twoFactorFormCacheService.getFormData(); + try { + const cachedData = await this.twoFactorFormCacheService.getFormData(); + emailAlreadySent = cachedData?.emailSent === true; + } catch (e) { + this.logService.error(e); + } } - if (providers.size > 1 && !cachedData?.emailSent) { + if (providers.size > 1 && !emailAlreadySent) { await this.sendEmail(false); } } @@ -129,11 +134,15 @@ export class TwoFactorAuthEmailComponent implements OnInit { // Update cache to indicate email was sent if (this.twoFactorFormCacheService) { - const cachedData = (await this.twoFactorFormCacheService.getFormData()) || {}; - await this.twoFactorFormCacheService.saveFormData({ - ...cachedData, - emailSent: true, - }); + try { + const cachedData = (await this.twoFactorFormCacheService.getFormData()) || {}; + await this.twoFactorFormCacheService.saveFormData({ + ...cachedData, + emailSent: true, + }); + } catch (e) { + this.logService.error(e); + } } if (doToast) { diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index 1e49f1297e5..7d452df7abb 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts @@ -54,8 +54,8 @@ import { TwoFactorAuthSecurityKeyIcon, TwoFactorAuthDuoIcon, } from "../icons/two-factor-auth"; -import { TwoFactorFormCacheServiceAbstraction } from "./abstractions/two-factor-form-cache.service.abstraction"; +import { TwoFactorFormCacheService } from "./abstractions"; import { TwoFactorAuthAuthenticatorComponent } from "./child-components/two-factor-auth-authenticator.component"; import { TwoFactorAuthDuoComponent } from "./child-components/two-factor-auth-duo/two-factor-auth-duo.component"; import { TwoFactorAuthEmailComponent } from "./child-components/two-factor-auth-email/two-factor-auth-email.component"; @@ -171,7 +171,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { private anonLayoutWrapperDataService: AnonLayoutWrapperDataService, private environmentService: EnvironmentService, private loginSuccessHandlerService: LoginSuccessHandlerService, - private twoFactorFormCacheService: TwoFactorFormCacheServiceAbstraction, + private twoFactorFormCacheService: TwoFactorFormCacheService, ) {} async ngOnInit() {