diff --git a/apps/browser/src/auth/popup/home.component.ts b/apps/browser/src/auth/popup/home.component.ts index 505931ad0f1..cd9dfc3702b 100644 --- a/apps/browser/src/auth/popup/home.component.ts +++ b/apps/browser/src/auth/popup/home.component.ts @@ -41,7 +41,7 @@ export class HomeComponent implements OnInit, OnDestroy { ) {} async ngOnInit(): Promise { - const email = this.loginEmailService.getEmail(); + const email = await firstValueFrom(this.loginEmailService.loginEmail$); const rememberEmail = this.loginEmailService.getRememberEmail(); if (email != null) { @@ -93,7 +93,7 @@ export class HomeComponent implements OnInit, OnDestroy { async setLoginEmailValues() { // Note: Browser saves email settings here instead of the login component this.loginEmailService.setRememberEmail(this.formGroup.value.rememberEmail); - this.loginEmailService.setEmail(this.formGroup.value.email); + await this.loginEmailService.setLoginEmail(this.formGroup.value.email); await this.loginEmailService.saveEmailSettings(); } } diff --git a/apps/browser/src/auth/popup/login.component.ts b/apps/browser/src/auth/popup/login.component.ts index 6e73199969a..09bfdbbc240 100644 --- a/apps/browser/src/auth/popup/login.component.ts +++ b/apps/browser/src/auth/popup/login.component.ts @@ -1,4 +1,4 @@ -import { Component, NgZone } from "@angular/core"; +import { Component, NgZone, OnInit } from "@angular/core"; import { FormBuilder } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; import { firstValueFrom } from "rxjs"; @@ -31,7 +31,7 @@ import { flagEnabled } from "../../platform/flags"; selector: "app-login", templateUrl: "login.component.html", }) -export class LoginComponent extends BaseLoginComponent { +export class LoginComponent extends BaseLoginComponent implements OnInit { showPasswordless = false; constructor( devicesApiService: DevicesApiServiceAbstraction, @@ -83,13 +83,14 @@ export class LoginComponent extends BaseLoginComponent { }; super.successRoute = "/tabs/vault"; this.showPasswordless = flagEnabled("showPasswordless"); + } + async ngOnInit(): Promise { if (this.showPasswordless) { - this.formGroup.controls.email.setValue(this.loginEmailService.getEmail()); + const loginEmail = await firstValueFrom(this.loginEmailService.loginEmail$); + this.formGroup.controls.email.setValue(loginEmail); this.formGroup.controls.rememberEmail.setValue(this.loginEmailService.getRememberEmail()); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.validateEmail(); + await this.validateEmail(); } } diff --git a/apps/web/src/app/auth/hint.component.ts b/apps/web/src/app/auth/hint.component.ts index 42744546234..753bdb342f9 100644 --- a/apps/web/src/app/auth/hint.component.ts +++ b/apps/web/src/app/auth/hint.component.ts @@ -44,8 +44,8 @@ export class HintComponent extends BaseHintComponent implements OnInit { ); } - ngOnInit(): void { - super.ngOnInit(); + async ngOnInit(): Promise { + await super.ngOnInit(); this.emailFormControl.setValue(this.email); } diff --git a/libs/angular/src/auth/components/base-login-decryption-options.component.ts b/libs/angular/src/auth/components/base-login-decryption-options.component.ts index 80088bf7f91..6487c0cf847 100644 --- a/libs/angular/src/auth/components/base-login-decryption-options.component.ts +++ b/libs/angular/src/auth/components/base-login-decryption-options.component.ts @@ -251,12 +251,12 @@ export class BaseLoginDecryptionOptionsComponent implements OnInit, OnDestroy { return; } - this.loginEmailService.setEmail(this.data.userEmail); + this.loginEmailService.setLoginEmail(this.data.userEmail); await this.router.navigate(["/login-with-device"]); } async requestAdminApproval() { - this.loginEmailService.setEmail(this.data.userEmail); + this.loginEmailService.setLoginEmail(this.data.userEmail); await this.router.navigate(["/admin-approval-requested"]); } diff --git a/libs/angular/src/auth/components/hint.component.ts b/libs/angular/src/auth/components/hint.component.ts index 7a152efbb9f..f7ae1e4c182 100644 --- a/libs/angular/src/auth/components/hint.component.ts +++ b/libs/angular/src/auth/components/hint.component.ts @@ -1,5 +1,6 @@ import { Directive, OnInit } from "@angular/core"; import { Router } from "@angular/router"; +import { firstValueFrom } from "rxjs"; import { LoginEmailServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -27,8 +28,8 @@ export class HintComponent implements OnInit { protected toastService: ToastService, ) {} - ngOnInit(): void { - this.email = this.loginEmailService.getEmail() ?? ""; + async ngOnInit(): Promise { + this.email = (await firstValueFrom(this.loginEmailService.loginEmail$)) ?? ""; } async submit() { diff --git a/libs/angular/src/auth/components/login-via-auth-request.component.ts b/libs/angular/src/auth/components/login-via-auth-request.component.ts index 452b5ceee1e..a89952e024f 100644 --- a/libs/angular/src/auth/components/login-via-auth-request.component.ts +++ b/libs/angular/src/auth/components/login-via-auth-request.component.ts @@ -93,13 +93,6 @@ export class LoginViaAuthRequestComponent ) { super(environmentService, i18nService, platformUtilsService, toastService); - // TODO: I don't know why this is necessary. - // Why would the existence of the email depend on the navigation? - const navigation = this.router.getCurrentNavigation(); - if (navigation) { - this.email = this.loginEmailService.getEmail(); - } - // Gets signalR push notification // Only fires on approval to prevent enumeration this.authRequestService.authRequestPushNotification$ @@ -118,6 +111,7 @@ export class LoginViaAuthRequestComponent } async ngOnInit() { + this.email = await firstValueFrom(this.loginEmailService.loginEmail$); this.userAuthNStatus = await this.authService.getAuthStatus(); const matchOptions: IsActiveMatchOptions = { @@ -165,7 +159,7 @@ export class LoginViaAuthRequestComponent } else { // Standard auth request // TODO: evaluate if we can remove the setting of this.email in the constructor - this.email = this.loginEmailService.getEmail(); + this.email = await firstValueFrom(this.loginEmailService.loginEmail$); if (!this.email) { this.toastService.showToast({ diff --git a/libs/angular/src/auth/components/login.component.ts b/libs/angular/src/auth/components/login.component.ts index 501d753a976..3b927a05716 100644 --- a/libs/angular/src/auth/components/login.component.ts +++ b/libs/angular/src/auth/components/login.component.ts @@ -304,7 +304,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, private async loadEmailSettings() { // Try to load from memory first - const email = this.loginEmailService.getEmail(); + const email = await firstValueFrom(this.loginEmailService.loginEmail$); const rememberEmail = this.loginEmailService.getRememberEmail(); if (email) { this.formGroup.controls.email.setValue(email); @@ -321,7 +321,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, } protected async saveEmailSettings() { - this.loginEmailService.setEmail(this.formGroup.value.email); + this.loginEmailService.setLoginEmail(this.formGroup.value.email); this.loginEmailService.setRememberEmail(this.formGroup.value.rememberEmail); await this.loginEmailService.saveEmailSettings(); } diff --git a/libs/auth/src/common/abstractions/login-email.service.ts b/libs/auth/src/common/abstractions/login-email.service.ts index d4fbbaff840..496d890f162 100644 --- a/libs/auth/src/common/abstractions/login-email.service.ts +++ b/libs/auth/src/common/abstractions/login-email.service.ts @@ -1,29 +1,28 @@ import { Observable } from "rxjs"; export abstract class LoginEmailServiceAbstraction { + /** + * An observable that monitors the loginEmail in memory. + * The loginEmail is the email that is being used in the current login process. + */ + loginEmail$: Observable; /** * An observable that monitors the storedEmail on disk. * This will return null if an account is being added. */ storedEmail$: Observable; /** - * Gets the current email being used in the login process from memory. - * @returns A string of the email. + * Sets the loginEmail in memory. + * The loginEmail is the email that is being used in the current login process. */ - getEmail: () => string; - /** - * Sets the current email being used in the login process in memory. - * @param email The email to be set. - */ - setEmail: (email: string) => void; + setLoginEmail: (email: string) => Promise; /** * Gets from memory whether or not the email should be stored on disk when `saveEmailSettings` is called. * @returns A boolean stating whether or not the email should be stored on disk. */ getRememberEmail: () => boolean; /** - * Sets in memory whether or not the email should be stored on disk when - * `saveEmailSettings` is called. + * Sets in memory whether or not the email should be stored on disk when `saveEmailSettings` is called. */ setRememberEmail: (value: boolean) => void; /** diff --git a/libs/auth/src/common/services/login-email/login-email.service.spec.ts b/libs/auth/src/common/services/login-email/login-email.service.spec.ts index 55e54c82f6e..8bb9b962eaf 100644 --- a/libs/auth/src/common/services/login-email/login-email.service.spec.ts +++ b/libs/auth/src/common/services/login-email/login-email.service.spec.ts @@ -43,7 +43,7 @@ describe("LoginEmailService", () => { describe("storedEmail$", () => { it("returns the stored email when not adding an account", async () => { - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(true); await sut.saveEmailSettings(); @@ -53,7 +53,7 @@ describe("LoginEmailService", () => { }); it("returns the stored email when not adding an account and the user has just logged in", async () => { - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(true); await sut.saveEmailSettings(); @@ -66,7 +66,7 @@ describe("LoginEmailService", () => { }); it("returns null when adding an account", async () => { - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(true); await sut.saveEmailSettings(); @@ -83,7 +83,7 @@ describe("LoginEmailService", () => { describe("saveEmailSettings", () => { it("saves the email when not adding an account", async () => { - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(true); await sut.saveEmailSettings(); @@ -95,7 +95,7 @@ describe("LoginEmailService", () => { it("clears the email when not adding an account and rememberEmail is false", async () => { storedEmailState.stateSubject.next("initialEmail@bitwarden.com"); - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(false); await sut.saveEmailSettings(); @@ -110,7 +110,7 @@ describe("LoginEmailService", () => { ["OtherUserId" as UserId]: AuthenticationStatus.Locked, }); - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(true); await sut.saveEmailSettings(); @@ -127,7 +127,7 @@ describe("LoginEmailService", () => { ["OtherUserId" as UserId]: AuthenticationStatus.Locked, }); - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(false); await sut.saveEmailSettings(); @@ -140,11 +140,11 @@ describe("LoginEmailService", () => { it("does not clear the email and rememberEmail after saving", async () => { // Browser uses these values to maintain the email between login and 2fa components so // we do not want to clear them too early. - sut.setEmail("userEmail@bitwarden.com"); + await sut.setLoginEmail("userEmail@bitwarden.com"); sut.setRememberEmail(true); await sut.saveEmailSettings(); - const result = sut.getEmail(); + const result = await firstValueFrom(sut.loginEmail$); expect(result).toBe("userEmail@bitwarden.com"); }); diff --git a/libs/auth/src/common/services/login-email/login-email.service.ts b/libs/auth/src/common/services/login-email/login-email.service.ts index 7793d3e7ff6..bb89b412c51 100644 --- a/libs/auth/src/common/services/login-email/login-email.service.ts +++ b/libs/auth/src/common/services/login-email/login-email.service.ts @@ -8,21 +8,28 @@ import { GlobalState, KeyDefinition, LOGIN_EMAIL_DISK, + LOGIN_EMAIL_MEMORY, StateProvider, } from "../../../../../common/src/platform/state"; import { LoginEmailServiceAbstraction } from "../../abstractions/login-email.service"; +export const LOGIN_EMAIL = new KeyDefinition(LOGIN_EMAIL_MEMORY, "loginEmail", { + deserializer: (value: string) => value, +}); + export const STORED_EMAIL = new KeyDefinition(LOGIN_EMAIL_DISK, "storedEmail", { deserializer: (value: string) => value, }); export class LoginEmailService implements LoginEmailServiceAbstraction { - private email: string | null; private rememberEmail: boolean; // True if an account is currently being added through account switching private readonly addingAccount$: Observable; + private readonly loginEmailState: GlobalState; + loginEmail$: Observable; + private readonly storedEmailState: GlobalState; storedEmail$: Observable; @@ -31,6 +38,7 @@ export class LoginEmailService implements LoginEmailServiceAbstraction { private authService: AuthService, private stateProvider: StateProvider, ) { + this.loginEmailState = this.stateProvider.getGlobal(LOGIN_EMAIL); this.storedEmailState = this.stateProvider.getGlobal(STORED_EMAIL); // In order to determine if an account is being added, we check if any account is not logged out @@ -46,6 +54,8 @@ export class LoginEmailService implements LoginEmailServiceAbstraction { }), ); + this.loginEmail$ = this.loginEmailState.state$; + this.storedEmail$ = this.storedEmailState.state$.pipe( switchMap(async (storedEmail) => { // When adding an account, we don't show the stored email @@ -57,12 +67,8 @@ export class LoginEmailService implements LoginEmailServiceAbstraction { ); } - getEmail() { - return this.email; - } - - setEmail(email: string) { - this.email = email; + async setLoginEmail(email: string) { + await this.loginEmailState.update((_) => email); } getRememberEmail() { @@ -76,25 +82,27 @@ export class LoginEmailService implements LoginEmailServiceAbstraction { // Note: only clear values on successful login or you are sure they are not needed. // Browser uses these values to maintain the email between login and 2fa components so // we do not want to clear them too early. - clearValues() { - this.email = null; + async clearValues() { + await this.setLoginEmail(null); this.rememberEmail = false; } async saveEmailSettings() { const addingAccount = await firstValueFrom(this.addingAccount$); + const email = await firstValueFrom(this.loginEmail$); + await this.storedEmailState.update((storedEmail) => { // If we're adding an account, only overwrite the stored email when rememberEmail is true if (addingAccount) { if (this.rememberEmail) { - return this.email; + return email; } return storedEmail; } // Saving with rememberEmail set to false will clear the stored email if (this.rememberEmail) { - return this.email; + return email; } return null; }); diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index 32307203b29..47b7199b940 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -53,6 +53,7 @@ export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); export const LOGIN_EMAIL_DISK = new StateDefinition("loginEmail", "disk", { web: "disk-local", }); +export const LOGIN_EMAIL_MEMORY = new StateDefinition("loginEmail", "memory"); export const LOGIN_STRATEGY_MEMORY = new StateDefinition("loginStrategy", "memory"); export const MASTER_PASSWORD_DISK = new StateDefinition("masterPassword", "disk"); export const MASTER_PASSWORD_MEMORY = new StateDefinition("masterPassword", "memory");