diff --git a/apps/browser/src/auth/popup/home.component.ts b/apps/browser/src/auth/popup/home.component.ts index e553648d79..43f8f3dcf4 100644 --- a/apps/browser/src/auth/popup/home.component.ts +++ b/apps/browser/src/auth/popup/home.component.ts @@ -1,11 +1,10 @@ import { Component, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; import { Router } from "@angular/router"; -import { Subject, firstValueFrom, takeUntil } from "rxjs"; +import { Subject, firstValueFrom, switchMap, takeUntil } from "rxjs"; import { EnvironmentSelectorComponent } from "@bitwarden/angular/auth/components/environment-selector.component"; import { LoginEmailServiceAbstraction, RegisterRouteService } from "@bitwarden/auth/common"; -import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -34,7 +33,6 @@ export class HomeComponent implements OnInit, OnDestroy { private formBuilder: FormBuilder, private router: Router, private i18nService: I18nService, - private environmentService: EnvironmentService, private loginEmailService: LoginEmailServiceAbstraction, private accountSwitcherService: AccountSwitcherService, private registerRouteService: RegisterRouteService, @@ -55,13 +53,14 @@ export class HomeComponent implements OnInit, OnDestroy { } this.environmentSelector.onOpenSelfHostedSettings - .pipe(takeUntil(this.destroyed$)) - .subscribe(() => { - this.setLoginEmailValues(); - // 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.router.navigate(["environment"]); - }); + .pipe( + switchMap(async () => { + await this.setLoginEmailValues(); + await this.router.navigate(["environment"]); + }), + takeUntil(this.destroyed$), + ) + .subscribe(); } ngOnDestroy(): void { @@ -85,12 +84,14 @@ export class HomeComponent implements OnInit, OnDestroy { return; } - this.setLoginEmailValues(); + await this.setLoginEmailValues(); await this.router.navigate(["login"], { queryParams: { email: this.formGroup.value.email } }); } - setLoginEmailValues() { - this.loginEmailService.setEmail(this.formGroup.value.email); + 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.saveEmailSettings(); } } diff --git a/apps/browser/src/auth/popup/login.component.html b/apps/browser/src/auth/popup/login.component.html index 7a4211a5cc..9d2b4fccad 100644 --- a/apps/browser/src/auth/popup/login.component.html +++ b/apps/browser/src/auth/popup/login.component.html @@ -52,7 +52,7 @@ diff --git a/apps/browser/src/auth/popup/login.component.ts b/apps/browser/src/auth/popup/login.component.ts index 0d6c4c0f4a..79a02ede85 100644 --- a/apps/browser/src/auth/popup/login.component.ts +++ b/apps/browser/src/auth/popup/login.component.ts @@ -99,7 +99,7 @@ export class LoginComponent extends BaseLoginComponent { async launchSsoBrowser() { // Save off email for SSO await this.ssoLoginService.setSsoEmail(this.formGroup.value.email); - await this.loginEmailService.saveEmailSettings(); + // Generate necessary sso params const passwordOptions: any = { type: "password", @@ -142,4 +142,9 @@ export class LoginComponent extends BaseLoginComponent { encodeURIComponent(this.formGroup.controls.email.value), ); } + + async saveEmailSettings() { + // values should be saved on home component + return; + } } diff --git a/apps/desktop/src/auth/login/login.component.html b/apps/desktop/src/auth/login/login.component.html index e7f01accab..8a8611ad03 100644 --- a/apps/desktop/src/auth/login/login.component.html +++ b/apps/desktop/src/auth/login/login.component.html @@ -99,7 +99,7 @@ class="btn block" type="button" routerLink="/accessibility-cookie" - (click)="setLoginEmailValues()" + (click)="saveEmailSettings()" > {{ "loadAccessibilityCookie" | i18n }} @@ -139,7 +139,7 @@ type="button" class="text text-primary password-hint-btn" routerLink="/hint" - (click)="setLoginEmailValues()" + (click)="saveEmailSettings()" > {{ "getMasterPasswordHint" | i18n }} diff --git a/apps/web/src/app/auth/login/login.component.html b/apps/web/src/app/auth/login/login.component.html index 1ee2c8dd84..64f1e80a99 100644 --- a/apps/web/src/app/auth/login/login.component.html +++ b/apps/web/src/app/auth/login/login.component.html @@ -75,7 +75,7 @@ class="tw-mt-2" routerLink="/hint" (mousedown)="goToHint()" - (click)="setLoginEmailValues()" + (click)="saveEmailSettings()" >{{ "getMasterPasswordHint" | i18n }} diff --git a/apps/web/src/app/auth/login/login.component.ts b/apps/web/src/app/auth/login/login.component.ts index fb8eb1bea1..2ff81c02b4 100644 --- a/apps/web/src/app/auth/login/login.component.ts +++ b/apps/web/src/app/auth/login/login.component.ts @@ -160,7 +160,7 @@ export class LoginComponent extends BaseLoginComponent implements OnInit { } async goToHint() { - this.setLoginEmailValues(); + await this.saveEmailSettings(); await this.router.navigateByUrl("/hint"); } diff --git a/libs/angular/src/auth/components/login.component.ts b/libs/angular/src/auth/components/login.component.ts index dc2b8a43de..057d67b152 100644 --- a/libs/angular/src/auth/components/login.component.ts +++ b/libs/angular/src/auth/components/login.component.ts @@ -110,17 +110,8 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, }); if (!this.paramEmailSet) { - const storedEmail = await firstValueFrom(this.loginEmailService.storedEmail$); - this.formGroup.controls.email.setValue(storedEmail ?? ""); + await this.loadEmailSettings(); } - - let rememberEmail = this.loginEmailService.getRememberEmail(); - - if (!rememberEmail) { - rememberEmail = (await firstValueFrom(this.loginEmailService.storedEmail$)) != null; - } - - this.formGroup.controls.rememberEmail.setValue(rememberEmail); } ngOnDestroy() { @@ -158,8 +149,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, this.formPromise = this.loginStrategyService.logIn(credentials); const response = await this.formPromise; - this.setLoginEmailValues(); - await this.loginEmailService.saveEmailSettings(); + await this.saveEmailSettings(); if (this.handleCaptchaRequired(response)) { return; @@ -181,6 +171,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, // eslint-disable-next-line @typescript-eslint/no-floating-promises this.onSuccessfulLoginForceResetNavigate(); } else { + this.loginEmailService.clearValues(); // 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.router.navigate([this.forcePasswordResetRoute]); @@ -196,6 +187,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, // eslint-disable-next-line @typescript-eslint/no-floating-promises this.onSuccessfulLoginNavigate(); } else { + this.loginEmailService.clearValues(); // 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.router.navigate([this.successRoute]); @@ -225,12 +217,14 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, return; } - this.setLoginEmailValues(); + await this.saveEmailSettings(); await this.router.navigate(["/login-with-device"]); } async launchSsoBrowser(clientId: string, ssoRedirectUri: string) { - await this.saveEmailSettings(); + // Save off email for SSO + await this.ssoLoginService.setSsoEmail(this.formGroup.value.email); + // Generate necessary sso params const passwordOptions: any = { type: "password", @@ -301,17 +295,28 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, } } - setLoginEmailValues() { - this.loginEmailService.setEmail(this.formGroup.value.email); - this.loginEmailService.setRememberEmail(this.formGroup.value.rememberEmail); + private async loadEmailSettings() { + // Try to load from memory first + const email = this.loginEmailService.getEmail(); + const rememberEmail = this.loginEmailService.getRememberEmail(); + if (email) { + this.formGroup.controls.email.setValue(email); + this.formGroup.controls.rememberEmail.setValue(rememberEmail); + } else { + // If not in memory, check email on disk + const storedEmail = await firstValueFrom(this.loginEmailService.storedEmail$); + if (storedEmail) { + // If we have a stored email, rememberEmail should default to true + this.formGroup.controls.email.setValue(storedEmail); + this.formGroup.controls.rememberEmail.setValue(true); + } + } } - async saveEmailSettings() { - this.setLoginEmailValues(); + protected async saveEmailSettings() { + this.loginEmailService.setEmail(this.formGroup.value.email); + this.loginEmailService.setRememberEmail(this.formGroup.value.rememberEmail); await this.loginEmailService.saveEmailSettings(); - - // Save off email for SSO - await this.ssoLoginService.setSsoEmail(this.formGroup.value.email); } // Legacy accounts used the master key to encrypt data. Migration is required 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 190c9ecf16..55e54c82f6 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 @@ -137,14 +137,16 @@ describe("LoginEmailService", () => { expect(result).toEqual("initialEmail@bitwarden.com"); }); - it("clears the email and rememberEmail after saving", async () => { + 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"); sut.setRememberEmail(true); await sut.saveEmailSettings(); const result = sut.getEmail(); - expect(result).toBeNull(); + 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 3babc6f43c..7793d3e7ff 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 @@ -73,6 +73,9 @@ export class LoginEmailService implements LoginEmailServiceAbstraction { this.rememberEmail = value ?? false; } + // 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; this.rememberEmail = false; @@ -89,12 +92,11 @@ export class LoginEmailService implements LoginEmailServiceAbstraction { return storedEmail; } - // Logging in with rememberEmail set to false will clear the stored email + // Saving with rememberEmail set to false will clear the stored email if (this.rememberEmail) { return this.email; } return null; }); - this.clearValues(); } }