From c70a5aa02457e515c6a94854da71e1200ea1b6e2 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 30 Apr 2024 09:13:02 -0400 Subject: [PATCH] [PM-6688] Use AccountService as account source (#8893) * Use account service to track accounts and active account * Remove state service active account Observables. * Add email verified to account service * Do not store account info on logged out accounts * Add account activity tracking to account service * Use last account activity from account service * migrate or replicate account service data * Add `AccountActivityService` that handles storing account last active data * Move active and next active user to account service * Remove authenticated accounts from state object * Fold account activity into account service * Fix builds * Fix desktop app switch * Fix logging out non active user * Expand helper to handle new authenticated accounts location * Prefer view observable to tons of async pipes * Fix `npm run test:types` * Correct user activity sorting test * Be more precise about log out messaging * Fix dev compare errors All stored values are serializable, the next step wasn't necessary and was erroring on some types that lack `toString`. * If the account in unlocked on load of lock component, navigate away from lock screen * Handle no users case for auth service statuses * Specify account to switch to * Filter active account out of inactive accounts * Prefer constructor init * Improve comparator * Use helper methods internally * Fixup component tests * Clarify name * Ensure accounts object has only valid userIds * Capitalize const values * Prefer descriptive, single-responsibility guards * Update libs/common/src/state-migrations/migrate.ts Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> * Fix merge * Add user Id validation activity for undefined was being set, which was resulting in requests for the auth status of `"undefined"` (string) userId, due to key enumeration. These changes stop that at both locations, as well as account add for good measure. --------- Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> --- .../account-switcher.component.html | 4 +- .../account-switcher.component.ts | 9 +- .../services/account-switcher.service.spec.ts | 3 + apps/browser/src/auth/popup/lock.component.ts | 3 +- .../context-menu-clicked-handler.spec.ts | 9 +- .../browser/context-menu-clicked-handler.ts | 17 +- .../browser/src/background/main.background.ts | 20 +- .../src/background/runtime.background.ts | 13 +- .../src/platform/popup/header.component.ts | 17 +- .../services/browser-state.service.spec.ts | 16 +- .../services/default-browser-state.service.ts | 2 - .../local-backed-session-storage.service.ts | 33 +-- apps/browser/src/popup/app.component.ts | 23 +- .../popup/send/send-add-edit.component.ts | 3 + apps/cli/src/bw.ts | 5 +- apps/desktop/src/app/app-routing.module.ts | 4 +- apps/desktop/src/app/app.component.ts | 65 ++--- .../layout/account-switcher.component.html | 208 ++++++++-------- .../app/layout/account-switcher.component.ts | 134 +++++----- .../src/app/layout/search/search.component.ts | 6 +- .../src/app/services/services.module.ts | 3 +- .../src/app/tools/generator.component.spec.ts | 5 + .../src/app/tools/send/add-edit.component.ts | 3 + apps/desktop/src/auth/guards/login.guard.ts | 29 --- .../src/auth/guards/max-accounts.guard.ts | 38 +++ apps/desktop/src/auth/lock.component.spec.ts | 9 +- apps/desktop/src/auth/lock.component.ts | 3 + apps/desktop/src/main/menu/menubar.ts | 5 +- apps/web/src/app/app.component.ts | 17 +- .../layouts/header/web-header.component.html | 4 +- .../layouts/header/web-header.component.ts | 20 +- .../src/app/tools/send/add-edit.component.ts | 3 + .../vault-items/vault-items.stories.ts | 1 - .../src/auth/components/lock.component.ts | 38 ++- libs/angular/src/pipes/user-name.pipe.ts | 2 +- .../src/tools/send/add-edit.component.ts | 6 +- .../auth-request-login.strategy.spec.ts | 1 + .../login-strategies/login.strategy.spec.ts | 5 +- .../common/login-strategies/login.strategy.ts | 8 + .../password-login.strategy.spec.ts | 3 + .../user-decryption-options.service.spec.ts | 1 + libs/common/spec/fake-account-service.ts | 57 ++++- .../src/auth/abstractions/account.service.ts | 47 +++- .../src/auth/services/account.service.spec.ts | 203 ++++++++++++++- .../src/auth/services/account.service.ts | 90 ++++++- .../src/auth/services/auth.service.spec.ts | 21 +- libs/common/src/auth/services/auth.service.ts | 28 +-- ...-enrollment.service.implementation.spec.ts | 1 + .../platform/abstractions/state.service.ts | 10 +- libs/common/src/platform/misc/utils.spec.ts | 27 ++ .../src/platform/models/domain/state.ts | 3 - .../default-environment.service.spec.ts | 3 + .../src/platform/services/state.service.ts | 234 +++++------------- .../src/platform/services/system.service.ts | 26 +- ...default-active-user-state.provider.spec.ts | 3 +- .../default-active-user-state.spec.ts | 1 + .../default-state.provider.spec.ts | 14 +- .../src/platform/state/state-definitions.ts | 1 + .../vault-timeout.service.spec.ts | 49 ++-- .../vault-timeout/vault-timeout.service.ts | 33 ++- libs/common/src/state-migrations/migrate.ts | 6 +- .../state-migrations/migration-helper.spec.ts | 43 ++++ .../src/state-migrations/migration-helper.ts | 28 ++- .../migrations/60-known-accounts.spec.ts | 145 +++++++++++ .../migrations/60-known-accounts.ts | 111 +++++++++ .../tools/send/services/send.service.spec.ts | 1 + .../src/vault/services/sync/sync.service.ts | 5 +- 67 files changed, 1375 insertions(+), 613 deletions(-) delete mode 100644 apps/desktop/src/auth/guards/login.guard.ts create mode 100644 apps/desktop/src/auth/guards/max-accounts.guard.ts create mode 100644 libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts create mode 100644 libs/common/src/state-migrations/migrations/60-known-accounts.ts diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html index aebf2219ff5f..806dae084dd2 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html @@ -49,7 +49,7 @@ + - - + +
+ + + + + {{ "accountSwitcherLimitReached" | i18n }} + - - {{ "accountSwitcherLimitReached" | i18n }} - - - - + + + diff --git a/apps/desktop/src/app/layout/account-switcher.component.ts b/apps/desktop/src/app/layout/account-switcher.component.ts index 4e39ab002922..c8a26065c11b 100644 --- a/apps/desktop/src/app/layout/account-switcher.component.ts +++ b/apps/desktop/src/app/layout/account-switcher.component.ts @@ -1,19 +1,17 @@ import { animate, state, style, transition, trigger } from "@angular/animations"; import { ConnectedPosition } from "@angular/cdk/overlay"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component } from "@angular/core"; import { Router } from "@angular/router"; -import { concatMap, firstValueFrom, Subject, takeUntil } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable, switchMap } from "rxjs"; import { LoginEmailServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; 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"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { Account } from "@bitwarden/common/platform/models/domain/account"; import { UserId } from "@bitwarden/common/types/guid"; type ActiveAccount = { @@ -52,12 +50,18 @@ type InactiveAccount = ActiveAccount & { ]), ], }) -export class AccountSwitcherComponent implements OnInit, OnDestroy { - activeAccount?: ActiveAccount; - inactiveAccounts: { [userId: string]: InactiveAccount } = {}; - +export class AccountSwitcherComponent { + activeAccount$: Observable; + inactiveAccounts$: Observable<{ [userId: string]: InactiveAccount }>; authStatus = AuthenticationStatus; + view$: Observable<{ + activeAccount: ActiveAccount | null; + inactiveAccounts: { [userId: string]: InactiveAccount }; + numberOfAccounts: number; + showSwitcher: boolean; + }>; + isOpen = false; overlayPosition: ConnectedPosition[] = [ { @@ -68,21 +72,9 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { }, ]; - private destroy$ = new Subject(); + showSwitcher$: Observable; - get showSwitcher() { - const userIsInAVault = !Utils.isNullOrWhitespace(this.activeAccount?.email); - const userIsAddingAnAdditionalAccount = Object.keys(this.inactiveAccounts).length > 0; - return userIsInAVault || userIsAddingAnAdditionalAccount; - } - - get numberOfAccounts() { - if (this.inactiveAccounts == null) { - this.isOpen = false; - return 0; - } - return Object.keys(this.inactiveAccounts).length; - } + numberOfAccounts$: Observable; constructor( private stateService: StateService, @@ -90,37 +82,65 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { private avatarService: AvatarService, private messagingService: MessagingService, private router: Router, - private tokenService: TokenService, private environmentService: EnvironmentService, private loginEmailService: LoginEmailServiceAbstraction, - ) {} - - async ngOnInit(): Promise { - this.stateService.accounts$ - .pipe( - concatMap(async (accounts: { [userId: string]: Account }) => { - this.inactiveAccounts = await this.createInactiveAccounts(accounts); - - try { - this.activeAccount = { - id: await this.tokenService.getUserId(), - name: (await this.tokenService.getName()) ?? (await this.tokenService.getEmail()), - email: await this.tokenService.getEmail(), - avatarColor: await firstValueFrom(this.avatarService.avatarColor$), - server: (await this.environmentService.getEnvironment())?.getHostname(), - }; - } catch { - this.activeAccount = undefined; - } - }), - takeUntil(this.destroy$), - ) - .subscribe(); - } - - ngOnDestroy(): void { - this.destroy$.next(); - this.destroy$.complete(); + private accountService: AccountService, + ) { + this.activeAccount$ = this.accountService.activeAccount$.pipe( + switchMap(async (active) => { + if (active == null) { + return null; + } + + return { + id: active.id, + name: active.name, + email: active.email, + avatarColor: await firstValueFrom(this.avatarService.avatarColor$), + server: (await this.environmentService.getEnvironment())?.getHostname(), + }; + }), + ); + this.inactiveAccounts$ = combineLatest([ + this.activeAccount$, + this.accountService.accounts$, + this.authService.authStatuses$, + ]).pipe( + switchMap(async ([activeAccount, accounts, accountStatuses]) => { + // Filter out logged out accounts and active account + accounts = Object.fromEntries( + Object.entries(accounts).filter( + ([id]: [UserId, AccountInfo]) => + accountStatuses[id] !== AuthenticationStatus.LoggedOut || id === activeAccount?.id, + ), + ); + return this.createInactiveAccounts(accounts); + }), + ); + this.showSwitcher$ = combineLatest([this.activeAccount$, this.inactiveAccounts$]).pipe( + map(([activeAccount, inactiveAccounts]) => { + const hasActiveUser = activeAccount != null; + const userIsAddingAnAdditionalAccount = Object.keys(inactiveAccounts).length > 0; + return hasActiveUser || userIsAddingAnAdditionalAccount; + }), + ); + this.numberOfAccounts$ = this.inactiveAccounts$.pipe( + map((accounts) => Object.keys(accounts).length), + ); + + this.view$ = combineLatest([ + this.activeAccount$, + this.inactiveAccounts$, + this.numberOfAccounts$, + this.showSwitcher$, + ]).pipe( + map(([activeAccount, inactiveAccounts, numberOfAccounts, showSwitcher]) => ({ + activeAccount, + inactiveAccounts, + numberOfAccounts, + showSwitcher, + })), + ); } toggle() { @@ -144,11 +164,13 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { await this.loginEmailService.saveEmailSettings(); await this.router.navigate(["/login"]); - await this.stateService.setActiveUser(null); + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + await this.stateService.clearDecryptedData(activeAccount?.id as UserId); + await this.accountService.switchAccount(null); } private async createInactiveAccounts(baseAccounts: { - [userId: string]: Account; + [userId: string]: AccountInfo; }): Promise<{ [userId: string]: InactiveAccount }> { const inactiveAccounts: { [userId: string]: InactiveAccount } = {}; @@ -159,8 +181,8 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { inactiveAccounts[userId] = { id: userId, - name: baseAccounts[userId].profile.name, - email: baseAccounts[userId].profile.email, + name: baseAccounts[userId].name, + email: baseAccounts[userId].email, authenticationStatus: await this.authService.getAuthStatus(userId), avatarColor: await firstValueFrom(this.avatarService.getUserAvatarColor$(userId as UserId)), server: (await this.environmentService.getEnvironment(userId))?.getHostname(), diff --git a/apps/desktop/src/app/layout/search/search.component.ts b/apps/desktop/src/app/layout/search/search.component.ts index 9a7226218a66..06c67d8af227 100644 --- a/apps/desktop/src/app/layout/search/search.component.ts +++ b/apps/desktop/src/app/layout/search/search.component.ts @@ -2,7 +2,7 @@ import { Component, OnDestroy, OnInit } from "@angular/core"; import { UntypedFormControl } from "@angular/forms"; import { Subscription } from "rxjs"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { SearchBarService, SearchBarState } from "./search-bar.service"; @@ -18,7 +18,7 @@ export class SearchComponent implements OnInit, OnDestroy { constructor( private searchBarService: SearchBarService, - private stateService: StateService, + private accountService: AccountService, ) { // eslint-disable-next-line rxjs-angular/prefer-takeuntil this.searchBarService.state$.subscribe((state) => { @@ -33,7 +33,7 @@ export class SearchComponent implements OnInit, OnDestroy { ngOnInit() { // eslint-disable-next-line rxjs-angular/prefer-takeuntil - this.activeAccountSubscription = this.stateService.activeAccount$.subscribe((value) => { + this.activeAccountSubscription = this.accountService.activeAccount$.subscribe((_) => { this.searchBarService.setSearchText(""); this.searchText.patchValue(""); }); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 1e3a7fdfa59d..a485b925ba6d 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -59,7 +59,6 @@ import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/ge import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service"; import { DialogService } from "@bitwarden/components"; -import { LoginGuard } from "../../auth/guards/login.guard"; import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { Account } from "../../models/account"; import { DesktopSettingsService } from "../../platform/services/desktop-settings.service"; @@ -102,7 +101,6 @@ const safeProviders: SafeProvider[] = [ safeProvider(InitService), safeProvider(NativeMessagingService), safeProvider(SearchBarService), - safeProvider(LoginGuard), safeProvider(DialogService), safeProvider({ provide: APP_INITIALIZER as SafeInjectionToken<() => void>, @@ -192,6 +190,7 @@ const safeProviders: SafeProvider[] = [ AutofillSettingsServiceAbstraction, VaultTimeoutSettingsService, BiometricStateService, + AccountServiceAbstraction, ], }), safeProvider({ diff --git a/apps/desktop/src/app/tools/generator.component.spec.ts b/apps/desktop/src/app/tools/generator.component.spec.ts index 51b5bf93a25c..d908de8ef77a 100644 --- a/apps/desktop/src/app/tools/generator.component.spec.ts +++ b/apps/desktop/src/app/tools/generator.component.spec.ts @@ -4,6 +4,7 @@ import { ActivatedRoute } from "@angular/router"; import { mock, MockProxy } from "jest-mock-extended"; import { I18nPipe } from "@bitwarden/angular/platform/pipes/i18n.pipe"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.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"; @@ -59,6 +60,10 @@ describe("GeneratorComponent", () => { provide: CipherService, useValue: mock(), }, + { + provide: AccountService, + useValue: mock(), + }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); diff --git a/apps/desktop/src/app/tools/send/add-edit.component.ts b/apps/desktop/src/app/tools/send/add-edit.component.ts index 7bdd5efbba96..804a39043806 100644 --- a/apps/desktop/src/app/tools/send/add-edit.component.ts +++ b/apps/desktop/src/app/tools/send/add-edit.component.ts @@ -4,6 +4,7 @@ import { FormBuilder } from "@angular/forms"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/tools/send/add-edit.component"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -34,6 +35,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService: DialogService, formBuilder: FormBuilder, billingAccountProfileStateService: BillingAccountProfileStateService, + accountService: AccountService, ) { super( i18nService, @@ -49,6 +51,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService, formBuilder, billingAccountProfileStateService, + accountService, ); } diff --git a/apps/desktop/src/auth/guards/login.guard.ts b/apps/desktop/src/auth/guards/login.guard.ts deleted file mode 100644 index f6c67d5af9c4..000000000000 --- a/apps/desktop/src/auth/guards/login.guard.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Injectable } from "@angular/core"; -import { CanActivate } from "@angular/router"; -import { firstValueFrom } from "rxjs"; - -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"; - -const maxAllowedAccounts = 5; - -@Injectable() -export class LoginGuard implements CanActivate { - protected homepage = "vault"; - constructor( - private stateService: StateService, - private platformUtilsService: PlatformUtilsService, - private i18nService: I18nService, - ) {} - - async canActivate() { - const accounts = await firstValueFrom(this.stateService.accounts$); - if (accounts != null && Object.keys(accounts).length >= maxAllowedAccounts) { - this.platformUtilsService.showToast("error", null, this.i18nService.t("accountLimitReached")); - return false; - } - - return true; - } -} diff --git a/apps/desktop/src/auth/guards/max-accounts.guard.ts b/apps/desktop/src/auth/guards/max-accounts.guard.ts new file mode 100644 index 000000000000..65c4ac99d01b --- /dev/null +++ b/apps/desktop/src/auth/guards/max-accounts.guard.ts @@ -0,0 +1,38 @@ +import { inject } from "@angular/core"; +import { CanActivateFn } from "@angular/router"; +import { Observable, map } from "rxjs"; + +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +const maxAllowedAccounts = 5; + +function maxAccountsGuard(): Observable { + const authService = inject(AuthService); + const toastService = inject(ToastService); + const i18nService = inject(I18nService); + + return authService.authStatuses$.pipe( + map((statuses) => + Object.values(statuses).filter((status) => status != AuthenticationStatus.LoggedOut), + ), + map((accounts) => { + if (accounts != null && Object.keys(accounts).length >= maxAllowedAccounts) { + toastService.showToast({ + variant: "error", + title: null, + message: i18nService.t("accountLimitReached"), + }); + return false; + } + + return true; + }), + ); +} + +export function maxAccountsGuardFn(): CanActivateFn { + return () => maxAccountsGuard(); +} diff --git a/apps/desktop/src/auth/lock.component.spec.ts b/apps/desktop/src/auth/lock.component.spec.ts index f998e75d7a0d..2137b707f676 100644 --- a/apps/desktop/src/auth/lock.component.spec.ts +++ b/apps/desktop/src/auth/lock.component.spec.ts @@ -13,6 +13,7 @@ import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeou import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; @@ -50,7 +51,7 @@ describe("LockComponent", () => { let component: LockComponent; let fixture: ComponentFixture; let stateServiceMock: MockProxy; - const biometricStateService = mock(); + let biometricStateService: MockProxy; let messagingServiceMock: MockProxy; let broadcasterServiceMock: MockProxy; let platformUtilsServiceMock: MockProxy; @@ -62,7 +63,6 @@ describe("LockComponent", () => { beforeEach(async () => { stateServiceMock = mock(); - stateServiceMock.activeAccount$ = of(null); messagingServiceMock = mock(); broadcasterServiceMock = mock(); @@ -73,6 +73,7 @@ describe("LockComponent", () => { mockMasterPasswordService = new FakeMasterPasswordService(); + biometricStateService = mock(); biometricStateService.dismissedRequirePasswordOnStartCallout$ = of(false); biometricStateService.promptAutomatically$ = of(false); biometricStateService.promptCancelled$ = of(false); @@ -165,6 +166,10 @@ describe("LockComponent", () => { provide: AccountService, useValue: accountService, }, + { + provide: AuthService, + useValue: mock(), + }, { provide: KdfConfigService, useValue: mock(), diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index 8e87b6663fc1..d95df419e1a7 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -10,6 +10,7 @@ import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeou import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; @@ -64,6 +65,7 @@ export class LockComponent extends BaseLockComponent { pinCryptoService: PinCryptoServiceAbstraction, biometricStateService: BiometricStateService, accountService: AccountService, + authService: AuthService, kdfConfigService: KdfConfigService, ) { super( @@ -89,6 +91,7 @@ export class LockComponent extends BaseLockComponent { pinCryptoService, biometricStateService, accountService, + authService, kdfConfigService, ); } diff --git a/apps/desktop/src/main/menu/menubar.ts b/apps/desktop/src/main/menu/menubar.ts index eb1dacf82502..b71774c5afef 100644 --- a/apps/desktop/src/main/menu/menubar.ts +++ b/apps/desktop/src/main/menu/menubar.ts @@ -65,9 +65,10 @@ export class Menubar { isLocked = updateRequest.accounts[updateRequest.activeUserId]?.isLocked ?? true; } - const isLockable = !isLocked && updateRequest?.accounts[updateRequest.activeUserId]?.isLockable; + const isLockable = + !isLocked && updateRequest?.accounts?.[updateRequest.activeUserId]?.isLockable; const hasMasterPassword = - updateRequest?.accounts[updateRequest.activeUserId]?.hasMasterPassword ?? false; + updateRequest?.accounts?.[updateRequest.activeUserId]?.hasMasterPassword ?? false; this.items = [ new FileMenu( diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 1da2d94c15b3..1939bb11f5fb 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -2,7 +2,7 @@ import { DOCUMENT } from "@angular/common"; import { Component, Inject, NgZone, OnDestroy, OnInit } from "@angular/core"; import { NavigationEnd, Router } from "@angular/router"; import * as jq from "jquery"; -import { Subject, switchMap, takeUntil, timer } from "rxjs"; +import { Subject, firstValueFrom, map, switchMap, takeUntil, timer } from "rxjs"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; @@ -10,6 +10,7 @@ import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { PaymentMethodWarningsServiceAbstraction as PaymentMethodWarningService } from "@bitwarden/common/billing/abstractions/payment-method-warnings-service.abstraction"; @@ -51,7 +52,7 @@ const PaymentMethodWarningsRefresh = 60000; // 1 Minute templateUrl: "app.component.html", }) export class AppComponent implements OnDestroy, OnInit { - private lastActivity: number = null; + private lastActivity: Date = null; private idleTimer: number = null; private isIdle = false; private destroy$ = new Subject(); @@ -86,6 +87,7 @@ export class AppComponent implements OnDestroy, OnInit { private stateEventRunnerService: StateEventRunnerService, private paymentMethodWarningService: PaymentMethodWarningService, private organizationService: InternalOrganizationServiceAbstraction, + private accountService: AccountService, ) {} ngOnInit() { @@ -298,15 +300,16 @@ export class AppComponent implements OnDestroy, OnInit { } private async recordActivity() { - const now = new Date().getTime(); - if (this.lastActivity != null && now - this.lastActivity < 250) { + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ); + const now = new Date(); + if (this.lastActivity != null && now.getTime() - this.lastActivity.getTime() < 250) { return; } this.lastActivity = now; - // 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.stateService.setLastActive(now); + await this.accountService.setAccountActivity(activeUserId, now); // Idle states if (this.isIdle) { this.isIdle = false; diff --git a/apps/web/src/app/layouts/header/web-header.component.html b/apps/web/src/app/layouts/header/web-header.component.html index e24013de6f29..e2b3e7910abf 100644 --- a/apps/web/src/app/layouts/header/web-header.component.html +++ b/apps/web/src/app/layouts/header/web-header.component.html @@ -58,7 +58,7 @@ [bitMenuTriggerFor]="accountMenu" class="tw-border-0 tw-bg-transparent tw-p-0" > - + @@ -67,7 +67,7 @@ class="tw-flex tw-items-center tw-px-4 tw-py-1 tw-leading-tight tw-text-info" appStopProp > - +
{{ "loggedInAs" | i18n }} diff --git a/apps/web/src/app/layouts/header/web-header.component.ts b/apps/web/src/app/layouts/header/web-header.component.ts index 1f012e52ddce..9906bd53bab7 100644 --- a/apps/web/src/app/layouts/header/web-header.component.ts +++ b/apps/web/src/app/layouts/header/web-header.component.ts @@ -1,16 +1,17 @@ import { Component, Input } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { combineLatest, map, Observable } from "rxjs"; +import { map, Observable } from "rxjs"; +import { User } from "@bitwarden/angular/pipes/user-name.pipe"; import { UnassignedItemsBannerService } from "@bitwarden/angular/services/unassigned-items-banner.service"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { AccountProfile } from "@bitwarden/common/platform/models/domain/account"; +import { UserId } from "@bitwarden/common/types/guid"; @Component({ selector: "app-header", @@ -28,7 +29,7 @@ export class WebHeaderComponent { @Input() icon: string; protected routeData$: Observable<{ titleId: string }>; - protected account$: Observable; + protected account$: Observable; protected canLock$: Observable; protected selfHosted: boolean; protected hostname = location.hostname; @@ -38,12 +39,12 @@ export class WebHeaderComponent { constructor( private route: ActivatedRoute, - private stateService: StateService, private platformUtilsService: PlatformUtilsService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private messagingService: MessagingService, protected unassignedItemsBannerService: UnassignedItemsBannerService, private configService: ConfigService, + private accountService: AccountService, ) { this.routeData$ = this.route.data.pipe( map((params) => { @@ -55,14 +56,7 @@ export class WebHeaderComponent { this.selfHosted = this.platformUtilsService.isSelfHost(); - this.account$ = combineLatest([ - this.stateService.activeAccount$, - this.stateService.accounts$, - ]).pipe( - map(([activeAccount, accounts]) => { - return accounts[activeAccount]?.profile; - }), - ); + this.account$ = this.accountService.activeAccount$; this.canLock$ = this.vaultTimeoutSettingsService .availableVaultTimeoutActions$() .pipe(map((actions) => actions.includes(VaultTimeoutAction.Lock))); diff --git a/apps/web/src/app/tools/send/add-edit.component.ts b/apps/web/src/app/tools/send/add-edit.component.ts index ee4be4148891..cca416db9cd4 100644 --- a/apps/web/src/app/tools/send/add-edit.component.ts +++ b/apps/web/src/app/tools/send/add-edit.component.ts @@ -5,6 +5,7 @@ import { FormBuilder } from "@angular/forms"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/tools/send/add-edit.component"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -40,6 +41,7 @@ export class AddEditComponent extends BaseAddEditComponent { billingAccountProfileStateService: BillingAccountProfileStateService, protected dialogRef: DialogRef, @Inject(DIALOG_DATA) params: { sendId: string }, + accountService: AccountService, ) { super( i18nService, @@ -55,6 +57,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService, formBuilder, billingAccountProfileStateService, + accountService, ); this.sendId = params.sendId; diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts index ad80c9f4e581..41aa766e3a4f 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts @@ -55,7 +55,6 @@ export default { { provide: StateService, useValue: { - activeAccount$: new BehaviorSubject("1").asObservable(), accounts$: new BehaviorSubject({ "1": { profile: { name: "Foo" } } }).asObservable(), async getShowFavicon() { return true; diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index 89af31da81ab..7eb30d759ac0 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -1,7 +1,7 @@ import { Directive, NgZone, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; import { firstValueFrom, Subject } from "rxjs"; -import { concatMap, take, takeUntil } from "rxjs/operators"; +import { concatMap, map, take, takeUntil } from "rxjs/operators"; import { PinCryptoServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -11,10 +11,12 @@ import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abs import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SecretVerificationRequest } from "@bitwarden/common/auth/models/request/secret-verification.request"; import { MasterPasswordPolicyResponse } from "@bitwarden/common/auth/models/response/master-password-policy.response"; @@ -30,6 +32,7 @@ import { BiometricStateService } from "@bitwarden/common/platform/biometrics/bio import { HashPurpose, KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { PinLockType } from "@bitwarden/common/services/vault-timeout/vault-timeout-settings.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; +import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; import { DialogService } from "@bitwarden/components"; @@ -46,6 +49,7 @@ export class LockComponent implements OnInit, OnDestroy { supportsBiometric: boolean; biometricLock: boolean; + private activeUserId: UserId; protected successRoute = "vault"; protected forcePasswordResetRoute = "update-temp-password"; protected onSuccessfulSubmit: () => Promise; @@ -80,14 +84,16 @@ export class LockComponent implements OnInit, OnDestroy { protected pinCryptoService: PinCryptoServiceAbstraction, protected biometricStateService: BiometricStateService, protected accountService: AccountService, + protected authService: AuthService, protected kdfConfigService: KdfConfigService, ) {} async ngOnInit() { - this.stateService.activeAccount$ + this.accountService.activeAccount$ .pipe( - concatMap(async () => { - await this.load(); + concatMap(async (account) => { + this.activeUserId = account?.id; + await this.load(account?.id); }), takeUntil(this.destroy$), ) @@ -116,7 +122,7 @@ export class LockComponent implements OnInit, OnDestroy { }); if (confirmed) { - this.messagingService.send("logout"); + this.messagingService.send("logout", { userId: this.activeUserId }); } } @@ -321,23 +327,35 @@ export class LockComponent implements OnInit, OnDestroy { } } - private async load() { + private async load(userId: UserId) { // TODO: Investigate PM-3515 // The loading of the lock component works as follows: - // 1. First, is locking a valid timeout action? If not, we will log the user out. - // 2. If locking IS a valid timeout action, we proceed to show the user the lock screen. + // 1. If the user is unlocked, we're here in error so we navigate to the home page + // 2. First, is locking a valid timeout action? If not, we will log the user out. + // 3. If locking IS a valid timeout action, we proceed to show the user the lock screen. // The user will be able to unlock as follows: // - If they have a PIN set, they will be presented with the PIN input // - If they have a master password and no PIN, they will be presented with the master password input // - If they have biometrics enabled, they will be presented with the biometric prompt + const isUnlocked = await firstValueFrom( + this.authService + .authStatusFor$(userId) + .pipe(map((status) => status === AuthenticationStatus.Unlocked)), + ); + if (isUnlocked) { + // navigate to home + await this.router.navigate(["/"]); + return; + } + const availableVaultTimeoutActions = await firstValueFrom( - this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(userId), ); const supportsLock = availableVaultTimeoutActions.includes(VaultTimeoutAction.Lock); if (!supportsLock) { - return await this.vaultTimeoutService.logOut(); + return await this.vaultTimeoutService.logOut(userId); } this.pinStatus = await this.vaultTimeoutSettingsService.isPinLockSet(); diff --git a/libs/angular/src/pipes/user-name.pipe.ts b/libs/angular/src/pipes/user-name.pipe.ts index 88b088a7e229..f007f4ad8738 100644 --- a/libs/angular/src/pipes/user-name.pipe.ts +++ b/libs/angular/src/pipes/user-name.pipe.ts @@ -1,6 +1,6 @@ import { Pipe, PipeTransform } from "@angular/core"; -interface User { +export interface User { name?: string; email?: string; } diff --git a/libs/angular/src/tools/send/add-edit.component.ts b/libs/angular/src/tools/send/add-edit.component.ts index da859e50bfb6..b4f7ec171a1c 100644 --- a/libs/angular/src/tools/send/add-edit.component.ts +++ b/libs/angular/src/tools/send/add-edit.component.ts @@ -5,6 +5,7 @@ import { Subject, firstValueFrom, takeUntil, map, BehaviorSubject, concatMap } f import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -118,6 +119,7 @@ export class AddEditComponent implements OnInit, OnDestroy { protected dialogService: DialogService, protected formBuilder: FormBuilder, protected billingAccountProfileStateService: BillingAccountProfileStateService, + protected accountService: AccountService, ) { this.typeOptions = [ { name: i18nService.t("sendTypeFile"), value: SendType.File, premium: true }, @@ -215,7 +217,9 @@ export class AddEditComponent implements OnInit, OnDestroy { } async load() { - this.emailVerified = await this.stateService.getEmailVerified(); + this.emailVerified = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.emailVerified ?? false)), + ); this.type = !this.canAccessPremium || !this.emailVerified ? SendType.Text : SendType.File; if (this.send == null) { diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts index a123e3005385..0efb9569eb58 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts @@ -128,6 +128,7 @@ describe("AuthRequestLoginStrategy", () => { masterPasswordService.masterKeySubject.next(masterKey); cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + tokenService.decodeAccessToken.mockResolvedValue({ sub: mockUserId }); await authRequestLoginStrategy.logIn(credentials); diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 3284f6e94744..c3a8f61d7822 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -218,7 +218,7 @@ describe("LoginStrategy", () => { expect(messagingService.send).toHaveBeenCalledWith("loggedIn"); }); - it("throws if active account isn't found after being initialized", async () => { + it("throws if new account isn't active after being initialized", async () => { const idTokenResponse = identityTokenResponseFactory(); apiService.postIdentityToken.mockResolvedValue(idTokenResponse); @@ -228,7 +228,8 @@ describe("LoginStrategy", () => { stateService.getVaultTimeoutAction.mockResolvedValue(mockVaultTimeoutAction); stateService.getVaultTimeout.mockResolvedValue(mockVaultTimeout); - accountService.activeAccountSubject.next(null); + accountService.switchAccount = jest.fn(); // block internal switch to new account + accountService.activeAccountSubject.next(null); // simulate no active account await expect(async () => await passwordLoginStrategy.logIn(credentials)).rejects.toThrow(); }); diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index fd268d955efa..3a3109349e88 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -169,6 +169,12 @@ export abstract class LoginStrategy { const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction({ userId }); const vaultTimeout = await this.stateService.getVaultTimeout({ userId }); + await this.accountService.addAccount(userId, { + name: accountInformation.name, + email: accountInformation.email, + emailVerified: accountInformation.email_verified, + }); + // set access token and refresh token before account initialization so authN status can be accurate // User id will be derived from the access token. await this.tokenService.setTokens( @@ -178,6 +184,8 @@ export abstract class LoginStrategy { tokenResponse.refreshToken, // Note: CLI login via API key sends undefined for refresh token. ); + await this.accountService.switchAccount(userId); + await this.stateService.addAccount( new Account({ profile: { diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index 5c1fe9b1fe8d..c97639f1023f 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -164,6 +164,7 @@ describe("PasswordLoginStrategy", () => { masterPasswordService.masterKeySubject.next(masterKey); cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); await passwordLoginStrategy.logIn(credentials); @@ -199,6 +200,7 @@ describe("PasswordLoginStrategy", () => { it("forces the user to update their master password on successful login when it does not meet master password policy requirements", async () => { passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any); policyService.evaluateMasterPassword.mockReturnValue(false); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); const result = await passwordLoginStrategy.logIn(credentials); @@ -213,6 +215,7 @@ describe("PasswordLoginStrategy", () => { it("forces the user to update their master password on successful 2FA login when it does not meet master password policy requirements", async () => { passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any); policyService.evaluateMasterPassword.mockReturnValue(false); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); const token2FAResponse = new IdentityTwoFactorResponse({ TwoFactorProviders: ["0"], diff --git a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts index 16479f19ea53..ae1813d3d7b8 100644 --- a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts +++ b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts @@ -65,6 +65,7 @@ describe("UserDecryptionOptionsService", () => { await fakeAccountService.addAccount(givenUser, { name: "Test User 1", email: "test1@email.com", + emailVerified: false, }); await fakeStateProvider.setUserState( USER_DECRYPTION_OPTIONS, diff --git a/libs/common/spec/fake-account-service.ts b/libs/common/spec/fake-account-service.ts index a8b09b7417f0..649a158d757e 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { ReplaySubject } from "rxjs"; +import { ReplaySubject, combineLatest, map } from "rxjs"; import { AccountInfo, AccountService } from "../src/auth/abstractions/account.service"; import { UserId } from "../src/types/guid"; @@ -7,15 +7,20 @@ import { UserId } from "../src/types/guid"; export function mockAccountServiceWith( userId: UserId, info: Partial = {}, + activity: Record = {}, ): FakeAccountService { const fullInfo: AccountInfo = { ...info, ...{ name: "name", email: "email", + emailVerified: true, }, }; - const service = new FakeAccountService({ [userId]: fullInfo }); + + const fullActivity = { [userId]: new Date(), ...activity }; + + const service = new FakeAccountService({ [userId]: fullInfo }, fullActivity); service.activeAccountSubject.next({ id: userId, ...fullInfo }); return service; } @@ -26,17 +31,46 @@ export class FakeAccountService implements AccountService { accountsSubject = new ReplaySubject>(1); // eslint-disable-next-line rxjs/no-exposed-subjects -- test class activeAccountSubject = new ReplaySubject<{ id: UserId } & AccountInfo>(1); + // eslint-disable-next-line rxjs/no-exposed-subjects -- test class + accountActivitySubject = new ReplaySubject>(1); private _activeUserId: UserId; get activeUserId() { return this._activeUserId; } accounts$ = this.accountsSubject.asObservable(); activeAccount$ = this.activeAccountSubject.asObservable(); + accountActivity$ = this.accountActivitySubject.asObservable(); + get sortedUserIds$() { + return this.accountActivity$.pipe( + map((activity) => { + return Object.entries(activity) + .map(([userId, lastActive]: [UserId, Date]) => ({ userId, lastActive })) + .sort((a, b) => a.lastActive.getTime() - b.lastActive.getTime()) + .map((a) => a.userId); + }), + ); + } + get nextUpAccount$() { + return combineLatest([this.accounts$, this.activeAccount$, this.sortedUserIds$]).pipe( + map(([accounts, activeAccount, sortedUserIds]) => { + const nextId = sortedUserIds.find((id) => id !== activeAccount?.id && accounts[id] != null); + return nextId ? { id: nextId, ...accounts[nextId] } : null; + }), + ); + } - constructor(initialData: Record) { + constructor(initialData: Record, accountActivity?: Record) { this.accountsSubject.next(initialData); this.activeAccountSubject.subscribe((data) => (this._activeUserId = data?.id)); this.activeAccountSubject.next(null); + this.accountActivitySubject.next(accountActivity); + } + setAccountActivity(userId: UserId, lastActivity: Date): Promise { + this.accountActivitySubject.next({ + ...this.accountActivitySubject["_buffer"][0], + [userId]: lastActivity, + }); + return this.mock.setAccountActivity(userId, lastActivity); } async addAccount(userId: UserId, accountData: AccountInfo): Promise { @@ -53,10 +87,27 @@ export class FakeAccountService implements AccountService { await this.mock.setAccountEmail(userId, email); } + async setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise { + await this.mock.setAccountEmailVerified(userId, emailVerified); + } + async switchAccount(userId: UserId): Promise { const next = userId == null ? null : { id: userId, ...this.accountsSubject["_buffer"]?.[0]?.[userId] }; this.activeAccountSubject.next(next); await this.mock.switchAccount(userId); } + + async clean(userId: UserId): Promise { + const current = this.accountsSubject["_buffer"][0] ?? {}; + const updated = { ...current, [userId]: loggedOutInfo }; + this.accountsSubject.next(updated); + await this.mock.clean(userId); + } } + +const loggedOutInfo: AccountInfo = { + name: undefined, + email: "", + emailVerified: false, +}; diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index fa9ad36378de..b7fd6d9bb937 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -8,18 +8,44 @@ import { UserId } from "../../types/guid"; */ export type AccountInfo = { email: string; + emailVerified: boolean; name: string | undefined; }; export function accountInfoEqual(a: AccountInfo, b: AccountInfo) { - return a?.email === b?.email && a?.name === b?.name; + if (a == null && b == null) { + return true; + } + + if (a == null || b == null) { + return false; + } + + const keys = new Set([...Object.keys(a), ...Object.keys(b)]) as Set; + for (const key of keys) { + if (a[key] !== b[key]) { + return false; + } + } + return true; } export abstract class AccountService { accounts$: Observable>; activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>; + + /** + * Observable of the last activity time for each account. + */ + accountActivity$: Observable>; + /** Account list in order of descending recency */ + sortedUserIds$: Observable; + /** Next account that is not the current active account */ + nextUpAccount$: Observable<{ id: UserId } & AccountInfo>; /** * Updates the `accounts$` observable with the new account data. + * + * @note Also sets the last active date of the account to `now`. * @param userId * @param accountData */ @@ -36,11 +62,30 @@ export abstract class AccountService { * @param email */ abstract setAccountEmail(userId: UserId, email: string): Promise; + /** + * updates the `accounts$` observable with the new email verification status for the account. + * @param userId + * @param emailVerified + */ + abstract setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise; /** * Updates the `activeAccount$` observable with the new active account. * @param userId */ abstract switchAccount(userId: UserId): Promise; + /** + * Cleans personal information for the given account from the `accounts$` observable. Does not remove the userId from the observable. + * + * @note Also sets the last active date of the account to `null`. + * @param userId + */ + abstract clean(userId: UserId): Promise; + /** + * Updates the given user's last activity time. + * @param userId + * @param lastActivity + */ + abstract setAccountActivity(userId: UserId, lastActivity: Date): Promise; } export abstract class InternalAccountService extends AccountService { diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts index a9cec82c511a..0ae14b0cc12d 100644 --- a/libs/common/src/auth/services/account.service.spec.ts +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -1,3 +1,8 @@ +/** + * need to update test environment so structuredClone works appropriately + * @jest-environment ../../libs/shared/test.environment.ts + */ + import { MockProxy, mock } from "jest-mock-extended"; import { firstValueFrom } from "rxjs"; @@ -6,15 +11,57 @@ import { FakeGlobalStateProvider } from "../../../spec/fake-state-provider"; import { trackEmissions } from "../../../spec/utils"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { Utils } from "../../platform/misc/utils"; import { UserId } from "../../types/guid"; -import { AccountInfo } from "../abstractions/account.service"; +import { AccountInfo, accountInfoEqual } from "../abstractions/account.service"; import { ACCOUNT_ACCOUNTS, ACCOUNT_ACTIVE_ACCOUNT_ID, + ACCOUNT_ACTIVITY, AccountServiceImplementation, } from "./account.service"; +describe("accountInfoEqual", () => { + const accountInfo: AccountInfo = { name: "name", email: "email", emailVerified: true }; + + it("compares nulls", () => { + expect(accountInfoEqual(null, null)).toBe(true); + expect(accountInfoEqual(null, accountInfo)).toBe(false); + expect(accountInfoEqual(accountInfo, null)).toBe(false); + }); + + it("compares all keys, not just those defined in AccountInfo", () => { + const different = { ...accountInfo, extra: "extra" }; + + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares name", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, name: "name2" }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares email", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, email: "email2" }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares emailVerified", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, emailVerified: false }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); +}); + describe("accountService", () => { let messagingService: MockProxy; let logService: MockProxy; @@ -22,8 +69,8 @@ describe("accountService", () => { let sut: AccountServiceImplementation; let accountsState: FakeGlobalState>; let activeAccountIdState: FakeGlobalState; - const userId = "userId" as UserId; - const userInfo = { email: "email", name: "name" }; + const userId = Utils.newGuid() as UserId; + const userInfo = { email: "email", name: "name", emailVerified: true }; beforeEach(() => { messagingService = mock(); @@ -86,6 +133,25 @@ describe("accountService", () => { expect(currentValue).toEqual({ [userId]: userInfo }); }); + + it("sets the last active date of the account to now", async () => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({}); + await sut.addAccount(userId, userInfo); + + expect(state.nextMock).toHaveBeenCalledWith({ [userId]: expect.any(Date) }); + }); + + it.each([null, undefined, 123, "not a guid"])( + "does not set last active if the userId is not a valid guid", + async (userId) => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({}); + await expect(sut.addAccount(userId as UserId, userInfo)).rejects.toThrow( + "userId is required", + ); + }, + ); }); describe("setAccountName", () => { @@ -134,6 +200,58 @@ describe("accountService", () => { }); }); + describe("setAccountEmailVerified", () => { + const initialState = { [userId]: userInfo }; + initialState[userId].emailVerified = false; + beforeEach(() => { + accountsState.stateSubject.next(initialState); + }); + + it("should update the account", async () => { + await sut.setAccountEmailVerified(userId, true); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: { ...userInfo, emailVerified: true }, + }); + }); + + it("should not update if the email is the same", async () => { + await sut.setAccountEmailVerified(userId, false); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual(initialState); + }); + }); + + describe("clean", () => { + beforeEach(() => { + accountsState.stateSubject.next({ [userId]: userInfo }); + }); + + it("removes account info of the given user", async () => { + await sut.clean(userId); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: { + email: "", + emailVerified: false, + name: undefined, + }, + }); + }); + + it("removes account activity of the given user", async () => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({ [userId]: new Date() }); + + await sut.clean(userId); + + expect(state.nextMock).toHaveBeenCalledWith({}); + }); + }); + describe("switchAccount", () => { beforeEach(() => { accountsState.stateSubject.next({ [userId]: userInfo }); @@ -152,4 +270,83 @@ describe("accountService", () => { expect(sut.switchAccount("unknown" as UserId)).rejects.toThrowError("Account does not exist"); }); }); + + describe("account activity", () => { + let state: FakeGlobalState>; + + beforeEach(() => { + state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + }); + describe("accountActivity$", () => { + it("returns the account activity state", async () => { + state.stateSubject.next({ + [toId("user1")]: new Date(1), + [toId("user2")]: new Date(2), + }); + + await expect(firstValueFrom(sut.accountActivity$)).resolves.toEqual({ + [toId("user1")]: new Date(1), + [toId("user2")]: new Date(2), + }); + }); + + it("returns an empty object when account activity is null", async () => { + state.stateSubject.next(null); + + await expect(firstValueFrom(sut.accountActivity$)).resolves.toEqual({}); + }); + }); + + describe("sortedUserIds$", () => { + it("returns the sorted user ids by date with most recent first", async () => { + state.stateSubject.next({ + [toId("user1")]: new Date(3), + [toId("user2")]: new Date(2), + [toId("user3")]: new Date(1), + }); + + await expect(firstValueFrom(sut.sortedUserIds$)).resolves.toEqual([ + "user1" as UserId, + "user2" as UserId, + "user3" as UserId, + ]); + }); + + it("returns an empty array when account activity is null", async () => { + state.stateSubject.next(null); + + await expect(firstValueFrom(sut.sortedUserIds$)).resolves.toEqual([]); + }); + }); + + describe("setAccountActivity", () => { + const userId = Utils.newGuid() as UserId; + it("sets the account activity", async () => { + await sut.setAccountActivity(userId, new Date(1)); + + expect(state.nextMock).toHaveBeenCalledWith({ [userId]: new Date(1) }); + }); + + it("does not update if the activity is the same", async () => { + state.stateSubject.next({ [userId]: new Date(1) }); + + await sut.setAccountActivity(userId, new Date(1)); + + expect(state.nextMock).not.toHaveBeenCalled(); + }); + + it.each([null, undefined, 123, "not a guid"])( + "does not set last active if the userId is not a valid guid", + async (userId) => { + await sut.setAccountActivity(userId as UserId, new Date(1)); + + expect(state.nextMock).not.toHaveBeenCalled(); + }, + ); + }); + }); }); + +function toId(userId: string) { + return userId as UserId; +} diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 77d61fae9136..6740387ded8d 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -1,4 +1,4 @@ -import { Subject, combineLatestWith, map, distinctUntilChanged, shareReplay } from "rxjs"; +import { combineLatestWith, map, distinctUntilChanged, shareReplay, combineLatest } from "rxjs"; import { AccountInfo, @@ -7,8 +7,9 @@ import { } from "../../auth/abstractions/account.service"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { Utils } from "../../platform/misc/utils"; import { - ACCOUNT_MEMORY, + ACCOUNT_DISK, GlobalState, GlobalStateProvider, KeyDefinition, @@ -16,25 +17,36 @@ import { import { UserId } from "../../types/guid"; export const ACCOUNT_ACCOUNTS = KeyDefinition.record( - ACCOUNT_MEMORY, + ACCOUNT_DISK, "accounts", { deserializer: (accountInfo) => accountInfo, }, ); -export const ACCOUNT_ACTIVE_ACCOUNT_ID = new KeyDefinition(ACCOUNT_MEMORY, "activeAccountId", { +export const ACCOUNT_ACTIVE_ACCOUNT_ID = new KeyDefinition(ACCOUNT_DISK, "activeAccountId", { deserializer: (id: UserId) => id, }); +export const ACCOUNT_ACTIVITY = KeyDefinition.record(ACCOUNT_DISK, "activity", { + deserializer: (activity) => new Date(activity), +}); + +const LOGGED_OUT_INFO: AccountInfo = { + email: "", + emailVerified: false, + name: undefined, +}; + export class AccountServiceImplementation implements InternalAccountService { - private lock = new Subject(); - private logout = new Subject(); private accountsState: GlobalState>; private activeAccountIdState: GlobalState; accounts$; activeAccount$; + accountActivity$; + sortedUserIds$; + nextUpAccount$; constructor( private messagingService: MessagingService, @@ -53,14 +65,40 @@ export class AccountServiceImplementation implements InternalAccountService { distinctUntilChanged((a, b) => a?.id === b?.id && accountInfoEqual(a, b)), shareReplay({ bufferSize: 1, refCount: false }), ); + this.accountActivity$ = this.globalStateProvider + .get(ACCOUNT_ACTIVITY) + .state$.pipe(map((activity) => activity ?? {})); + this.sortedUserIds$ = this.accountActivity$.pipe( + map((activity) => { + return Object.entries(activity) + .map(([userId, lastActive]: [UserId, Date]) => ({ userId, lastActive })) + .sort((a, b) => b.lastActive.getTime() - a.lastActive.getTime()) // later dates first + .map((a) => a.userId); + }), + ); + this.nextUpAccount$ = combineLatest([ + this.accounts$, + this.activeAccount$, + this.sortedUserIds$, + ]).pipe( + map(([accounts, activeAccount, sortedUserIds]) => { + const nextId = sortedUserIds.find((id) => id !== activeAccount?.id && accounts[id] != null); + return nextId ? { id: nextId, ...accounts[nextId] } : null; + }), + ); } async addAccount(userId: UserId, accountData: AccountInfo): Promise { + if (!Utils.isGuid(userId)) { + throw new Error("userId is required"); + } + await this.accountsState.update((accounts) => { accounts ||= {}; accounts[userId] = accountData; return accounts; }); + await this.setAccountActivity(userId, new Date()); } async setAccountName(userId: UserId, name: string): Promise { @@ -71,6 +109,15 @@ export class AccountServiceImplementation implements InternalAccountService { await this.setAccountInfo(userId, { email }); } + async setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise { + await this.setAccountInfo(userId, { emailVerified }); + } + + async clean(userId: UserId) { + await this.setAccountInfo(userId, LOGGED_OUT_INFO); + await this.removeAccountActivity(userId); + } + async switchAccount(userId: UserId): Promise { await this.activeAccountIdState.update( (_, accounts) => { @@ -94,6 +141,37 @@ export class AccountServiceImplementation implements InternalAccountService { ); } + async setAccountActivity(userId: UserId, lastActivity: Date): Promise { + if (!Utils.isGuid(userId)) { + // only store for valid userIds + return; + } + + await this.globalStateProvider.get(ACCOUNT_ACTIVITY).update( + (activity) => { + activity ||= {}; + activity[userId] = lastActivity; + return activity; + }, + { + shouldUpdate: (oldActivity) => oldActivity?.[userId]?.getTime() !== lastActivity?.getTime(), + }, + ); + } + + async removeAccountActivity(userId: UserId): Promise { + await this.globalStateProvider.get(ACCOUNT_ACTIVITY).update( + (activity) => { + if (activity == null) { + return activity; + } + delete activity[userId]; + return activity; + }, + { shouldUpdate: (oldActivity) => oldActivity?.[userId] != null }, + ); + } + // TODO: update to use our own account status settings. Requires inverting direction of state service accounts flow async delete(): Promise { try { diff --git a/libs/common/src/auth/services/auth.service.spec.ts b/libs/common/src/auth/services/auth.service.spec.ts index 3bdf85d3e156..9a93a4207b72 100644 --- a/libs/common/src/auth/services/auth.service.spec.ts +++ b/libs/common/src/auth/services/auth.service.spec.ts @@ -56,6 +56,7 @@ describe("AuthService", () => { status: AuthenticationStatus.Unlocked, id: userId, email: "email", + emailVerified: false, name: "name", }; @@ -109,6 +110,7 @@ describe("AuthService", () => { status: AuthenticationStatus.Unlocked, id: Utils.newGuid() as UserId, email: "email2", + emailVerified: false, name: "name2", }; @@ -126,7 +128,11 @@ describe("AuthService", () => { it("requests auth status for all known users", async () => { const userId2 = Utils.newGuid() as UserId; - await accountService.addAccount(userId2, { email: "email2", name: "name2" }); + await accountService.addAccount(userId2, { + email: "email2", + emailVerified: false, + name: "name2", + }); const mockFn = jest.fn().mockReturnValue(of(AuthenticationStatus.Locked)); sut.authStatusFor$ = mockFn; @@ -147,11 +153,14 @@ describe("AuthService", () => { cryptoService.getInMemoryUserKeyFor$.mockReturnValue(of(undefined)); }); - it("emits LoggedOut when userId is null", async () => { - expect(await firstValueFrom(sut.authStatusFor$(null))).toEqual( - AuthenticationStatus.LoggedOut, - ); - }); + it.each([null, undefined, "not a userId"])( + "emits LoggedOut when userId is invalid (%s)", + async () => { + expect(await firstValueFrom(sut.authStatusFor$(null))).toEqual( + AuthenticationStatus.LoggedOut, + ); + }, + ); it("emits LoggedOut when there is no access token", async () => { tokenService.hasAccessToken$.mockReturnValue(of(false)); diff --git a/libs/common/src/auth/services/auth.service.ts b/libs/common/src/auth/services/auth.service.ts index c9e711b4cc56..a4529084a2a4 100644 --- a/libs/common/src/auth/services/auth.service.ts +++ b/libs/common/src/auth/services/auth.service.ts @@ -2,6 +2,7 @@ import { Observable, combineLatest, distinctUntilChanged, + firstValueFrom, map, of, shareReplay, @@ -12,6 +13,7 @@ import { ApiService } from "../../abstractions/api.service"; import { CryptoService } from "../../platform/abstractions/crypto.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; import { StateService } from "../../platform/abstractions/state.service"; +import { Utils } from "../../platform/misc/utils"; import { UserId } from "../../types/guid"; import { AccountService } from "../abstractions/account.service"; import { AuthService as AuthServiceAbstraction } from "../abstractions/auth.service"; @@ -39,13 +41,16 @@ export class AuthService implements AuthServiceAbstraction { this.authStatuses$ = this.accountService.accounts$.pipe( map((accounts) => Object.keys(accounts) as UserId[]), - switchMap((entries) => - combineLatest( + switchMap((entries) => { + if (entries.length === 0) { + return of([] as { userId: UserId; status: AuthenticationStatus }[]); + } + return combineLatest( entries.map((userId) => this.authStatusFor$(userId).pipe(map((status) => ({ userId, status }))), ), - ), - ), + ); + }), map((statuses) => { return statuses.reduce( (acc, { userId, status }) => { @@ -59,7 +64,7 @@ export class AuthService implements AuthServiceAbstraction { } authStatusFor$(userId: UserId): Observable { - if (userId == null) { + if (!Utils.isGuid(userId)) { return of(AuthenticationStatus.LoggedOut); } @@ -84,17 +89,8 @@ export class AuthService implements AuthServiceAbstraction { } async getAuthStatus(userId?: string): Promise { - // If we don't have an access token or userId, we're logged out - const isAuthenticated = await this.stateService.getIsAuthenticated({ userId: userId }); - if (!isAuthenticated) { - return AuthenticationStatus.LoggedOut; - } - - // Note: since we aggresively set the auto user key to memory if it exists on app init (see InitService) - // we only need to check if the user key is in memory. - const hasUserKey = await this.cryptoService.hasUserKeyInMemory(userId as UserId); - - return hasUserKey ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked; + userId ??= await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); + return await firstValueFrom(this.authStatusFor$(userId as UserId)); } logOut(callback: () => void) { diff --git a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts index fc5060af5fdb..19b29f059326 100644 --- a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts +++ b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts @@ -90,6 +90,7 @@ describe("PasswordResetEnrollmentServiceImplementation", () => { const user1AccountInfo: AccountInfo = { name: "Test User 1", email: "test1@email.com", + emailVerified: true, }; activeAccountSubject.next(Object.assign(user1AccountInfo, { id: "userId" as UserId })); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 13c33305d1f4..5ca604b52602 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -25,11 +25,10 @@ export type InitOptions = { export abstract class StateService { accounts$: Observable<{ [userId: string]: T }>; - activeAccount$: Observable; addAccount: (account: T) => Promise; - setActiveUser: (userId: string) => Promise; - clean: (options?: StorageOptions) => Promise; + clearDecryptedData: (userId: UserId) => Promise; + clean: (options?: StorageOptions) => Promise; init: (initOptions?: InitOptions) => Promise; /** @@ -122,8 +121,6 @@ export abstract class StateService { setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; getEmail: (options?: StorageOptions) => Promise; setEmail: (value: string, options?: StorageOptions) => Promise; - getEmailVerified: (options?: StorageOptions) => Promise; - setEmailVerified: (value: boolean, options?: StorageOptions) => Promise; getEnableBrowserIntegration: (options?: StorageOptions) => Promise; setEnableBrowserIntegration: (value: boolean, options?: StorageOptions) => Promise; getEnableBrowserIntegrationFingerprint: (options?: StorageOptions) => Promise; @@ -147,8 +144,6 @@ export abstract class StateService { */ setEncryptedPinProtected: (value: string, options?: StorageOptions) => Promise; getIsAuthenticated: (options?: StorageOptions) => Promise; - getLastActive: (options?: StorageOptions) => Promise; - setLastActive: (value: number, options?: StorageOptions) => Promise; getLastSync: (options?: StorageOptions) => Promise; setLastSync: (value: string, options?: StorageOptions) => Promise; getMinimizeOnCopyToClipboard: (options?: StorageOptions) => Promise; @@ -180,5 +175,4 @@ export abstract class StateService { setVaultTimeout: (value: number, options?: StorageOptions) => Promise; getVaultTimeoutAction: (options?: StorageOptions) => Promise; setVaultTimeoutAction: (value: string, options?: StorageOptions) => Promise; - nextUpActiveUser: () => Promise; } diff --git a/libs/common/src/platform/misc/utils.spec.ts b/libs/common/src/platform/misc/utils.spec.ts index a7a520a77c95..964a2a19413e 100644 --- a/libs/common/src/platform/misc/utils.spec.ts +++ b/libs/common/src/platform/misc/utils.spec.ts @@ -3,6 +3,33 @@ import * as path from "path"; import { Utils } from "./utils"; describe("Utils Service", () => { + describe("isGuid", () => { + it("is false when null", () => { + expect(Utils.isGuid(null)).toBe(false); + }); + + it("is false when undefined", () => { + expect(Utils.isGuid(undefined)).toBe(false); + }); + + it("is false when empty", () => { + expect(Utils.isGuid("")).toBe(false); + }); + + it("is false when not a string", () => { + expect(Utils.isGuid(123 as any)).toBe(false); + }); + + it("is false when not a guid", () => { + expect(Utils.isGuid("not a guid")).toBe(false); + }); + + it("is true when a guid", () => { + // we use a limited guid scope in which all zeroes is invalid + expect(Utils.isGuid("00000000-0000-1000-8000-000000000000")).toBe(true); + }); + }); + describe("getDomain", () => { it("should fail for invalid urls", () => { expect(Utils.getDomain(null)).toBeNull(); diff --git a/libs/common/src/platform/models/domain/state.ts b/libs/common/src/platform/models/domain/state.ts index 95557e082a98..5dde49f99dbe 100644 --- a/libs/common/src/platform/models/domain/state.ts +++ b/libs/common/src/platform/models/domain/state.ts @@ -9,9 +9,6 @@ export class State< > { accounts: { [userId: string]: TAccount } = {}; globals: TGlobalState; - activeUserId: string; - authenticatedAccounts: string[] = []; - accountActivity: { [userId: string]: number } = {}; constructor(globals: TGlobalState) { this.globals = globals; diff --git a/libs/common/src/platform/services/default-environment.service.spec.ts b/libs/common/src/platform/services/default-environment.service.spec.ts index dd504dc30236..7d266e93fc3b 100644 --- a/libs/common/src/platform/services/default-environment.service.spec.ts +++ b/libs/common/src/platform/services/default-environment.service.spec.ts @@ -31,10 +31,12 @@ describe("EnvironmentService", () => { [testUser]: { name: "name", email: "email", + emailVerified: false, }, [alternateTestUser]: { name: "name", email: "email", + emailVerified: false, }, }); stateProvider = new FakeStateProvider(accountService); @@ -47,6 +49,7 @@ describe("EnvironmentService", () => { id: userId, email: "test@example.com", name: `Test Name ${userId}`, + emailVerified: false, }); await awaitAsync(); }; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index cab5768d2afd..9479d6471098 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, firstValueFrom, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { AccountService } from "../../auth/abstractions/account.service"; @@ -33,10 +33,7 @@ const keys = { state: "state", stateVersion: "stateVersion", global: "global", - authenticatedAccounts: "authenticatedAccounts", - activeUserId: "activeUserId", tempAccountSettings: "tempAccountSettings", // used to hold account specific settings (i.e clear clipboard) between initial migration and first account authentication - accountActivity: "accountActivity", }; const partialKeys = { @@ -58,9 +55,6 @@ export class StateService< protected accountsSubject = new BehaviorSubject<{ [userId: string]: TAccount }>({}); accounts$ = this.accountsSubject.asObservable(); - protected activeAccountSubject = new BehaviorSubject(null); - activeAccount$ = this.activeAccountSubject.asObservable(); - private hasBeenInited = false; protected isRecoveredSession = false; @@ -112,36 +106,16 @@ export class StateService< } // Get all likely authenticated accounts - const authenticatedAccounts = ( - (await this.storageService.get(keys.authenticatedAccounts)) ?? [] - ).filter((account) => account != null); + const authenticatedAccounts = await firstValueFrom( + this.accountService.accounts$.pipe(map((accounts) => Object.keys(accounts))), + ); await this.updateState(async (state) => { for (const i in authenticatedAccounts) { state = await this.syncAccountFromDisk(authenticatedAccounts[i]); } - // After all individual accounts have been added - state.authenticatedAccounts = authenticatedAccounts; - - const storedActiveUser = await this.storageService.get(keys.activeUserId); - if (storedActiveUser != null) { - state.activeUserId = storedActiveUser; - } await this.pushAccounts(); - this.activeAccountSubject.next(state.activeUserId); - // TODO: Temporary update to avoid routing all account status changes through account service for now. - // account service tracks logged out accounts, but State service does not, so we need to add the active account - // if it's not in the accounts list. - if (state.activeUserId != null && this.accountsSubject.value[state.activeUserId] == null) { - const activeDiskAccount = await this.getAccountFromDisk({ userId: state.activeUserId }); - await this.accountService.addAccount(state.activeUserId as UserId, { - name: activeDiskAccount.profile.name, - email: activeDiskAccount.profile.email, - }); - } - await this.accountService.switchAccount(state.activeUserId as UserId); - // End TODO return state; }); @@ -161,61 +135,25 @@ export class StateService< return state; }); - // TODO: Temporary update to avoid routing all account status changes through account service for now. - // The determination of state should be handled by the various services that control those values. - await this.accountService.addAccount(userId as UserId, { - name: diskAccount.profile.name, - email: diskAccount.profile.email, - }); - return state; } async addAccount(account: TAccount) { await this.environmentService.seedUserEnvironment(account.profile.userId as UserId); await this.updateState(async (state) => { - state.authenticatedAccounts.push(account.profile.userId); - await this.storageService.save(keys.authenticatedAccounts, state.authenticatedAccounts); state.accounts[account.profile.userId] = account; return state; }); await this.scaffoldNewAccountStorage(account); - await this.setLastActive(new Date().getTime(), { userId: account.profile.userId }); - // TODO: Temporary update to avoid routing all account status changes through account service for now. - await this.accountService.addAccount(account.profile.userId as UserId, { - name: account.profile.name, - email: account.profile.email, - }); - await this.setActiveUser(account.profile.userId); - } - - async setActiveUser(userId: string): Promise { - await this.clearDecryptedDataForActiveUser(); - await this.updateState(async (state) => { - state.activeUserId = userId; - await this.storageService.save(keys.activeUserId, userId); - this.activeAccountSubject.next(state.activeUserId); - // TODO: temporary update to avoid routing all account status changes through account service for now. - await this.accountService.switchAccount(userId as UserId); - - return state; - }); - - await this.pushAccounts(); } - async clean(options?: StorageOptions): Promise { + async clean(options?: StorageOptions): Promise { options = this.reconcileOptions(options, await this.defaultInMemoryOptions()); await this.deAuthenticateAccount(options.userId); - let currentUser = (await this.state())?.activeUserId; - if (options.userId === currentUser) { - currentUser = await this.dynamicallySetActiveUser(); - } await this.removeAccountFromDisk(options?.userId); await this.removeAccountFromMemory(options?.userId); await this.pushAccounts(); - return currentUser as UserId; } /** @@ -515,24 +453,6 @@ export class StateService< ); } - async getEmailVerified(options?: StorageOptions): Promise { - return ( - (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.profile.emailVerified ?? false - ); - } - - async setEmailVerified(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.profile.emailVerified = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getEnableBrowserIntegration(options?: StorageOptions): Promise { return ( (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) @@ -642,35 +562,6 @@ export class StateService< ); } - async getLastActive(options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskOptions()); - - const accountActivity = await this.storageService.get<{ [userId: string]: number }>( - keys.accountActivity, - options, - ); - - if (accountActivity == null || Object.keys(accountActivity).length < 1) { - return null; - } - - return accountActivity[options.userId]; - } - - async setLastActive(value: number, options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskOptions()); - if (options.userId == null) { - return; - } - const accountActivity = - (await this.storageService.get<{ [userId: string]: number }>( - keys.accountActivity, - options, - )) ?? {}; - accountActivity[options.userId] = value; - await this.storageService.save(keys.accountActivity, accountActivity, options); - } - async getLastSync(options?: StorageOptions): Promise { return ( await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions())) @@ -910,24 +801,28 @@ export class StateService< } protected async getAccountFromMemory(options: StorageOptions): Promise { + const userId = + options.userId ?? + (await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + )); + return await this.state().then(async (state) => { if (state.accounts == null) { return null; } - return state.accounts[await this.getUserIdFromMemory(options)]; - }); - } - - protected async getUserIdFromMemory(options: StorageOptions): Promise { - return await this.state().then((state) => { - return options?.userId != null - ? state.accounts[options.userId]?.profile?.userId - : state.activeUserId; + return state.accounts[userId]; }); } protected async getAccountFromDisk(options: StorageOptions): Promise { - if (options?.userId == null && (await this.state())?.activeUserId == null) { + const userId = + options.userId ?? + (await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + )); + + if (userId == null) { return null; } @@ -1086,53 +981,76 @@ export class StateService< } protected async defaultInMemoryOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Memory, - userId: (await this.state()).activeUserId, + userId, }; } protected async defaultOnDiskOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Session, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, useSecureStorage: false, }; } protected async defaultOnDiskLocalOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Local, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, useSecureStorage: false, }; } protected async defaultOnDiskMemoryOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Memory, - userId: (await this.state())?.activeUserId ?? (await this.getUserId()), + userId, useSecureStorage: false, }; } protected async defaultSecureStorageOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, useSecureStorage: true, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, }; } protected async getActiveUserIdFromStorage(): Promise { - return await this.storageService.get(keys.activeUserId); + return await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); } protected async removeAccountFromLocalStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + const storedAccount = await this.getAccount( this.reconcileOptions({ userId: userId }, await this.defaultOnDiskLocalOptions()), ); @@ -1143,7 +1061,10 @@ export class StateService< } protected async removeAccountFromSessionStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + const storedAccount = await this.getAccount( this.reconcileOptions({ userId: userId }, await this.defaultOnDiskOptions()), ); @@ -1154,7 +1075,10 @@ export class StateService< } protected async removeAccountFromSecureStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + await this.setUserKeyAutoUnlock(null, { userId: userId }); await this.setUserKeyBiometric(null, { userId: userId }); await this.setCryptoMasterKeyAuto(null, { userId: userId }); @@ -1163,8 +1087,11 @@ export class StateService< } protected async removeAccountFromMemory(userId: string = null): Promise { + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + await this.updateState(async (state) => { - userId = userId ?? state.activeUserId; delete state.accounts[userId]; return state; }); @@ -1178,15 +1105,16 @@ export class StateService< return Object.assign(this.createAccount(), persistentAccountInformation); } - protected async clearDecryptedDataForActiveUser(): Promise { + async clearDecryptedData(userId: UserId): Promise { await this.updateState(async (state) => { - const userId = state?.activeUserId; if (userId != null && state?.accounts[userId]?.data != null) { state.accounts[userId].data = new AccountData(); } return state; }); + + await this.pushAccounts(); } protected createAccount(init: Partial = null): TAccount { @@ -1201,14 +1129,6 @@ export class StateService< // We must have a manual call to clear tokens as we can't leverage state provider to clean // up our data as we have secure storage in the mix. await this.tokenService.clearTokens(userId as UserId); - await this.setLastActive(null, { userId: userId }); - await this.updateState(async (state) => { - state.authenticatedAccounts = state.authenticatedAccounts.filter((id) => id !== userId); - - await this.storageService.save(keys.authenticatedAccounts, state.authenticatedAccounts); - - return state; - }); } protected async removeAccountFromDisk(userId: string) { @@ -1217,32 +1137,6 @@ export class StateService< await this.removeAccountFromSecureStorage(userId); } - async nextUpActiveUser() { - const accounts = (await this.state())?.accounts; - if (accounts == null || Object.keys(accounts).length < 1) { - return null; - } - - let newActiveUser; - for (const userId in accounts) { - if (userId == null) { - continue; - } - if (await this.getIsAuthenticated({ userId: userId })) { - newActiveUser = userId; - break; - } - newActiveUser = null; - } - return newActiveUser as UserId; - } - - protected async dynamicallySetActiveUser() { - const newActiveUser = await this.nextUpActiveUser(); - await this.setActiveUser(newActiveUser); - return newActiveUser; - } - protected async saveSecureStorageKey( key: string, value: T, diff --git a/libs/common/src/platform/services/system.service.ts b/libs/common/src/platform/services/system.service.ts index d19390c45e01..80053673d8a6 100644 --- a/libs/common/src/platform/services/system.service.ts +++ b/libs/common/src/platform/services/system.service.ts @@ -1,10 +1,12 @@ -import { firstValueFrom, timeout } from "rxjs"; +import { firstValueFrom, map, timeout } from "rxjs"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountService } from "../../auth/abstractions/account.service"; import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AutofillSettingsServiceAbstraction } from "../../autofill/services/autofill-settings.service"; import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; +import { UserId } from "../../types/guid"; import { MessagingService } from "../abstractions/messaging.service"; import { PlatformUtilsService } from "../abstractions/platform-utils.service"; import { StateService } from "../abstractions/state.service"; @@ -25,15 +27,18 @@ export class SystemService implements SystemServiceAbstraction { private autofillSettingsService: AutofillSettingsServiceAbstraction, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private biometricStateService: BiometricStateService, + private accountService: AccountService, ) {} async startProcessReload(authService: AuthService): Promise { - const accounts = await firstValueFrom(this.stateService.accounts$); + const accounts = await firstValueFrom(this.accountService.accounts$); if (accounts != null) { const keys = Object.keys(accounts); if (keys.length > 0) { for (const userId of keys) { - if ((await authService.getAuthStatus(userId)) === AuthenticationStatus.Unlocked) { + let status = await firstValueFrom(authService.authStatusFor$(userId as UserId)); + status = await authService.getAuthStatus(userId); + if (status === AuthenticationStatus.Unlocked) { return; } } @@ -63,15 +68,24 @@ export class SystemService implements SystemServiceAbstraction { clearInterval(this.reloadInterval); this.reloadInterval = null; - const currentUser = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500))); + const currentUser = await firstValueFrom( + this.accountService.activeAccount$.pipe( + map((a) => a?.id), + timeout(500), + ), + ); // Replace current active user if they will be logged out on reload if (currentUser != null) { const timeoutAction = await firstValueFrom( this.vaultTimeoutSettingsService.vaultTimeoutAction$().pipe(timeout(500)), ); if (timeoutAction === VaultTimeoutAction.LogOut) { - const nextUser = await this.stateService.nextUpActiveUser(); - await this.stateService.setActiveUser(nextUser); + const nextUser = await firstValueFrom( + this.accountService.nextUpAccount$.pipe(map((account) => account?.id ?? null)), + ); + // Can be removed once we migrate password generation history to state providers + await this.stateService.clearDecryptedData(currentUser); + await this.accountService.switchAccount(nextUser); } } diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts index c1cc15a176f7..681963f82334 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts @@ -1,7 +1,6 @@ import { mock } from "jest-mock-extended"; import { mockAccountServiceWith, trackEmissions } from "../../../../spec"; -import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { UserId } from "../../../types/guid"; import { SingleUserStateProvider } from "../user-state.provider"; @@ -14,7 +13,7 @@ describe("DefaultActiveUserStateProvider", () => { id: userId, name: "name", email: "email", - status: AuthenticationStatus.Locked, + emailVerified: false, }; const accountService = mockAccountServiceWith(userId, accountInfo); let sut: DefaultActiveUserStateProvider; diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts index 51a972a9dc69..c652136a0d12 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts @@ -82,6 +82,7 @@ describe("DefaultActiveUserState", () => { activeAccountSubject.next({ id: userId, email: `test${id}@example.com`, + emailVerified: false, name: `Test User ${id}`, }); await awaitAsync(); diff --git a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts index 3243b53d6702..98d423cf4845 100644 --- a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts @@ -69,7 +69,12 @@ describe("DefaultStateProvider", () => { userId?: UserId, ) => Observable, ) => { - const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; + const accountInfo = { + email: "email", + emailVerified: false, + name: "name", + status: AuthenticationStatus.LoggedOut, + }; const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { deserializer: (s) => s, }); @@ -114,7 +119,12 @@ describe("DefaultStateProvider", () => { ); describe("getUserState$", () => { - const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; + const accountInfo = { + email: "email", + emailVerified: false, + name: "name", + status: AuthenticationStatus.LoggedOut, + }; const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { deserializer: (s) => s, }); diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index ee5005202fa4..6b309ecfb94b 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -38,6 +38,7 @@ export const BILLING_DISK = new StateDefinition("billing", "disk"); export const KDF_CONFIG_DISK = new StateDefinition("kdfConfig", "disk"); export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); export const ACCOUNT_MEMORY = new StateDefinition("account", "memory"); +export const ACCOUNT_DISK = new StateDefinition("account", "disk"); export const MASTER_PASSWORD_MEMORY = new StateDefinition("masterPassword", "memory"); export const MASTER_PASSWORD_DISK = new StateDefinition("masterPassword", "disk"); export const TWO_FACTOR_MEMORY = new StateDefinition("twoFactor", "memory"); diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts index 5344093a25af..12c24dcdef39 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts @@ -1,9 +1,10 @@ import { MockProxy, any, mock } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountInfo } from "../../auth/abstractions/account.service"; import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { FakeMasterPasswordService } from "../../auth/services/master-password/fake-master-password.service"; @@ -13,7 +14,6 @@ import { MessagingService } from "../../platform/abstractions/messaging.service" import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { StateService } from "../../platform/abstractions/state.service"; import { Utils } from "../../platform/misc/utils"; -import { Account } from "../../platform/models/domain/account"; import { StateEventRunnerService } from "../../platform/state"; import { UserId } from "../../types/guid"; import { CipherService } from "../../vault/abstractions/cipher.service"; @@ -39,7 +39,6 @@ describe("VaultTimeoutService", () => { let lockedCallback: jest.Mock, [userId: string]>; let loggedOutCallback: jest.Mock, [expired: boolean, userId?: string]>; - let accountsSubject: BehaviorSubject>; let vaultTimeoutActionSubject: BehaviorSubject; let availableVaultTimeoutActionsSubject: BehaviorSubject; @@ -65,10 +64,6 @@ describe("VaultTimeoutService", () => { lockedCallback = jest.fn(); loggedOutCallback = jest.fn(); - accountsSubject = new BehaviorSubject(null); - - stateService.accounts$ = accountsSubject; - vaultTimeoutActionSubject = new BehaviorSubject(VaultTimeoutAction.Lock); vaultTimeoutSettingsService.vaultTimeoutAction$.mockReturnValue(vaultTimeoutActionSubject); @@ -127,21 +122,39 @@ describe("VaultTimeoutService", () => { return Promise.resolve(accounts[userId]?.vaultTimeout); }); - stateService.getLastActive.mockImplementation((options) => { - return Promise.resolve(accounts[options.userId]?.lastActive); - }); - stateService.getUserId.mockResolvedValue(globalSetups?.userId); - stateService.activeAccount$ = new BehaviorSubject(globalSetups?.userId); - + // Set desired user active and known users on accounts service : note the only thing that matters here is that the ID are set if (globalSetups?.userId) { accountService.activeAccountSubject.next({ id: globalSetups.userId as UserId, email: null, + emailVerified: false, name: null, }); } + accountService.accounts$ = of( + Object.entries(accounts).reduce( + (agg, [id]) => { + agg[id] = { + email: "", + emailVerified: true, + name: "", + }; + return agg; + }, + {} as Record, + ), + ); + accountService.accountActivity$ = of( + Object.entries(accounts).reduce( + (agg, [id, info]) => { + agg[id] = info.lastActive ? new Date(info.lastActive) : null; + return agg; + }, + {} as Record, + ), + ); platformUtilsService.isViewOpen.mockResolvedValue(globalSetups?.isViewOpen ?? false); @@ -158,16 +171,6 @@ describe("VaultTimeoutService", () => { ], ); }); - - const accountsSubjectValue: Record = Object.keys(accounts).reduce( - (agg, key) => { - const newPartial: Record = {}; - newPartial[key] = null; // No values actually matter on this other than the key - return Object.assign(agg, newPartial); - }, - {} as Record, - ); - accountsSubject.next(accountsSubjectValue); }; const expectUserToHaveLocked = (userId: string) => { diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.ts index 8baf6c04c491..8e0978d07d03 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, timeout } from "rxjs"; +import { combineLatest, firstValueFrom, switchMap } from "rxjs"; import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; @@ -64,14 +64,25 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { // Get whether or not the view is open a single time so it can be compared for each user const isViewOpen = await this.platformUtilsService.isViewOpen(); - const activeUserId = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500))); - - const accounts = await firstValueFrom(this.stateService.accounts$); - for (const userId in accounts) { - if (userId != null && (await this.shouldLock(userId, activeUserId, isViewOpen))) { - await this.executeTimeoutAction(userId); - } - } + await firstValueFrom( + combineLatest([ + this.accountService.activeAccount$, + this.accountService.accountActivity$, + ]).pipe( + switchMap(async ([activeAccount, accountActivity]) => { + const activeUserId = activeAccount?.id; + for (const userIdString in accountActivity) { + const userId = userIdString as UserId; + if ( + userId != null && + (await this.shouldLock(userId, accountActivity[userId], activeUserId, isViewOpen)) + ) { + await this.executeTimeoutAction(userId); + } + } + }), + ), + ); } async lock(userId?: string): Promise { @@ -123,6 +134,7 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { private async shouldLock( userId: string, + lastActive: Date, activeUserId: string, isViewOpen: boolean, ): Promise { @@ -146,13 +158,12 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { return false; } - const lastActive = await this.stateService.getLastActive({ userId: userId }); if (lastActive == null) { return false; } const vaultTimeoutSeconds = vaultTimeout * 60; - const diffSeconds = (new Date().getTime() - lastActive) / 1000; + const diffSeconds = (new Date().getTime() - lastActive.getTime()) / 1000; return diffSeconds >= vaultTimeoutSeconds; } diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 31bc5460b436..0a1f4b1d1100 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -57,13 +57,14 @@ import { CipherServiceMigrator } from "./migrations/57-move-cipher-service-to-st import { RemoveRefreshTokenMigratedFlagMigrator } from "./migrations/58-remove-refresh-token-migrated-state-provider-flag"; import { KdfConfigMigrator } from "./migrations/59-move-kdf-config-to-state-provider"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; +import { KnownAccountsMigrator } from "./migrations/60-known-accounts"; import { MoveBiometricAutoPromptToAccount } from "./migrations/7-move-biometric-auto-prompt-to-account"; import { MoveStateVersionMigrator } from "./migrations/8-move-state-version"; import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-settings-to-global"; import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 3; -export const CURRENT_VERSION = 59; +export const CURRENT_VERSION = 60; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -124,7 +125,8 @@ export function createMigrationBuilder() { .with(AuthRequestMigrator, 55, 56) .with(CipherServiceMigrator, 56, 57) .with(RemoveRefreshTokenMigratedFlagMigrator, 57, 58) - .with(KdfConfigMigrator, 58, CURRENT_VERSION); + .with(KdfConfigMigrator, 58, 59) + .with(KnownAccountsMigrator, 59, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migration-helper.spec.ts b/libs/common/src/state-migrations/migration-helper.spec.ts index 5f366f259724..162fac2fabc4 100644 --- a/libs/common/src/state-migrations/migration-helper.spec.ts +++ b/libs/common/src/state-migrations/migration-helper.spec.ts @@ -27,6 +27,14 @@ const exampleJSON = { }, global_serviceName_key: "global_serviceName_key", user_userId_serviceName_key: "user_userId_serviceName_key", + global_account_accounts: { + "c493ed01-4e08-4e88-abc7-332f380ca760": { + otherStuff: "otherStuff3", + }, + "23e61a5f-2ece-4f5e-b499-f0bc489482a9": { + otherStuff: "otherStuff4", + }, + }, }; describe("RemoveLegacyEtmKeyMigrator", () => { @@ -81,6 +89,41 @@ describe("RemoveLegacyEtmKeyMigrator", () => { const accounts = await sut.getAccounts(); expect(accounts).toEqual([]); }); + + it("handles global scoped known accounts for version 60 and after", async () => { + sut.currentVersion = 60; + const accounts = await sut.getAccounts(); + expect(accounts).toEqual([ + // Note, still gets values stored in state service objects, just grabs user ids from global + { + userId: "c493ed01-4e08-4e88-abc7-332f380ca760", + account: { otherStuff: "otherStuff1" }, + }, + { + userId: "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + account: { otherStuff: "otherStuff2" }, + }, + ]); + }); + }); + + describe("getKnownUserIds", () => { + it("returns all user ids", async () => { + const userIds = await sut.getKnownUserIds(); + expect(userIds).toEqual([ + "c493ed01-4e08-4e88-abc7-332f380ca760", + "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + ]); + }); + + it("returns all user ids when version is 60 or greater", async () => { + sut.currentVersion = 60; + const userIds = await sut.getKnownUserIds(); + expect(userIds).toEqual([ + "c493ed01-4e08-4e88-abc7-332f380ca760", + "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + ]); + }); }); describe("getFromGlobal", () => { diff --git a/libs/common/src/state-migrations/migration-helper.ts b/libs/common/src/state-migrations/migration-helper.ts index 2505e2b264a5..5d1de8dd49e9 100644 --- a/libs/common/src/state-migrations/migration-helper.ts +++ b/libs/common/src/state-migrations/migration-helper.ts @@ -162,7 +162,7 @@ export class MigrationHelper { async getAccounts(): Promise< { userId: string; account: ExpectedAccountType }[] > { - const userIds = (await this.get("authenticatedAccounts")) ?? []; + const userIds = await this.getKnownUserIds(); return Promise.all( userIds.map(async (userId) => ({ userId, @@ -171,6 +171,17 @@ export class MigrationHelper { ); } + /** + * Helper method to read known users ids. + */ + async getKnownUserIds(): Promise { + if (this.currentVersion < 61) { + return knownAccountUserIdsBuilderPre61(this.storageService); + } else { + return knownAccountUserIdsBuilder(this.storageService); + } + } + /** * Builds a user storage key appropriate for the current version. * @@ -233,3 +244,18 @@ function globalKeyBuilder(keyDefinition: KeyDefinitionLike): string { function globalKeyBuilderPre9(): string { throw Error("No key builder should be used for versions prior to 9."); } + +async function knownAccountUserIdsBuilderPre61( + storageService: AbstractStorageService, +): Promise { + return (await storageService.get("authenticatedAccounts")) ?? []; +} + +async function knownAccountUserIdsBuilder( + storageService: AbstractStorageService, +): Promise { + const accounts = await storageService.get>( + globalKeyBuilder({ stateDefinition: { name: "account" }, key: "accounts" }), + ); + return Object.keys(accounts ?? {}); +} diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts new file mode 100644 index 000000000000..28dedb3c390c --- /dev/null +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts @@ -0,0 +1,145 @@ +import { MockProxy } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { + ACCOUNT_ACCOUNTS, + ACCOUNT_ACTIVE_ACCOUNT_ID, + ACCOUNT_ACTIVITY, + KnownAccountsMigrator, +} from "./60-known-accounts"; + +const migrateJson = () => { + return { + authenticatedAccounts: ["user1", "user2"], + activeUserId: "user1", + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + accountActivity: { + user1: 1609459200000, // 2021-01-01 + user2: 1609545600000, // 2021-01-02 + }, + }; +}; + +const rollbackJson = () => { + return { + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + global_account_accounts: { + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + }, + global_account_activeAccountId: "user1", + global_account_activity: { + user1: "2021-01-01T00:00:00.000Z", + user2: "2021-01-02T00:00:00.000Z", + }, + }; +}; + +describe("ReplicateKnownAccounts", () => { + let helper: MockProxy; + let sut: KnownAccountsMigrator; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(migrateJson(), 59); + sut = new KnownAccountsMigrator(59, 60); + }); + + it("migrates accounts", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACCOUNTS, { + user1: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + user2: { + email: "", + emailVerified: false, + name: undefined, + }, + }); + expect(helper.remove).toHaveBeenCalledWith("authenticatedAccounts"); + }); + + it("migrates active account it", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVE_ACCOUNT_ID, "user1"); + expect(helper.remove).toHaveBeenCalledWith("activeUserId"); + }); + + it("migrates account activity", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVITY, { + user1: '"2021-01-01T00:00:00.000Z"', + user2: '"2021-01-02T00:00:00.000Z"', + }); + expect(helper.remove).toHaveBeenCalledWith("accountActivity"); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJson(), 60); + sut = new KnownAccountsMigrator(59, 60); + }); + + it("rolls back authenticated accounts", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("authenticatedAccounts", ["user1", "user2"]); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACCOUNTS); + }); + + it("rolls back active account id", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("activeUserId", "user1"); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVE_ACCOUNT_ID); + }); + + it("rolls back account activity", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("accountActivity", { + user1: 1609459200000, + user2: 1609545600000, + }); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVITY); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.ts new file mode 100644 index 000000000000..75117da5b476 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.ts @@ -0,0 +1,111 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +export const ACCOUNT_ACCOUNTS: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "accounts", +}; + +export const ACCOUNT_ACTIVE_ACCOUNT_ID: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "activeAccountId", +}; + +export const ACCOUNT_ACTIVITY: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "activity", +}; + +type ExpectedAccountType = { + profile?: { + email?: string; + name?: string; + emailVerified?: boolean; + }; +}; + +export class KnownAccountsMigrator extends Migrator<59, 60> { + async migrate(helper: MigrationHelper): Promise { + await this.migrateAuthenticatedAccounts(helper); + await this.migrateActiveAccountId(helper); + await this.migrateAccountActivity(helper); + } + async rollback(helper: MigrationHelper): Promise { + // authenticated account are removed, but the accounts record also contains logged out accounts. Best we can do is to add them all back + const accounts = (await helper.getFromGlobal>(ACCOUNT_ACCOUNTS)) ?? {}; + await helper.set("authenticatedAccounts", Object.keys(accounts)); + await helper.removeFromGlobal(ACCOUNT_ACCOUNTS); + + // Active Account Id + const activeAccountId = await helper.getFromGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID); + if (activeAccountId) { + await helper.set("activeUserId", activeAccountId); + } + await helper.removeFromGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID); + + // Account Activity + const accountActivity = await helper.getFromGlobal>(ACCOUNT_ACTIVITY); + if (accountActivity) { + const toStore = Object.entries(accountActivity).reduce( + (agg, [userId, dateString]) => { + agg[userId] = new Date(dateString).getTime(); + return agg; + }, + {} as Record, + ); + await helper.set("accountActivity", toStore); + } + await helper.removeFromGlobal(ACCOUNT_ACTIVITY); + } + + private async migrateAuthenticatedAccounts(helper: MigrationHelper) { + const authenticatedAccounts = (await helper.get("authenticatedAccounts")) ?? []; + const accounts = await Promise.all( + authenticatedAccounts.map(async (userId) => { + const account = await helper.get(userId); + return { userId, account }; + }), + ); + const accountsToStore = accounts.reduce( + (agg, { userId, account }) => { + if (account?.profile) { + agg[userId] = { + email: account.profile.email ?? "", + emailVerified: account.profile.emailVerified ?? false, + name: account.profile.name, + }; + } + return agg; + }, + {} as Record, + ); + + await helper.setToGlobal(ACCOUNT_ACCOUNTS, accountsToStore); + await helper.remove("authenticatedAccounts"); + } + + private async migrateAccountActivity(helper: MigrationHelper) { + const stored = await helper.get>("accountActivity"); + const accountActivity = Object.entries(stored ?? {}).reduce( + (agg, [userId, dateMs]) => { + agg[userId] = JSON.stringify(new Date(dateMs)); + return agg; + }, + {} as Record, + ); + await helper.setToGlobal(ACCOUNT_ACTIVITY, accountActivity); + await helper.remove("accountActivity"); + } + + private async migrateActiveAccountId(helper: MigrationHelper) { + const activeAccountId = await helper.get("activeUserId"); + await helper.setToGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID, activeAccountId); + await helper.remove("activeUserId"); + } +} diff --git a/libs/common/src/tools/send/services/send.service.spec.ts b/libs/common/src/tools/send/services/send.service.spec.ts index 41183c42af04..2f0f50c6168b 100644 --- a/libs/common/src/tools/send/services/send.service.spec.ts +++ b/libs/common/src/tools/send/services/send.service.spec.ts @@ -62,6 +62,7 @@ describe("SendService", () => { accountService.activeAccountSubject.next({ id: mockUserId, email: "email", + emailVerified: false, name: "name", }); diff --git a/libs/common/src/vault/services/sync/sync.service.ts b/libs/common/src/vault/services/sync/sync.service.ts index 73869ff488e6..995ab7319b20 100644 --- a/libs/common/src/vault/services/sync/sync.service.ts +++ b/libs/common/src/vault/services/sync/sync.service.ts @@ -326,7 +326,10 @@ export class SyncService implements SyncServiceAbstraction { await this.cryptoService.setOrgKeys(response.organizations, response.providerOrganizations); await this.avatarService.setSyncAvatarColor(response.id as UserId, response.avatarColor); await this.tokenService.setSecurityStamp(response.securityStamp, response.id as UserId); - await this.stateService.setEmailVerified(response.emailVerified); + await this.accountService.setAccountEmailVerified( + response.id as UserId, + response.emailVerified, + ); await this.billingAccountProfileStateService.setHasPremium( response.premiumPersonally,