1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 13:23:34 +00:00

fix(login): [PM-11502] Support Remember Email option consistently

* Moved saving of SSO email outside of browser/desktop code

* Clarified comments.

* Tests

* Refactored login component services to manage state

* Fixed input on login component

* Fixed tests

* Linting

* Moved web setting in state into web override

* updated tests

* Fixed typing.

* Fixed type safety issues.

* Added comments and renamed for clarity.

* Removed method parameters that weren't used

* Added clarifying comments

* Added more comments.

* Removed test that is not necessary on base

* Test cleanup

* More comments.

* Linting

* Fixed test.

* Fixed base URL

* Fixed typechecking.

* Type checking

* Moved setting of email state to default service

* Added comments.

* Consolidated SSO URL formatting

* Updated comment

* Fixed reference.

* Fixed missing parameter.

* Initialized service.

* Added comments

* Added initialization of new service

* Made email optional due to CLI.

* Fixed comment on handleSsoClick.

* Added SSO email persistence to v1 component.

* Updated login email service.

* Updated setting of remember me

* Removed unnecessary input checking and rearranged functions

* Fixed name

* Added handling of Remember Email to old component for passkey click

* Updated v1 component to persist the email on Continue click

* Fix merge conflicts.

* Merge conflicts in login component.

* Persisted login email on v1 browser component.

* Merge conflicts

* fix(snap) [PM-17464][PM-17463][PM-15587] Allow Snap to use custom callback protocol

* Removed Snap from custom protocol workaround

* Fixed tests.

* Updated case numbers on test

* Resolved PR feedback.

* PM-11502 - LoginEmailSvcAbstraction - mark methods as abstract to satisfy strict ts.

* Removed test

* Changed to persist on leaving fields instead of button click.

* Fixed type checking.

---------

Co-authored-by: Bernd Schoolmann <mail@quexten.com>
Co-authored-by: Jared Snider <jsnider@bitwarden.com>
Co-authored-by: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com>
This commit is contained in:
Todd Martin
2025-04-10 18:58:49 -04:00
committed by GitHub
parent e88813e983
commit f7934b98c6
13 changed files with 241 additions and 295 deletions

View File

@@ -107,22 +107,17 @@ describe("DesktopLoginComponentService", () => {
}); });
describe("redirectToSso", () => { describe("redirectToSso", () => {
// Array of all permutations of isAppImage, isSnapStore, and isDev // Array of all permutations of isAppImage and isDev
const permutations = [ const permutations = [
[true, false, false], // Case 1: isAppImage true [true, false], // Case 1: isAppImage true
[false, true, false], // Case 2: isSnapStore true [false, true], // Case 2: isDev true
[false, false, true], // Case 3: isDev true [true, true], // Case 3: all true
[true, true, false], // Case 4: isAppImage and isSnapStore true [false, false], // Case 4: all false
[true, false, true], // Case 5: isAppImage and isDev true
[false, true, true], // Case 6: isSnapStore and isDev true
[true, true, true], // Case 7: all true
[false, false, false], // Case 8: all false
]; ];
permutations.forEach(([isAppImage, isSnapStore, isDev]) => { permutations.forEach(([isAppImage, isDev]) => {
it(`executes correct logic for isAppImage=${isAppImage}, isSnapStore=${isSnapStore}, isDev=${isDev}`, async () => { it(`executes correct logic for isAppImage=${isAppImage}, isDev=${isDev}`, async () => {
(global as any).ipc.platform.isAppImage = isAppImage; (global as any).ipc.platform.isAppImage = isAppImage;
(global as any).ipc.platform.isSnapStore = isSnapStore;
(global as any).ipc.platform.isDev = isDev; (global as any).ipc.platform.isDev = isDev;
const email = "test@bitwarden.com"; const email = "test@bitwarden.com";
@@ -136,7 +131,7 @@ describe("DesktopLoginComponentService", () => {
await service.redirectToSsoLogin(email); await service.redirectToSsoLogin(email);
if (isAppImage || isSnapStore || isDev) { if (isAppImage || isDev) {
expect(ipc.platform.localhostCallbackService.openSsoPrompt).toHaveBeenCalledWith( expect(ipc.platform.localhostCallbackService.openSsoPrompt).toHaveBeenCalledWith(
codeChallenge, codeChallenge,
state, state,

View File

@@ -51,7 +51,7 @@ export class DesktopLoginComponentService
): Promise<void> { ): Promise<void> {
// For platforms that cannot support a protocol-based (e.g. bitwarden://) callback, we use a localhost callback // For platforms that cannot support a protocol-based (e.g. bitwarden://) callback, we use a localhost callback
// Otherwise, we launch the SSO component in a browser window and wait for the callback // Otherwise, we launch the SSO component in a browser window and wait for the callback
if (ipc.platform.isAppImage || ipc.platform.isSnapStore || ipc.platform.isDev) { if (ipc.platform.isAppImage || ipc.platform.isDev) {
await this.initiateSsoThroughLocalhostCallback(email, state, codeChallenge); await this.initiateSsoThroughLocalhostCallback(email, state, codeChallenge);
} else { } else {
const env = await firstValueFrom(this.environmentService.environment$); const env = await firstValueFrom(this.environmentService.environment$);

View File

@@ -1470,7 +1470,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({ safeProvider({
provide: LoginSuccessHandlerService, provide: LoginSuccessHandlerService,
useClass: DefaultLoginSuccessHandlerService, useClass: DefaultLoginSuccessHandlerService,
deps: [SyncService, UserAsymmetricKeysRegenerationService], deps: [SyncService, UserAsymmetricKeysRegenerationService, LoginEmailService],
}), }),
safeProvider({ safeProvider({
provide: TaskService, provide: TaskService,

View File

@@ -809,11 +809,6 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy {
} }
private async handleSuccessfulLoginNavigation(userId: UserId) { private async handleSuccessfulLoginNavigation(userId: UserId) {
if (this.flow === Flow.StandardAuthRequest) {
// Only need to set remembered email on standard login with auth req flow
await this.loginEmailService.saveEmailSettings();
}
await this.loginSuccessHandlerService.run(userId); await this.loginSuccessHandlerService.run(userId);
await this.router.navigate(["vault"]); await this.router.navigate(["vault"]);
} }

View File

@@ -27,7 +27,12 @@
<!-- Remember Email input --> <!-- Remember Email input -->
<bit-form-control> <bit-form-control>
<input type="checkbox" formControlName="rememberEmail" bitCheckbox /> <input
type="checkbox"
formControlName="rememberEmail"
(input)="onRememberEmailInput($event)"
bitCheckbox
/>
<bit-label>{{ "rememberEmail" | i18n }}</bit-label> <bit-label>{{ "rememberEmail" | i18n }}</bit-label>
</bit-form-control> </bit-form-control>
@@ -39,18 +44,18 @@
<div class="tw-text-center">{{ "or" | i18n }}</div> <div class="tw-text-center">{{ "or" | i18n }}</div>
<!-- Link to Login with Passkey page --> <!-- Button to Login with Passkey -->
<ng-container *ngIf="isLoginWithPasskeySupported()"> <ng-container *ngIf="isLoginWithPasskeySupported()">
<a <button
type="button"
bitButton bitButton
block block
linkType="primary" buttonType="secondary"
routerLink="/login-with-passkey" (click)="handleLoginWithPasskeyClick()"
(mousedown)="$event.preventDefault()"
> >
<i class="bwi bwi-passkey tw-mr-1"></i> <i class="bwi bwi-passkey tw-mr-1"></i>
{{ "logInWithPasskey" | i18n }} {{ "logInWithPasskey" | i18n }}
</a> </button>
</ng-container> </ng-container>
<!-- Button to Login with SSO --> <!-- Button to Login with SSO -->

View File

@@ -148,6 +148,62 @@ export class LoginComponent implements OnInit, OnDestroy {
this.destroy$.complete(); this.destroy$.complete();
} }
private async defaultOnInit(): Promise<void> {
let paramEmailIsSet = false;
const params = await firstValueFrom(this.activatedRoute.queryParams);
if (params) {
const qParamsEmail = params.email;
// If there is an email in the query params, set that email as the form field value
if (qParamsEmail != null && qParamsEmail.indexOf("@") > -1) {
this.formGroup.controls.email.setValue(qParamsEmail);
paramEmailIsSet = true;
}
}
// If there are no params or no email in the query params, loadEmailSettings from state
if (!paramEmailIsSet) {
await this.loadRememberedEmail();
}
// Check to see if the device is known so that we can show the Login with Device option
if (this.emailFormControl.value) {
await this.getKnownDevice(this.emailFormControl.value);
}
// Backup check to handle unknown case where activatedRoute is not available
// This shouldn't happen under normal circumstances
if (!this.activatedRoute) {
await this.loadRememberedEmail();
}
}
private async desktopOnInit(): Promise<void> {
// TODO: refactor to not use deprecated broadcaster service.
this.broadcasterService.subscribe(BroadcasterSubscriptionId, async (message: any) => {
this.ngZone.run(() => {
switch (message.command) {
case "windowIsFocused":
if (this.deferFocus === null) {
this.deferFocus = !message.windowIsFocused;
if (!this.deferFocus) {
this.focusInput();
}
} else if (this.deferFocus && message.windowIsFocused) {
this.focusInput();
this.deferFocus = false;
}
break;
default:
}
});
});
this.messagingService.send("getWindowIsFocused");
}
submit = async (): Promise<void> => { submit = async (): Promise<void> => {
if (this.clientType === ClientType.Desktop) { if (this.clientType === ClientType.Desktop) {
if (this.loginUiState !== LoginUiState.MASTER_PASSWORD_ENTRY) { if (this.loginUiState !== LoginUiState.MASTER_PASSWORD_ENTRY) {
@@ -172,7 +228,6 @@ export class LoginComponent implements OnInit, OnDestroy {
try { try {
const authResult = await this.loginStrategyService.logIn(credentials); const authResult = await this.loginStrategyService.logIn(credentials);
await this.saveEmailSettings();
await this.handleAuthResult(authResult); await this.handleAuthResult(authResult);
} catch (error) { } catch (error) {
this.logService.error(error); this.logService.error(error);
@@ -250,7 +305,6 @@ export class LoginComponent implements OnInit, OnDestroy {
// User logged in successfully so execute side effects // User logged in successfully so execute side effects
await this.loginSuccessHandlerService.run(authResult.userId); await this.loginSuccessHandlerService.run(authResult.userId);
this.loginEmailService.clearValues();
// Determine where to send the user next // Determine where to send the user next
if (authResult.forcePasswordReset != ForceSetPasswordReason.None) { if (authResult.forcePasswordReset != ForceSetPasswordReason.None) {
@@ -288,7 +342,6 @@ export class LoginComponent implements OnInit, OnDestroy {
await this.router.navigate(["vault"]); await this.router.navigate(["vault"]);
} }
} }
/** /**
* Checks if the master password meets the enforced policy requirements * Checks if the master password meets the enforced policy requirements
* and if the user is required to change their password. * and if the user is required to change their password.
@@ -344,11 +397,10 @@ export class LoginComponent implements OnInit, OnDestroy {
return; return;
} }
await this.saveEmailSettings();
await this.router.navigate(["/login-with-device"]); await this.router.navigate(["/login-with-device"]);
} }
protected async validateEmail(): Promise<boolean> { protected async emailIsValid(): Promise<boolean> {
this.formGroup.controls.email.markAsTouched(); this.formGroup.controls.email.markAsTouched();
this.formGroup.controls.email.updateValueAndValidity({ onlySelf: true, emitEvent: true }); this.formGroup.controls.email.updateValueAndValidity({ onlySelf: true, emitEvent: true });
return this.formGroup.controls.email.valid; return this.formGroup.controls.email.valid;
@@ -399,37 +451,14 @@ export class LoginComponent implements OnInit, OnDestroy {
} }
} }
/**
* Set the email value from the input field.
* @param event The event object from the input field.
*/
onEmailInput(event: Event) {
const emailInput = event.target as HTMLInputElement;
this.formGroup.controls.email.setValue(emailInput.value);
this.loginEmailService.setLoginEmail(emailInput.value);
}
isLoginWithPasskeySupported() { isLoginWithPasskeySupported() {
return this.loginComponentService.isLoginWithPasskeySupported(); return this.loginComponentService.isLoginWithPasskeySupported();
} }
protected async goToHint(): Promise<void> { protected async goToHint(): Promise<void> {
await this.saveEmailSettings();
await this.router.navigateByUrl("/hint"); await this.router.navigateByUrl("/hint");
} }
protected async saveEmailSettings(): Promise<void> {
const email = this.formGroup.value.email;
if (!email) {
this.logService.error("Email is required to save email settings.");
return;
}
await this.loginEmailService.setLoginEmail(email);
this.loginEmailService.setRememberEmail(this.formGroup.value.rememberEmail ?? false);
await this.loginEmailService.saveEmailSettings();
}
/** /**
* Continue button clicked (or enter key pressed). * Continue button clicked (or enter key pressed).
* Adds the login url to the browser's history so that the back button can be used to go back to the email entry state. * Adds the login url to the browser's history so that the back button can be used to go back to the email entry state.
@@ -445,13 +474,44 @@ export class LoginComponent implements OnInit, OnDestroy {
* Continue to the master password entry state (only if email is validated) * Continue to the master password entry state (only if email is validated)
*/ */
protected async continue(): Promise<void> { protected async continue(): Promise<void> {
const isEmailValid = await this.validateEmail(); const isEmailValid = await this.emailIsValid();
if (isEmailValid) { if (isEmailValid) {
await this.toggleLoginUiState(LoginUiState.MASTER_PASSWORD_ENTRY); await this.toggleLoginUiState(LoginUiState.MASTER_PASSWORD_ENTRY);
} }
} }
/**
* Handle the Login with Passkey button click.
* We need a handler here in order to persist the remember email selection to state before routing.
* @param event - The event object.
*/
async handleLoginWithPasskeyClick() {
await this.router.navigate(["/login-with-passkey"]);
}
/**
* Handle the SSO button click.
* @param event - The event object.
*/
async handleSsoClick() {
// Make sure the email is valid
const isEmailValid = await this.emailIsValid();
if (!isEmailValid) {
return;
}
// Make sure the email is not empty, for type safety
const email = this.formGroup.value.email;
if (!email) {
this.logService.error("Email is required for SSO");
return;
}
// Send the user to SSO, either through routing or through redirecting to the web app
await this.loginComponentService.redirectToSsoLogin(email);
}
/** /**
* Call to check if the device is known. * Call to check if the device is known.
* Known means that the user has logged in with this device before. * Known means that the user has logged in with this device before.
@@ -473,23 +533,17 @@ export class LoginComponent implements OnInit, OnDestroy {
} }
} }
private async loadEmailSettings(): Promise<void> { /**
// Try to load the email from memory first * Check to see if the user has remembered an email on the current device.
const email = await firstValueFrom(this.loginEmailService.loginEmail$); * If so, set the email in the form field and set rememberEmail to true. If not, set rememberEmail to false.
const rememberEmail = this.loginEmailService.getRememberEmail(); */
private async loadRememberedEmail(): Promise<void> {
if (email) { const storedEmail = await firstValueFrom(this.loginEmailService.rememberedEmail$);
this.formGroup.controls.email.setValue(email); if (storedEmail) {
this.formGroup.controls.rememberEmail.setValue(rememberEmail); this.formGroup.controls.email.setValue(storedEmail);
this.formGroup.controls.rememberEmail.setValue(true);
} else { } else {
// If there is no email in memory, check for a storedEmail on disk this.formGroup.controls.rememberEmail.setValue(false);
const storedEmail = await firstValueFrom(this.loginEmailService.storedEmail$);
if (storedEmail) {
this.formGroup.controls.email.setValue(storedEmail);
// If there is a storedEmail, rememberEmail defaults to true
this.formGroup.controls.rememberEmail.setValue(true);
}
} }
} }
@@ -503,62 +557,6 @@ export class LoginComponent implements OnInit, OnDestroy {
?.focus(); ?.focus();
} }
private async defaultOnInit(): Promise<void> {
let paramEmailIsSet = false;
const params = await firstValueFrom(this.activatedRoute.queryParams);
if (params) {
const qParamsEmail = params.email;
// If there is an email in the query params, set that email as the form field value
if (qParamsEmail != null && qParamsEmail.indexOf("@") > -1) {
this.formGroup.controls.email.setValue(qParamsEmail);
paramEmailIsSet = true;
}
}
// If there are no params or no email in the query params, loadEmailSettings from state
if (!paramEmailIsSet) {
await this.loadEmailSettings();
}
// Check to see if the device is known so that we can show the Login with Device option
if (this.emailFormControl.value) {
await this.getKnownDevice(this.emailFormControl.value);
}
// Backup check to handle unknown case where activatedRoute is not available
// This shouldn't happen under normal circumstances
if (!this.activatedRoute) {
await this.loadEmailSettings();
}
}
private async desktopOnInit(): Promise<void> {
// TODO: refactor to not use deprecated broadcaster service.
this.broadcasterService.subscribe(BroadcasterSubscriptionId, async (message: any) => {
this.ngZone.run(() => {
switch (message.command) {
case "windowIsFocused":
if (this.deferFocus === null) {
this.deferFocus = !message.windowIsFocused;
if (!this.deferFocus) {
this.focusInput();
}
} else if (this.deferFocus && message.windowIsFocused) {
this.focusInput();
this.deferFocus = false;
}
break;
default:
}
});
});
this.messagingService.send("getWindowIsFocused");
}
/** /**
* Helper function to determine if the back button should be shown. * Helper function to determine if the back button should be shown.
* @returns true if the back button should be shown. * @returns true if the back button should be shown.
@@ -597,27 +595,46 @@ export class LoginComponent implements OnInit, OnDestroy {
}; };
/** /**
* Handle the SSO button click. * Persist the entered email address and the user's choice to remember it to state.
*/ */
async handleSsoClick() { private async persistEmailIfValid(): Promise<void> {
const email = this.formGroup.value.email; if (await this.emailIsValid()) {
const email = this.formGroup.value.email;
// Make sure the email is valid const rememberEmail = this.formGroup.value.rememberEmail ?? false;
const isEmailValid = await this.validateEmail(); if (!email) {
if (!isEmailValid) { return;
return; }
await this.loginEmailService.setLoginEmail(email);
await this.loginEmailService.setRememberedEmailChoice(email, rememberEmail);
} else {
await this.loginEmailService.clearLoginEmail();
await this.loginEmailService.clearRememberedEmail();
} }
}
// Make sure the email is not empty, for type safety /**
if (!email) { * Set the email value from the input field.
this.logService.error("Email is required for SSO"); * We only update the form controls onSubmit instead of onBlur because we don't want to show validation errors until
return; * the user submits. This is because currently our validation errors are shown below the input fields, and
} * displaying them causes the screen to "jump".
* @param event The event object from the input field.
*/
async onEmailInput(event: Event) {
const emailInput = event.target as HTMLInputElement;
this.formGroup.controls.email.setValue(emailInput.value);
await this.persistEmailIfValid();
}
// Save the email configuration for the login component /**
await this.saveEmailSettings(); * Set the Remember Email value from the input field.
* We only update the form controls onSubmit instead of onBlur because we don't want to show validation errors until
// Send the user to SSO, either through routing or through redirecting to the web app * the user submits. This is because currently our validation errors are shown below the input fields, and
await this.loginComponentService.redirectToSsoLogin(email); * displaying them causes the screen to "jump".
* @param event The event object from the input field.
*/
async onRememberEmailInput(event: Event) {
const rememberEmailInput = event.target as HTMLInputElement;
this.formGroup.controls.rememberEmail.setValue(rememberEmailInput.checked);
await this.persistEmailIfValid();
} }
} }

View File

@@ -5,10 +5,10 @@ import { Router } from "@angular/router";
import { Subject, takeUntil } from "rxjs"; import { Subject, takeUntil } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
import { LoginSuccessHandlerService } from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { import {
AsyncActionsModule, AsyncActionsModule,
ButtonModule, ButtonModule,
@@ -17,7 +17,6 @@ import {
LinkModule, LinkModule,
} from "@bitwarden/components"; } from "@bitwarden/components";
import { LoginEmailServiceAbstraction } from "../../common/abstractions/login-email.service";
import { LoginStrategyServiceAbstraction } from "../../common/abstractions/login-strategy.service"; import { LoginStrategyServiceAbstraction } from "../../common/abstractions/login-strategy.service";
/** /**
@@ -60,8 +59,7 @@ export class NewDeviceVerificationComponent implements OnInit, OnDestroy {
private loginStrategyService: LoginStrategyServiceAbstraction, private loginStrategyService: LoginStrategyServiceAbstraction,
private logService: LogService, private logService: LogService,
private i18nService: I18nService, private i18nService: I18nService,
private syncService: SyncService, private loginSuccessHandlerService: LoginSuccessHandlerService,
private loginEmailService: LoginEmailServiceAbstraction,
) {} ) {}
async ngOnInit() { async ngOnInit() {
@@ -143,9 +141,7 @@ export class NewDeviceVerificationComponent implements OnInit, OnDestroy {
return; return;
} }
this.loginEmailService.clearValues(); this.loginSuccessHandlerService.run(authResult.userId);
await this.syncService.fullSync(true);
// If verification succeeds, navigate to vault // If verification succeeds, navigate to vault
await this.router.navigate(["/vault"]); await this.router.navigate(["/vault"]);

View File

@@ -254,19 +254,6 @@ describe("TwoFactorAuthComponent", () => {
); );
}); });
it("calls loginEmailService.clearValues() when login is successful", async () => {
// Arrange
mockLoginStrategyService.logInTwoFactor.mockResolvedValue(new AuthResult());
// spy on loginEmailService.clearValues
const clearValuesSpy = jest.spyOn(mockLoginEmailService, "clearValues");
// Act
await component.submit(token, remember);
// Assert
expect(clearValuesSpy).toHaveBeenCalled();
});
describe("Set Master Password scenarios", () => { describe("Set Master Password scenarios", () => {
beforeEach(() => { beforeEach(() => {
const authResult = new AuthResult(); const authResult = new AuthResult();

View File

@@ -382,7 +382,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
// User is fully logged in so handle any post login logic before executing navigation // User is fully logged in so handle any post login logic before executing navigation
await this.loginSuccessHandlerService.run(authResult.userId); await this.loginSuccessHandlerService.run(authResult.userId);
this.loginEmailService.clearValues();
// Save off the OrgSsoIdentifier for use in the TDE flows // Save off the OrgSsoIdentifier for use in the TDE flows
// - TDE login decryption options component // - TDE login decryption options component

View File

@@ -1,43 +1,34 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Observable } from "rxjs"; import { Observable } from "rxjs";
export abstract class LoginEmailServiceAbstraction { export abstract class LoginEmailServiceAbstraction {
/** /**
* An observable that monitors the loginEmail in memory. * An observable that monitors the loginEmail.
* The loginEmail is the email that is being used in the current login process. * The loginEmail is the email that is being used in the current login process.
*/ */
loginEmail$: Observable<string | null>; abstract loginEmail$: Observable<string | null>;
/** /**
* An observable that monitors the storedEmail on disk. * An observable that monitors the remembered email.
* This will return null if an account is being added. * This will return null if an account is being added.
*/ */
storedEmail$: Observable<string | null>; abstract rememberedEmail$: Observable<string | null>;
/** /**
* Sets the loginEmail in memory. * Sets the loginEmail in memory.
* The loginEmail is the email that is being used in the current login process. * The loginEmail is the email that is being used in the current login process.
* Consumed through `loginEmail$` observable.
*/ */
setLoginEmail: (email: string) => Promise<void>; abstract setLoginEmail: (email: string) => Promise<void>;
/** /**
* Gets from memory whether or not the email should be stored on disk when `saveEmailSettings` is called. * Persist the user's choice of whether to remember their email on subsequent login attempts.
* @returns A boolean stating whether or not the email should be stored on disk. * Consumed through `rememberedEmail$` observable.
*/ */
getRememberEmail: () => boolean; abstract setRememberedEmailChoice: (email: string, remember: boolean) => Promise<void>;
/** /**
* Sets in memory whether or not the email should be stored on disk when `saveEmailSettings` is called. * Clears the in-progress login email, to be used after a successful login.
*/ */
setRememberEmail: (value: boolean) => void; abstract clearLoginEmail: () => Promise<void>;
/** /**
* Sets the email and rememberEmail properties in memory to null. * Clears the remembered email.
*/ */
clearValues: () => void; abstract clearRememberedEmail: () => Promise<void>;
/**
* Saves or clears the email on disk
* - If an account is being added, only changes the stored email when rememberEmail is true.
* - If rememberEmail is true, sets the email on disk to the current email.
* - If rememberEmail is false, sets the email on disk to null.
* Always clears the email and rememberEmail properties from memory.
* @returns A promise that resolves once the email settings are saved.
*/
saveEmailSettings: () => Promise<void>;
} }

View File

@@ -14,7 +14,7 @@ import { UserId } from "@bitwarden/common/types/guid";
import { LoginEmailService, STORED_EMAIL } from "./login-email.service"; import { LoginEmailService, STORED_EMAIL } from "./login-email.service";
describe("LoginEmailService", () => { describe("LoginEmailService", () => {
let sut: LoginEmailService; let service: LoginEmailService;
let accountService: FakeAccountService; let accountService: FakeAccountService;
let authService: MockProxy<AuthService>; let authService: MockProxy<AuthService>;
@@ -34,119 +34,93 @@ describe("LoginEmailService", () => {
mockAuthStatuses$ = new BehaviorSubject<Record<UserId, AuthenticationStatus>>({}); mockAuthStatuses$ = new BehaviorSubject<Record<UserId, AuthenticationStatus>>({});
authService.authStatuses$ = mockAuthStatuses$; authService.authStatuses$ = mockAuthStatuses$;
sut = new LoginEmailService(accountService, authService, stateProvider); service = new LoginEmailService(accountService, authService, stateProvider);
}); });
afterEach(() => { afterEach(() => {
jest.clearAllMocks(); jest.clearAllMocks();
}); });
describe("storedEmail$", () => { describe("rememberedEmail$", () => {
it("returns the stored email when not adding an account", async () => { it("returns the remembered email when not adding an account", async () => {
await sut.setLoginEmail("userEmail@bitwarden.com"); const testEmail = "test@bitwarden.com";
sut.setRememberEmail(true);
await sut.saveEmailSettings();
const result = await firstValueFrom(sut.storedEmail$); await service.setRememberedEmailChoice(testEmail, true);
expect(result).toEqual("userEmail@bitwarden.com"); const result = await firstValueFrom(service.rememberedEmail$);
expect(result).toEqual(testEmail);
}); });
it("returns the stored email when not adding an account and the user has just logged in", async () => { it("returns the remembered email when not adding an account and the user has just logged in", async () => {
await sut.setLoginEmail("userEmail@bitwarden.com"); const testEmail = "test@bitwarden.com";
sut.setRememberEmail(true);
await sut.saveEmailSettings(); await service.setRememberedEmailChoice(testEmail, true);
mockAuthStatuses$.next({ [userId]: AuthenticationStatus.Unlocked }); mockAuthStatuses$.next({ [userId]: AuthenticationStatus.Unlocked });
// account service already initialized with userId as active user // account service already initialized with userId as active user
const result = await firstValueFrom(sut.storedEmail$); const result = await firstValueFrom(service.rememberedEmail$);
expect(result).toEqual("userEmail@bitwarden.com"); expect(result).toEqual(testEmail);
}); });
it("returns null when adding an account", async () => { it("returns null when adding an account", async () => {
await sut.setLoginEmail("userEmail@bitwarden.com"); const testEmail = "test@bitwarden.com";
sut.setRememberEmail(true);
await sut.saveEmailSettings(); await service.setRememberedEmailChoice(testEmail, true);
mockAuthStatuses$.next({ mockAuthStatuses$.next({
[userId]: AuthenticationStatus.Unlocked, [userId]: AuthenticationStatus.Unlocked,
["OtherUserId" as UserId]: AuthenticationStatus.Locked, ["OtherUserId" as UserId]: AuthenticationStatus.Locked,
}); });
const result = await firstValueFrom(sut.storedEmail$); const result = await firstValueFrom(service.rememberedEmail$);
expect(result).toBeNull(); expect(result).toBeNull();
}); });
}); });
describe("saveEmailSettings", () => { describe("setRememberedEmailChoice", () => {
it("saves the email when not adding an account", async () => { it("sets the remembered email when remember is true", async () => {
await sut.setLoginEmail("userEmail@bitwarden.com"); const testEmail = "test@bitwarden.com";
sut.setRememberEmail(true);
await sut.saveEmailSettings(); await service.setRememberedEmailChoice(testEmail, true);
const result = await firstValueFrom(storedEmailState.state$); const result = await firstValueFrom(storedEmailState.state$);
expect(result).toEqual("userEmail@bitwarden.com"); expect(result).toEqual(testEmail);
}); });
it("clears the email when not adding an account and rememberEmail is false", async () => { it("clears the remembered email when remember is false", async () => {
storedEmailState.stateSubject.next("initialEmail@bitwarden.com"); storedEmailState.stateSubject.next("initialEmail@bitwarden.com");
await sut.setLoginEmail("userEmail@bitwarden.com"); const testEmail = "test@bitwarden.com";
sut.setRememberEmail(false);
await sut.saveEmailSettings(); await service.setRememberedEmailChoice(testEmail, false);
const result = await firstValueFrom(storedEmailState.state$); const result = await firstValueFrom(storedEmailState.state$);
expect(result).toBeNull(); expect(result).toBeNull();
}); });
});
it("saves the email when adding an account", async () => { describe("setLoginEmail", () => {
mockAuthStatuses$.next({ it("sets the login email", async () => {
[userId]: AuthenticationStatus.Unlocked, const testEmail = "test@bitwarden.com";
["OtherUserId" as UserId]: AuthenticationStatus.Locked, await service.setLoginEmail(testEmail);
});
await sut.setLoginEmail("userEmail@bitwarden.com"); expect(await firstValueFrom(service.loginEmail$)).toEqual(testEmail);
sut.setRememberEmail(true);
await sut.saveEmailSettings();
const result = await firstValueFrom(storedEmailState.state$);
expect(result).toEqual("userEmail@bitwarden.com");
}); });
});
it("does not clear the email when adding an account and rememberEmail is false", async () => { describe("clearLoginEmail", () => {
storedEmailState.stateSubject.next("initialEmail@bitwarden.com"); it("clears the login email", async () => {
const testEmail = "test@bitwarden.com";
await service.setLoginEmail(testEmail);
await service.clearLoginEmail();
mockAuthStatuses$.next({ expect(await firstValueFrom(service.loginEmail$)).toBeNull();
[userId]: AuthenticationStatus.Unlocked,
["OtherUserId" as UserId]: AuthenticationStatus.Locked,
});
await sut.setLoginEmail("userEmail@bitwarden.com");
sut.setRememberEmail(false);
await sut.saveEmailSettings();
const result = await firstValueFrom(storedEmailState.state$);
// result should not be null
expect(result).toEqual("initialEmail@bitwarden.com");
});
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.
await sut.setLoginEmail("userEmail@bitwarden.com");
sut.setRememberEmail(true);
await sut.saveEmailSettings();
const result = await firstValueFrom(sut.loginEmail$);
expect(result).toBe("userEmail@bitwarden.com");
}); });
}); });
}); });

View File

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Observable, firstValueFrom, switchMap } from "rxjs"; import { Observable, firstValueFrom, switchMap } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -26,8 +24,6 @@ export const STORED_EMAIL = new KeyDefinition<string>(LOGIN_EMAIL_DISK, "storedE
}); });
export class LoginEmailService implements LoginEmailServiceAbstraction { export class LoginEmailService implements LoginEmailServiceAbstraction {
private rememberEmail: boolean;
// True if an account is currently being added through account switching // True if an account is currently being added through account switching
private readonly addingAccount$: Observable<boolean>; private readonly addingAccount$: Observable<boolean>;
@@ -35,7 +31,7 @@ export class LoginEmailService implements LoginEmailServiceAbstraction {
loginEmail$: Observable<string | null>; loginEmail$: Observable<string | null>;
private readonly storedEmailState: GlobalState<string>; private readonly storedEmailState: GlobalState<string>;
storedEmail$: Observable<string | null>; rememberedEmail$: Observable<string | null>;
constructor( constructor(
private accountService: AccountService, private accountService: AccountService,
@@ -60,7 +56,7 @@ export class LoginEmailService implements LoginEmailServiceAbstraction {
this.loginEmail$ = this.loginEmailState.state$; this.loginEmail$ = this.loginEmailState.state$;
this.storedEmail$ = this.storedEmailState.state$.pipe( this.rememberedEmail$ = this.storedEmailState.state$.pipe(
switchMap(async (storedEmail) => { switchMap(async (storedEmail) => {
// When adding an account, we don't show the stored email // When adding an account, we don't show the stored email
if (await firstValueFrom(this.addingAccount$)) { if (await firstValueFrom(this.addingAccount$)) {
@@ -71,44 +67,32 @@ export class LoginEmailService implements LoginEmailServiceAbstraction {
); );
} }
/** Sets the login email in memory.
* The login email is the email that is being used in the current login process.
*/
async setLoginEmail(email: string) { async setLoginEmail(email: string) {
await this.loginEmailState.update((_) => email); await this.loginEmailState.update((_) => email);
} }
getRememberEmail() { /**
return this.rememberEmail; * Clears the in-progress login email from state.
* Note: Only clear on successful login or you are sure they are not needed.
* The extension client uses these values to maintain the email between login and 2fa components so
* we do not want to clear them too early.
*/
async clearLoginEmail() {
await this.loginEmailState.update((_) => null);
} }
setRememberEmail(value: boolean) { async setRememberedEmailChoice(email: string, remember: boolean): Promise<void> {
this.rememberEmail = value ?? false; if (remember) {
await this.storedEmailState.update((_) => email);
} else {
await this.storedEmailState.update((_) => null);
}
} }
// Note: only clear values on successful login or you are sure they are not needed. async clearRememberedEmail(): Promise<void> {
// Browser uses these values to maintain the email between login and 2fa components so await this.storedEmailState.update((_) => null);
// we do not want to clear them too early.
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 email;
}
return storedEmail;
}
// Saving with rememberEmail set to false will clear the stored email
if (this.rememberEmail) {
return email;
}
return null;
});
} }
} }

View File

@@ -3,14 +3,17 @@ import { UserId } from "@bitwarden/common/types/guid";
import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management";
import { LoginSuccessHandlerService } from "../../abstractions/login-success-handler.service"; import { LoginSuccessHandlerService } from "../../abstractions/login-success-handler.service";
import { LoginEmailService } from "../login-email/login-email.service";
export class DefaultLoginSuccessHandlerService implements LoginSuccessHandlerService { export class DefaultLoginSuccessHandlerService implements LoginSuccessHandlerService {
constructor( constructor(
private syncService: SyncService, private syncService: SyncService,
private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService,
private loginEmailService: LoginEmailService,
) {} ) {}
async run(userId: UserId): Promise<void> { async run(userId: UserId): Promise<void> {
await this.syncService.fullSync(true); await this.syncService.fullSync(true);
await this.userAsymmetricKeysRegenerationService.regenerateIfNeeded(userId); await this.userAsymmetricKeysRegenerationService.regenerateIfNeeded(userId);
await this.loginEmailService.clearLoginEmail();
} }
} }