From c06c0f9f2c2cd29e1afd7e9866ca5c9d22e8c312 Mon Sep 17 00:00:00 2001 From: Robyn MacCallum Date: Fri, 16 Dec 2022 10:08:44 -0500 Subject: [PATCH] [SG-742] (#4190) * Remove default landing on masterpassword page * Remove rememberEmail state service value that isn't needed * Remove last occurence of setRememberEmail * Remove alwaysRememberEmail functionality * Remove always remember email from browser and add option to * Add extra spacing around remember email check * [SG-884] Fix Remember Email functionality for Login with SSO (#4238) * Add saveEmailSettings method to LoginService * Add StateService as a dependency to LoginService * Update login components to utilize new login service method for saving rememberedEmail --- .../src/popup/accounts/home.component.html | 19 ++++++++--- .../src/popup/accounts/home.component.ts | 33 +++++++++++++++---- .../src/popup/accounts/login.component.ts | 3 +- apps/browser/src/popup/scss/pages.scss | 6 ++++ .../src/popup/services/services.module.ts | 1 + .../src/app/services/services.module.ts | 1 + .../login/login-with-device.component.ts | 10 +----- .../app/accounts/login/login.component.html | 2 +- .../src/app/accounts/login/login.component.ts | 1 - apps/web/src/app/core/core.module.ts | 1 + apps/web/src/app/core/state/state.service.ts | 17 ---------- .../angular/src/components/login.component.ts | 30 +++++++---------- .../src/services/jslib-services.module.ts | 1 + libs/common/src/abstractions/login.service.ts | 1 + libs/common/src/services/login.service.ts | 8 +++++ 15 files changed, 75 insertions(+), 59 deletions(-) diff --git a/apps/browser/src/popup/accounts/home.component.html b/apps/browser/src/popup/accounts/home.component.html index 7fc8163a7f0..24feb9c83e5 100644 --- a/apps/browser/src/popup/accounts/home.component.html +++ b/apps/browser/src/popup/accounts/home.component.html @@ -9,9 +9,18 @@ - -
@@ -22,10 +31,10 @@
- diff --git a/apps/browser/src/popup/accounts/home.component.ts b/apps/browser/src/popup/accounts/home.component.ts index 28d42dc482a..e6a31bea9a6 100644 --- a/apps/browser/src/popup/accounts/home.component.ts +++ b/apps/browser/src/popup/accounts/home.component.ts @@ -4,6 +4,7 @@ import { ActivatedRoute, Router } from "@angular/router"; import { EnvironmentService } from "@bitwarden/common/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/abstractions/i18n.service"; +import { LoginService } from "@bitwarden/common/abstractions/login.service"; import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service"; import { StateService } from "@bitwarden/common/abstractions/state.service"; @@ -16,6 +17,7 @@ export class HomeComponent implements OnInit { formGroup = this.formBuilder.group({ email: ["", [Validators.required, Validators.email]], + rememberEmail: [false], }); constructor( @@ -25,12 +27,26 @@ export class HomeComponent implements OnInit { private router: Router, private i18nService: I18nService, private environmentService: EnvironmentService, - private route: ActivatedRoute + private route: ActivatedRoute, + private loginService: LoginService ) {} async ngOnInit(): Promise { - const rememberedEmail = await this.stateService.getRememberedEmail(); - if (rememberedEmail != null) { - this.formGroup.patchValue({ email: await this.stateService.getRememberedEmail() }); + let savedEmail = this.loginService.getEmail(); + const rememberEmail = this.loginService.getRememberEmail(); + + if (savedEmail != null) { + this.formGroup.patchValue({ + email: savedEmail, + rememberEmail: rememberEmail, + }); + } else { + savedEmail = await this.stateService.getRememberedEmail(); + if (savedEmail != null) { + this.formGroup.patchValue({ + email: savedEmail, + rememberEmail: true, + }); + } } } @@ -45,12 +61,17 @@ export class HomeComponent implements OnInit { return; } - this.stateService.setRememberedEmail(this.formGroup.value.email); - + this.loginService.setEmail(this.formGroup.value.email); + this.loginService.setRememberEmail(this.formGroup.value.rememberEmail); this.router.navigate(["login"], { queryParams: { email: this.formGroup.value.email } }); } get selfHostedDomain() { return this.environmentService.hasBaseUrl() ? this.environmentService.getWebVaultUrl() : null; } + + setFormValues() { + this.loginService.setEmail(this.formGroup.value.email); + this.loginService.setRememberEmail(this.formGroup.value.rememberEmail); + } } diff --git a/apps/browser/src/popup/accounts/login.component.ts b/apps/browser/src/popup/accounts/login.component.ts index d44847b35e0..7630f31ac3d 100644 --- a/apps/browser/src/popup/accounts/login.component.ts +++ b/apps/browser/src/popup/accounts/login.component.ts @@ -23,8 +23,6 @@ import { Utils } from "@bitwarden/common/misc/utils"; templateUrl: "login.component.html", }) export class LoginComponent extends BaseLoginComponent { - protected skipRememberEmail = true; - constructor( apiService: ApiService, appIdService: AppIdService, @@ -73,6 +71,7 @@ export class LoginComponent extends BaseLoginComponent { } async launchSsoBrowser() { + await this.loginService.saveEmailSettings(); // Generate necessary sso params const passwordOptions: any = { type: "password", diff --git a/apps/browser/src/popup/scss/pages.scss b/apps/browser/src/popup/scss/pages.scss index 9ba21f63ca7..66bd190a5fc 100644 --- a/apps/browser/src/popup/scss/pages.scss +++ b/apps/browser/src/popup/scss/pages.scss @@ -143,6 +143,12 @@ body.body-full { padding: 30px 10px 0 10px; } +.remember-email-check { + padding-top: 8px; + padding-left: 10px; + padding-bottom: 18px; +} + .login-buttons > button { margin: 15px 0 15px 0; } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index b2039ba5996..05cd4ede8f5 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -367,6 +367,7 @@ function getBgService(service: keyof MainBackground) { { provide: LoginServiceAbstraction, useClass: LoginService, + deps: [StateServiceAbstraction], }, { provide: AbstractThemingService, diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 8696bd0b395..28d27a9ce22 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -180,6 +180,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); { provide: LoginServiceAbstraction, useClass: LoginService, + deps: [StateServiceAbstraction], }, ], }) diff --git a/apps/web/src/app/accounts/login/login-with-device.component.ts b/apps/web/src/app/accounts/login/login-with-device.component.ts index fabc2552208..82852000832 100644 --- a/apps/web/src/app/accounts/login/login-with-device.component.ts +++ b/apps/web/src/app/accounts/login/login-with-device.component.ts @@ -142,7 +142,7 @@ export class LoginWithDeviceComponent this.router.navigate([this.forcePasswordResetRoute]); } } else { - await this.setRememberEmailValues(); + await this.loginService.saveEmailSettings(); if (this.onSuccessfulLogin != null) { this.onSuccessfulLogin(); } @@ -202,12 +202,4 @@ export class LoginWithDeviceComponent localHashedPassword ); } - - private async setRememberEmailValues() { - const rememberEmail = this.loginService.getRememberEmail(); - const rememberedEmail = this.loginService.getEmail(); - await this.stateService.setRememberEmail(rememberEmail); - await this.stateService.setRememberedEmail(rememberEmail ? rememberedEmail : null); - this.loginService.clearValues(); - } } diff --git a/apps/web/src/app/accounts/login/login.component.html b/apps/web/src/app/accounts/login/login.component.html index 8202af0dfd8..b9529295671 100644 --- a/apps/web/src/app/accounts/login/login.component.html +++ b/apps/web/src/app/accounts/login/login.component.html @@ -120,7 +120,7 @@
{ await super.addAccount(account); } - async getRememberEmail(options?: StorageOptions) { - return ( - await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions())) - )?.rememberEmail; - } - - async setRememberEmail(value: boolean, options?: StorageOptions): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()) - ); - globals.rememberEmail = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()) - ); - } - async getEncryptedCiphers(options?: StorageOptions): Promise<{ [id: string]: CipherData }> { options = this.reconcileOptions(options, await this.defaultInMemoryOptions()); return await super.getEncryptedCiphers(options); diff --git a/libs/angular/src/components/login.component.ts b/libs/angular/src/components/login.component.ts index 0b35b827afc..0f9f15a0568 100644 --- a/libs/angular/src/components/login.component.ts +++ b/libs/angular/src/components/login.component.ts @@ -46,8 +46,6 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit protected twoFactorRoute = "2fa"; protected successRoute = "vault"; protected forcePasswordResetRoute = "update-temp-password"; - protected alwaysRememberEmail = false; - protected skipRememberEmail = false; get loggedEmail() { return this.formGroup.value.email; @@ -85,6 +83,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit const queryParamsEmail = params["email"]; if (queryParamsEmail != null && queryParamsEmail.indexOf("@") > -1) { this.formGroup.get("email").setValue(queryParamsEmail); + this.loginService.setEmail(queryParamsEmail); this.paramEmailSet = true; } } @@ -98,17 +97,11 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit if (!this.paramEmailSet) { this.formGroup.get("email")?.setValue(email ?? ""); } - if (!this.alwaysRememberEmail) { - let rememberEmail = this.loginService.getRememberEmail(); - if (rememberEmail == null) { - rememberEmail = (await this.stateService.getRememberedEmail()) != null; - } - this.formGroup.get("rememberEmail")?.setValue(rememberEmail); - } - - if (email) { - this.validateEmail(); + let rememberEmail = this.loginService.getRememberEmail(); + if (rememberEmail == null) { + rememberEmail = (await this.stateService.getRememberedEmail()) != null; } + this.formGroup.get("rememberEmail")?.setValue(rememberEmail); } async submit(showToast = true) { @@ -140,11 +133,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit this.formPromise = this.authService.logIn(credentials); const response = await this.formPromise; this.setFormValues(); - if (data.rememberEmail || this.alwaysRememberEmail) { - await this.stateService.setRememberedEmail(data.email); - } else { - await this.stateService.setRememberedEmail(null); - } + await this.loginService.saveEmailSettings(); if (this.handleCaptchaRequired(response)) { return; } else if (response.requiresTwoFactor) { @@ -162,7 +151,6 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit } else { const disableFavicon = await this.stateService.getDisableFavicon(); await this.stateService.setDisableFavicon(!!disableFavicon); - this.loginService.clearValues(); if (this.onSuccessfulLogin != null) { this.onSuccessfulLogin(); } @@ -189,6 +177,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit } async launchSsoBrowser(clientId: string, ssoRedirectUri: string) { + await this.saveEmailSettings(); // Generate necessary sso params const passwordOptions: any = { type: "password", @@ -243,6 +232,11 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit this.loginService.setRememberEmail(this.formGroup.value.rememberEmail); } + async saveEmailSettings() { + this.setFormValues(); + await this.loginService.saveEmailSettings(); + } + private getErrorToastMessage() { const error: AllValidationErrors = this.formValidationErrorService .getFormValidationErrors(this.formGroup.controls) diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 736190e8193..6c01fc9b337 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -598,6 +598,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction"; { provide: LoginServiceAbstraction, useClass: LoginService, + deps: [StateServiceAbstraction], }, ], }) diff --git a/libs/common/src/abstractions/login.service.ts b/libs/common/src/abstractions/login.service.ts index 0823611546c..9a884fd5d1c 100644 --- a/libs/common/src/abstractions/login.service.ts +++ b/libs/common/src/abstractions/login.service.ts @@ -4,4 +4,5 @@ export abstract class LoginService { setEmail: (value: string) => void; setRememberEmail: (value: boolean) => void; clearValues: () => void; + saveEmailSettings: () => Promise; } diff --git a/libs/common/src/services/login.service.ts b/libs/common/src/services/login.service.ts index 8a06f6b7a0d..e40121aa558 100644 --- a/libs/common/src/services/login.service.ts +++ b/libs/common/src/services/login.service.ts @@ -1,9 +1,12 @@ import { LoginService as LoginServiceAbstraction } from "../abstractions/login.service"; +import { StateService } from "../abstractions/state.service"; export class LoginService implements LoginServiceAbstraction { private _email: string; private _rememberEmail: boolean; + constructor(private stateService: StateService) {} + getEmail() { return this._email; } @@ -24,4 +27,9 @@ export class LoginService implements LoginServiceAbstraction { this._email = null; this._rememberEmail = null; } + + async saveEmailSettings() { + await this.stateService.setRememberedEmail(this._rememberEmail ? this._email : null); + this.clearValues(); + } }