Skip to content

Commit

Permalink
[PM-5264] Implement StateProvider in LoginEmailService (#7662)
Browse files Browse the repository at this point in the history
* setup StateProvider in LoginService

* replace implementations

* replace implementation

* remove stateService

* change storage location for web to 'disk-local'

* implement migrate() method of Migrator

* add RememberedEmailMigrator to migrate.ts

* add rollback

* add tests

* replace implementation

* replace implementation

* add StateProvider to Desktop services

* rename LoginService to RememberEmailService

* update state definition

* rename file

* rename to storedEmail

* rename service to EmailService to avoid confusion

* add jsdocs

* refactor login.component.ts

* fix typos

* fix test

* rename to LoginEmailService

* update factory

* more renaming

* remove duplicate logic and rename method

* convert storedEmail to observable

* refactor to remove setStoredEmail() method

* move service to libs/auth/common

* address floating promises

* remove comment

* remove unnecessary deps in service registration
  • Loading branch information
rr-bw committed Mar 30, 2024
1 parent 77cfa8a commit 2e51d96
Show file tree
Hide file tree
Showing 42 changed files with 396 additions and 240 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { LoginEmailServiceAbstraction, LoginEmailService } from "@bitwarden/auth/common";

import {
CachedServices,
factory,
FactoryOptions,
} from "../../../platform/background/service-factories/factory-options";
import {
stateProviderFactory,
StateProviderInitOptions,
} from "../../../platform/background/service-factories/state-provider.factory";

type LoginEmailServiceFactoryOptions = FactoryOptions;

export type LoginEmailServiceInitOptions = LoginEmailServiceFactoryOptions &
StateProviderInitOptions;

export function loginEmailServiceFactory(
cache: { loginEmailService?: LoginEmailServiceAbstraction } & CachedServices,
opts: LoginEmailServiceInitOptions,
): Promise<LoginEmailServiceAbstraction> {
return factory(
cache,
"loginEmailService",
opts,
async () => new LoginEmailService(await stateProviderFactory(cache, opts)),
);
}
6 changes: 3 additions & 3 deletions apps/browser/src/auth/popup/hint.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { Component } from "@angular/core";
import { ActivatedRoute, Router } from "@angular/router";

import { HintComponent as BaseHintComponent } from "@bitwarden/angular/auth/components/hint.component";
import { LoginEmailServiceAbstraction } from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { LoginService } from "@bitwarden/common/auth/abstractions/login.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
Expand All @@ -20,9 +20,9 @@ export class HintComponent extends BaseHintComponent {
apiService: ApiService,
logService: LogService,
private route: ActivatedRoute,
loginService: LoginService,
loginEmailService: LoginEmailServiceAbstraction,
) {
super(router, i18nService, apiService, platformUtilsService, logService, loginService);
super(router, i18nService, apiService, platformUtilsService, logService, loginEmailService);

super.onSuccessfulSubmit = async () => {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
Expand Down
2 changes: 1 addition & 1 deletion apps/browser/src/auth/popup/home.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</form>
<p class="createAccountLink">
{{ "newAroundHere" | i18n }}
<a routerLink="/register" (click)="setFormValues()">{{ "createAccount" | i18n }}</a>
<a routerLink="/register" (click)="setLoginEmailValues()">{{ "createAccount" | i18n }}</a>
</p>
</div>
</div>
47 changes: 19 additions & 28 deletions apps/browser/src/auth/popup/home.component.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { Component, OnDestroy, OnInit, ViewChild } from "@angular/core";
import { FormBuilder, Validators } from "@angular/forms";
import { Router } from "@angular/router";
import { Subject, takeUntil } from "rxjs";
import { Subject, firstValueFrom, takeUntil } from "rxjs";

import { EnvironmentSelectorComponent } from "@bitwarden/angular/auth/components/environment-selector.component";
import { LoginService } from "@bitwarden/common/auth/abstractions/login.service";
import { LoginEmailServiceAbstraction } 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";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";

import { AccountSwitcherService } from "./account-switching/services/account-switcher.service";

Expand All @@ -29,38 +28,32 @@ export class HomeComponent implements OnInit, OnDestroy {

constructor(
protected platformUtilsService: PlatformUtilsService,
private stateService: StateService,
private formBuilder: FormBuilder,
private router: Router,
private i18nService: I18nService,
private environmentService: EnvironmentService,
private loginService: LoginService,
private loginEmailService: LoginEmailServiceAbstraction,
private accountSwitcherService: AccountSwitcherService,
) {}

async ngOnInit(): Promise<void> {
let savedEmail = this.loginService.getEmail();
const rememberEmail = this.loginService.getRememberEmail();
const email = this.loginEmailService.getEmail();
const rememberEmail = this.loginEmailService.getRememberEmail();

if (savedEmail != null) {
this.formGroup.patchValue({
email: savedEmail,
rememberEmail: rememberEmail,
});
if (email != null) {
this.formGroup.patchValue({ email, rememberEmail });
} else {
savedEmail = await this.stateService.getRememberedEmail();
if (savedEmail != null) {
this.formGroup.patchValue({
email: savedEmail,
rememberEmail: true,
});
const storedEmail = await firstValueFrom(this.loginEmailService.storedEmail$);

if (storedEmail != null) {
this.formGroup.patchValue({ email: storedEmail, rememberEmail: true });
}
}

this.environmentSelector.onOpenSelfHostedSettings
.pipe(takeUntil(this.destroyed$))
.subscribe(() => {
this.setFormValues();
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"]);
Expand All @@ -76,8 +69,9 @@ export class HomeComponent implements OnInit, OnDestroy {
return this.accountSwitcherService.availableAccounts$;
}

submit() {
async submit() {
this.formGroup.markAllAsTouched();

if (this.formGroup.invalid) {
this.platformUtilsService.showToast(
"error",
Expand All @@ -87,15 +81,12 @@ export class HomeComponent implements OnInit, OnDestroy {
return;
}

this.loginService.setEmail(this.formGroup.value.email);
this.loginService.setRememberEmail(this.formGroup.value.rememberEmail);
// 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(["login"], { queryParams: { email: this.formGroup.value.email } });
this.setLoginEmailValues();
await this.router.navigate(["login"], { queryParams: { email: this.formGroup.value.email } });
}

setFormValues() {
this.loginService.setEmail(this.formGroup.value.email);
this.loginService.setRememberEmail(this.formGroup.value.rememberEmail);
setLoginEmailValues() {
this.loginEmailService.setEmail(this.formGroup.value.email);
this.loginEmailService.setRememberEmail(this.formGroup.value.rememberEmail);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import { LoginViaAuthRequestComponent as BaseLoginWithDeviceComponent } from "@b
import {
AuthRequestServiceAbstraction,
LoginStrategyServiceAbstraction,
LoginEmailServiceAbstraction,
} from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { AnonymousHubService } from "@bitwarden/common/auth/abstractions/anonymous-hub.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction";
import { LoginService } from "@bitwarden/common/auth/abstractions/login.service";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
Expand Down Expand Up @@ -44,7 +44,7 @@ export class LoginViaAuthRequestComponent extends BaseLoginWithDeviceComponent {
anonymousHubService: AnonymousHubService,
validationService: ValidationService,
stateService: StateService,
loginService: LoginService,
loginEmailService: LoginEmailServiceAbstraction,
syncService: SyncService,
deviceTrustCryptoService: DeviceTrustCryptoServiceAbstraction,
authRequestService: AuthRequestServiceAbstraction,
Expand All @@ -66,7 +66,7 @@ export class LoginViaAuthRequestComponent extends BaseLoginWithDeviceComponent {
anonymousHubService,
validationService,
stateService,
loginService,
loginEmailService,
deviceTrustCryptoService,
authRequestService,
loginStrategyService,
Expand Down
2 changes: 1 addition & 1 deletion apps/browser/src/auth/popup/login.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ <h1 class="login-center">
</div>
</div>
<div class="box-footer">
<button type="button" class="btn link" routerLink="/hint" (click)="setFormValues()">
<button type="button" class="btn link" routerLink="/hint" (click)="setLoginEmailValues()">
<b>{{ "getMasterPasswordHint" | i18n }}</b>
</button>
</div>
Expand Down
16 changes: 9 additions & 7 deletions apps/browser/src/auth/popup/login.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import { firstValueFrom } from "rxjs";

import { LoginComponent as BaseLoginComponent } from "@bitwarden/angular/auth/components/login.component";
import { FormValidationErrorsService } from "@bitwarden/angular/platform/abstractions/form-validation-errors.service";
import { LoginStrategyServiceAbstraction } from "@bitwarden/auth/common";
import {
LoginStrategyServiceAbstraction,
LoginEmailServiceAbstraction,
} from "@bitwarden/auth/common";
import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction";
import { LoginService } from "@bitwarden/common/auth/abstractions/login.service";
import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
import { WebAuthnLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login.service.abstraction";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
Expand Down Expand Up @@ -46,7 +48,7 @@ export class LoginComponent extends BaseLoginComponent {
formBuilder: FormBuilder,
formValidationErrorService: FormValidationErrorsService,
route: ActivatedRoute,
loginService: LoginService,
loginEmailService: LoginEmailServiceAbstraction,
ssoLoginService: SsoLoginServiceAbstraction,
webAuthnLoginService: WebAuthnLoginServiceAbstraction,
) {
Expand All @@ -66,7 +68,7 @@ export class LoginComponent extends BaseLoginComponent {
formBuilder,
formValidationErrorService,
route,
loginService,
loginEmailService,
ssoLoginService,
webAuthnLoginService,
);
Expand All @@ -77,8 +79,8 @@ export class LoginComponent extends BaseLoginComponent {
this.showPasswordless = flagEnabled("showPasswordless");

if (this.showPasswordless) {
this.formGroup.controls.email.setValue(this.loginService.getEmail());
this.formGroup.controls.rememberEmail.setValue(this.loginService.getRememberEmail());
this.formGroup.controls.email.setValue(this.loginEmailService.getEmail());
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();
Expand All @@ -94,7 +96,7 @@ export class LoginComponent extends BaseLoginComponent {
async launchSsoBrowser() {
// Save off email for SSO
await this.ssoLoginService.setSsoEmail(this.formGroup.value.email);
await this.loginService.saveEmailSettings();
await this.loginEmailService.saveEmailSettings();
// Generate necessary sso params
const passwordOptions: any = {
type: "password",
Expand Down
6 changes: 3 additions & 3 deletions apps/browser/src/auth/popup/two-factor.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { TwoFactorComponent as BaseTwoFactorComponent } from "@bitwarden/angular
import { WINDOW } from "@bitwarden/angular/services/injection-tokens";
import {
LoginStrategyServiceAbstraction,
LoginEmailServiceAbstraction,
UserDecryptionOptionsServiceAbstraction,
} from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { LoginService } from "@bitwarden/common/auth/abstractions/login.service";
import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service";
import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type";
Expand Down Expand Up @@ -57,7 +57,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent {
logService: LogService,
twoFactorService: TwoFactorService,
appIdService: AppIdService,
loginService: LoginService,
loginEmailService: LoginEmailServiceAbstraction,
userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
configService: ConfigService,
ssoLoginService: SsoLoginServiceAbstraction,
Expand All @@ -78,7 +78,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent {
logService,
twoFactorService,
appIdService,
loginService,
loginEmailService,
userDecryptionOptionsService,
ssoLoginService,
configService,
Expand Down
6 changes: 5 additions & 1 deletion apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
UserDecryptionOptionsService,
AuthRequestServiceAbstraction,
AuthRequestService,
LoginEmailServiceAbstraction,
} from "@bitwarden/auth/common";
import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service";
import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service";
Expand Down Expand Up @@ -258,6 +259,7 @@ export default class MainBackground {
auditService: AuditServiceAbstraction;
authService: AuthServiceAbstraction;
loginStrategyService: LoginStrategyServiceAbstraction;
loginEmailService: LoginEmailServiceAbstraction;
importApiService: ImportApiServiceAbstraction;
importService: ImportServiceAbstraction;
exportService: VaultExportServiceAbstraction;
Expand Down Expand Up @@ -1080,7 +1082,9 @@ export default class MainBackground {
await this.stateService.setActiveUser(userId);

if (userId == null) {
await this.stateService.setRememberedEmail(null);
this.loginEmailService.setRememberEmail(false);
await this.loginEmailService.saveEmailSettings();

await this.refreshBadge();
await this.refreshMenu();
await this.overlayBackground.updateOverlayCiphers();
Expand Down
7 changes: 0 additions & 7 deletions apps/browser/src/popup/services/services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,11 @@ import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/ab
import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction";
import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction";
import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service";
import { LoginService as LoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/login.service";
import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { AuthService } from "@bitwarden/common/auth/services/auth.service";
import { LoginService } from "@bitwarden/common/auth/services/login.service";
import {
AutofillSettingsService,
AutofillSettingsServiceAbstraction,
Expand Down Expand Up @@ -429,11 +427,6 @@ const safeProviders: SafeProvider[] = [
useClass: BrowserFileDownloadService,
deps: [],
}),
safeProvider({
provide: LoginServiceAbstraction,
useClass: LoginService,
deps: [StateServiceAbstraction],
}),
safeProvider({
provide: SYSTEM_THEME_OBSERVABLE,
useFactory: (platformUtilsService: PlatformUtilsService) => {
Expand Down
7 changes: 6 additions & 1 deletion apps/desktop/src/app/layout/account-switcher.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Component, OnDestroy, OnInit } from "@angular/core";
import { Router } from "@angular/router";
import { concatMap, firstValueFrom, Subject, takeUntil } from "rxjs";

import { LoginEmailServiceAbstraction } from "@bitwarden/auth/common";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service";
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
Expand Down Expand Up @@ -91,6 +92,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
private router: Router,
private tokenService: TokenService,
private environmentService: EnvironmentService,
private loginEmailService: LoginEmailServiceAbstraction,
) {}

async ngOnInit(): Promise<void> {
Expand Down Expand Up @@ -137,7 +139,10 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {

async addAccount() {
this.close();
await this.stateService.setRememberedEmail(null);

this.loginEmailService.setRememberEmail(false);
await this.loginEmailService.saveEmailSettings();

await this.router.navigate(["/login"]);
await this.stateService.setActiveUser(null);
}
Expand Down
7 changes: 0 additions & 7 deletions apps/desktop/src/app/services/services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vaul
import { PolicyService as PolicyServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service";
import { LoginService as LoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/login.service";
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
import { LoginService } from "@bitwarden/common/auth/services/login.service";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { BroadcasterService as BroadcasterServiceAbstraction } from "@bitwarden/common/platform/abstractions/broadcaster.service";
import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto-function.service";
Expand Down Expand Up @@ -221,11 +219,6 @@ const safeProviders: SafeProvider[] = [
DesktopAutofillSettingsService,
],
}),
safeProvider({
provide: LoginServiceAbstraction,
useClass: LoginService,
deps: [StateServiceAbstraction],
}),
safeProvider({
provide: CryptoFunctionServiceAbstraction,
useClass: RendererCryptoFunctionService,
Expand Down
Loading

0 comments on commit 2e51d96

Please sign in to comment.