[PM-4917, PM-8707, PM-9119] Persist login email memory through 2fa on browser (#9811)

* persist email memory through 2fa on browser

* fix tests

* fix desktop
This commit is contained in:
Jake Fink 2024-07-11 14:51:06 -04:00 committed by GitHub
parent 4f3760beae
commit 9c66b5bf9f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 60 additions and 45 deletions

View File

@ -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();
}
}

View File

@ -52,7 +52,7 @@
</div>
</div>
<div class="box-footer">
<button type="button" class="btn link" routerLink="/hint" (click)="setLoginEmailValues()">
<button type="button" class="btn link" routerLink="/hint" (click)="saveEmailSettings()">
<b>{{ "getMasterPasswordHint" | i18n }}</b>
</button>
</div>

View File

@ -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;
}
}

View File

@ -99,7 +99,7 @@
class="btn block"
type="button"
routerLink="/accessibility-cookie"
(click)="setLoginEmailValues()"
(click)="saveEmailSettings()"
>
<i class="bwi bwi-universal-access" aria-hidden="true"></i>
{{ "loadAccessibilityCookie" | i18n }}
@ -139,7 +139,7 @@
type="button"
class="text text-primary password-hint-btn"
routerLink="/hint"
(click)="setLoginEmailValues()"
(click)="saveEmailSettings()"
>
{{ "getMasterPasswordHint" | i18n }}
</button>

View File

@ -75,7 +75,7 @@
class="tw-mt-2"
routerLink="/hint"
(mousedown)="goToHint()"
(click)="setLoginEmailValues()"
(click)="saveEmailSettings()"
>{{ "getMasterPasswordHint" | i18n }}</a
>
</div>

View File

@ -160,7 +160,7 @@ export class LoginComponent extends BaseLoginComponent implements OnInit {
}
async goToHint() {
this.setLoginEmailValues();
await this.saveEmailSettings();
await this.router.navigateByUrl("/hint");
}

View File

@ -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

View File

@ -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");
});
});
});

View File

@ -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();
}
}