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

apps/browser/src/auth/popup/account-switching/account-switcher.component.html

@@ -49,7 +49,7 @@
         <button
           type="button"
           class="account-switcher-row tw-flex tw-w-full tw-items-center tw-gap-3 tw-rounded-md tw-p-3 disabled:tw-cursor-not-allowed disabled:tw-border-text-muted/60 disabled:!tw-text-muted/60"
-          (click)="lock()"
+          (click)="lock(currentAccount.id)"
           [disabled]="currentAccount.status === lockedStatus || !activeUserCanLock"
           [title]="!activeUserCanLock ? ('unlockMethodNeeded' | i18n) : ''"
         >
@@ -59,7 +59,7 @@
         <button
           type="button"
           class="account-switcher-row tw-flex tw-w-full tw-items-center tw-gap-3 tw-rounded-md tw-p-3"
-          (click)="logOut()"
+          (click)="logOut(currentAccount.id)"
         >
           <i class="bwi bwi-sign-out tw-text-2xl" aria-hidden="true"></i>
           {{ "logOut" | i18n }}

apps/browser/src/auth/popup/account-switching/account-switcher.component.ts

@@ -10,6 +10,7 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
+import { UserId } from "@bitwarden/common/types/guid";
 import { DialogService } from "@bitwarden/components";
 
 import { AccountSwitcherService } from "./services/account-switcher.service";
@@ -64,9 +65,9 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
     this.location.back();
   }
 
-  async lock(userId?: string) {
+  async lock(userId: string) {
     this.loading = true;
-    await this.vaultTimeoutService.lock(userId ? userId : null);
+    await this.vaultTimeoutService.lock(userId);
     // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
     this.router.navigate(["lock"]);
@@ -96,7 +97,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
       .subscribe(() => this.router.navigate(["lock"]));
   }
 
-  async logOut() {
+  async logOut(userId: UserId) {
     this.loading = true;
     const confirmed = await this.dialogService.openSimpleDialog({
       title: { key: "logOut" },
@@ -105,7 +106,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
     });
 
     if (confirmed) {
-      this.messagingService.send("logout");
+      this.messagingService.send("logout", { userId });
     }
 
     // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.

apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts

@@ -58,6 +58,7 @@ describe("AccountSwitcherService", () => {
       const accountInfo: AccountInfo = {
         name: "Test User 1",
         email: "test1@email.com",
+        emailVerified: true,
       };
 
       avatarService.getUserAvatarColor$.mockReturnValue(of("#cccccc"));
@@ -89,6 +90,7 @@ describe("AccountSwitcherService", () => {
         for (let i = 0; i < numberOfAccounts; i++) {
           seedAccounts[`${i}` as UserId] = {
             email: `test${i}@email.com`,
+            emailVerified: true,
             name: "Test User ${i}",
           };
           seedStatuses[`${i}` as UserId] = AuthenticationStatus.Unlocked;
@@ -113,6 +115,7 @@ describe("AccountSwitcherService", () => {
       const user1AccountInfo: AccountInfo = {
         name: "Test User 1",
         email: "",
+        emailVerified: true,
       };
       accountsSubject.next({ ["1" as UserId]: user1AccountInfo });
       authStatusSubject.next({ ["1" as UserId]: AuthenticationStatus.LoggedOut });

apps/browser/src/auth/popup/lock.component.ts

@@ -59,7 +59,7 @@ export class LockComponent extends BaseLockComponent {
     policyApiService: PolicyApiServiceAbstraction,
     policyService: InternalPolicyService,
     passwordStrengthService: PasswordStrengthServiceAbstraction,
-    private authService: AuthService,
+    authService: AuthService,
     dialogService: DialogService,
     deviceTrustService: DeviceTrustServiceAbstraction,
     userVerificationService: UserVerificationService,
@@ -92,6 +92,7 @@ export class LockComponent extends BaseLockComponent {
       pinCryptoService,
       biometricStateService,
       accountService,
+      authService,
       kdfConfigService,
     );
     this.successRoute = "/tabs/current";

apps/browser/src/autofill/browser/context-menu-clicked-handler.spec.ts

@@ -11,7 +11,8 @@ import {
   GENERATE_PASSWORD_ID,
   NOOP_COMMAND_SUFFIX,
 } from "@bitwarden/common/autofill/constants";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
+import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service";
 import { CipherType } from "@bitwarden/common/vault/enums";
@@ -65,7 +66,7 @@ describe("ContextMenuClickedHandler", () => {
   let autofill: AutofillAction;
   let authService: MockProxy<AuthService>;
   let cipherService: MockProxy<CipherService>;
-  let stateService: MockProxy<StateService>;
+  let accountService: FakeAccountService;
   let totpService: MockProxy<TotpService>;
   let eventCollectionService: MockProxy<EventCollectionService>;
   let userVerificationService: MockProxy<UserVerificationService>;
@@ -78,7 +79,7 @@ describe("ContextMenuClickedHandler", () => {
     autofill = jest.fn<Promise<void>, [tab: chrome.tabs.Tab, cipher: CipherView]>();
     authService = mock();
     cipherService = mock();
-    stateService = mock();
+    accountService = mockAccountServiceWith("userId" as UserId);
     totpService = mock();
     eventCollectionService = mock();
 
@@ -88,10 +89,10 @@ describe("ContextMenuClickedHandler", () => {
       autofill,
       authService,
       cipherService,
-      stateService,
       totpService,
       eventCollectionService,
       userVerificationService,
+      accountService,
     );
   });
 

apps/browser/src/autofill/browser/context-menu-clicked-handler.ts

@@ -1,4 +1,7 @@
+import { firstValueFrom, map } from "rxjs";
+
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
@@ -17,7 +20,6 @@ import {
   NOOP_COMMAND_SUFFIX,
 } from "@bitwarden/common/autofill/constants";
 import { EventType } from "@bitwarden/common/enums";
-import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
 import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
@@ -26,6 +28,7 @@ import { CipherType } from "@bitwarden/common/vault/enums";
 import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
+import { accountServiceFactory } from "../../auth/background/service-factories/account-service.factory";
 import {
   authServiceFactory,
   AuthServiceInitOptions,
@@ -37,7 +40,6 @@ import { autofillSettingsServiceFactory } from "../../autofill/background/servic
 import { eventCollectionServiceFactory } from "../../background/service-factories/event-collection-service.factory";
 import { Account } from "../../models/account";
 import { CachedServices } from "../../platform/background/service-factories/factory-options";
-import { stateServiceFactory } from "../../platform/background/service-factories/state-service.factory";
 import { BrowserApi } from "../../platform/browser/browser-api";
 import { passwordGenerationServiceFactory } from "../../tools/background/service_factories/password-generation-service.factory";
 import {
@@ -71,10 +73,10 @@ export class ContextMenuClickedHandler {
     private autofillAction: AutofillAction,
     private authService: AuthService,
     private cipherService: CipherService,
-    private stateService: StateService,
     private totpService: TotpService,
     private eventCollectionService: EventCollectionService,
     private userVerificationService: UserVerificationService,
+    private accountService: AccountService,
   ) {}
 
   static async mv3Create(cachedServices: CachedServices) {
@@ -128,10 +130,10 @@ export class ContextMenuClickedHandler {
       (tab, cipher) => autofillCommand.doAutofillTabWithCipherCommand(tab, cipher),
       await authServiceFactory(cachedServices, serviceOptions),
       await cipherServiceFactory(cachedServices, serviceOptions),
-      await stateServiceFactory(cachedServices, serviceOptions),
       await totpServiceFactory(cachedServices, serviceOptions),
       await eventCollectionServiceFactory(cachedServices, serviceOptions),
       await userVerificationServiceFactory(cachedServices, serviceOptions),
+      await accountServiceFactory(cachedServices, serviceOptions),
     );
   }
 
@@ -239,9 +241,10 @@ export class ContextMenuClickedHandler {
       return;
     }
 
-    // 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(new Date().getTime());
+    const activeUserId = await firstValueFrom(
+      this.accountService.activeAccount$.pipe(map((a) => a?.id)),
+    );
+    await this.accountService.setAccountActivity(activeUserId, new Date());
     switch (info.parentMenuItemId) {
       case AUTOFILL_ID:
       case AUTOFILL_IDENTITY_ID:

apps/browser/src/background/main.background.ts

@@ -1,4 +1,4 @@
-import { Subject, firstValueFrom, merge, timeout } from "rxjs";
+import { Subject, firstValueFrom, map, merge, timeout } from "rxjs";
 
 import {
   PinCryptoServiceAbstraction,
@@ -902,6 +902,7 @@ export default class MainBackground {
       this.autofillSettingsService,
       this.vaultTimeoutSettingsService,
       this.biometricStateService,
+      this.accountService,
     );
 
     // Other fields
@@ -920,7 +921,6 @@ export default class MainBackground {
         this.autofillService,
         this.platformUtilsService as BrowserPlatformUtilsService,
         this.notificationsService,
-        this.stateService,
         this.autofillSettingsService,
         this.systemService,
         this.environmentService,
@@ -929,6 +929,7 @@ export default class MainBackground {
         this.configService,
         this.fido2Background,
         messageListener,
+        this.accountService,
       );
       this.nativeMessagingBackground = new NativeMessagingBackground(
         this.accountService,
@@ -1018,10 +1019,10 @@ export default class MainBackground {
         },
         this.authService,
         this.cipherService,
-        this.stateService,
         this.totpService,
         this.eventCollectionService,
         this.userVerificationService,
+        this.accountService,
       );
 
       this.contextMenusBackground = new ContextMenusBackground(contextMenuClickedHandler);
@@ -1168,7 +1169,12 @@ export default class MainBackground {
    */
   async switchAccount(userId: UserId) {
     try {
-      await this.stateService.setActiveUser(userId);
+      const currentlyActiveAccount = await firstValueFrom(
+        this.accountService.activeAccount$.pipe(map((account) => account?.id)),
+      );
+      // can be removed once password generation history is migrated to state providers
+      await this.stateService.clearDecryptedData(currentlyActiveAccount);
+      await this.accountService.switchAccount(userId);
 
       if (userId == null) {
         this.loginEmailService.setRememberEmail(false);
@@ -1240,7 +1246,11 @@ export default class MainBackground {
     //Needs to be checked before state is cleaned
     const needStorageReseed = await this.needsStorageReseed();
 
-    const newActiveUser = await this.stateService.clean({ userId: userId });
+    const newActiveUser = await firstValueFrom(
+      this.accountService.nextUpAccount$.pipe(map((a) => a?.id)),
+    );
+    await this.stateService.clean({ userId: userId });
+    await this.accountService.clean(userId);
 
     await this.stateEventRunnerService.handleEvent("logout", userId);
 

apps/browser/src/background/runtime.background.ts

@@ -1,6 +1,7 @@
-import { firstValueFrom, mergeMap } from "rxjs";
+import { firstValueFrom, map, mergeMap } from "rxjs";
 
 import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants";
 import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
@@ -19,7 +20,6 @@ import {
 import { LockedVaultPendingNotificationsData } from "../autofill/background/abstractions/notification.background";
 import { AutofillService } from "../autofill/services/abstractions/autofill.service";
 import { BrowserApi } from "../platform/browser/browser-api";
-import { BrowserStateService } from "../platform/services/abstractions/browser-state.service";
 import { BrowserEnvironmentService } from "../platform/services/browser-environment.service";
 import { BrowserPlatformUtilsService } from "../platform/services/platform-utils/browser-platform-utils.service";
 import { Fido2Background } from "../vault/fido2/background/abstractions/fido2.background";
@@ -37,7 +37,6 @@ export default class RuntimeBackground {
     private autofillService: AutofillService,
     private platformUtilsService: BrowserPlatformUtilsService,
     private notificationsService: NotificationsService,
-    private stateService: BrowserStateService,
     private autofillSettingsService: AutofillSettingsServiceAbstraction,
     private systemService: SystemService,
     private environmentService: BrowserEnvironmentService,
@@ -46,6 +45,7 @@ export default class RuntimeBackground {
     private configService: ConfigService,
     private fido2Background: Fido2Background,
     private messageListener: MessageListener,
+    private accountService: AccountService,
   ) {
     // onInstalled listener must be wired up before anything else, so we do it in the ctor
     chrome.runtime.onInstalled.addListener((details: any) => {
@@ -111,9 +111,10 @@ export default class RuntimeBackground {
         switch (msg.sender) {
           case "autofiller":
           case "autofill_cmd": {
-            // 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(new Date().getTime());
+            const activeUserId = await firstValueFrom(
+              this.accountService.activeAccount$.pipe(map((a) => a?.id)),
+            );
+            await this.accountService.setAccountActivity(activeUserId, new Date());
             const totpCode = await this.autofillService.doAutoFillActiveTab(
               [
                 {

apps/browser/src/platform/popup/header.component.ts

@@ -1,10 +1,8 @@
 import { Component, Input } from "@angular/core";
-import { Observable, combineLatest, map, of, switchMap } from "rxjs";
+import { Observable, map, of, switchMap } from "rxjs";
 
-import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
-import { UserId } from "@bitwarden/common/types/guid";
 
 import { enableAccountSwitching } from "../flags";
 
@@ -16,18 +14,15 @@ export class HeaderComponent {
   @Input() noTheme = false;
   @Input() hideAccountSwitcher = false;
   authedAccounts$: Observable<boolean>;
-  constructor(accountService: AccountService, authService: AuthService) {
-    this.authedAccounts$ = accountService.accounts$.pipe(
-      switchMap((accounts) => {
+  constructor(authService: AuthService) {
+    this.authedAccounts$ = authService.authStatuses$.pipe(
+      map((record) => Object.values(record)),
+      switchMap((statuses) => {
         if (!enableAccountSwitching()) {
           return of(false);
         }
 
-        return combineLatest(
-          Object.keys(accounts).map((id) => authService.authStatusFor$(id as UserId)),
-        ).pipe(
-          map((statuses) => statuses.some((status) => status !== AuthenticationStatus.LoggedOut)),
-        );
+        return of(statuses.some((status) => status !== AuthenticationStatus.LoggedOut));
       }),
     );
   }

apps/browser/src/platform/services/browser-state.service.spec.ts

@@ -1,5 +1,4 @@
 import { mock, MockProxy } from "jest-mock-extended";
-import { firstValueFrom } from "rxjs";
 
 import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
@@ -50,7 +49,6 @@ describe("Browser State Service", () => {
     state.accounts[userId] = new Account({
       profile: { userId: userId },
     });
-    state.activeUserId = userId;
   });
 
   afterEach(() => {
@@ -78,18 +76,8 @@ describe("Browser State Service", () => {
       );
     });
 
-    describe("add Account", () => {
-      it("should add account", async () => {
-        const newUserId = "newUserId" as UserId;
-        const newAcct = new Account({
-          profile: { userId: newUserId },
-        });
-
-        await sut.addAccount(newAcct);
-
-        const accts = await firstValueFrom(sut.accounts$);
-        expect(accts[newUserId]).toBeDefined();
-      });
+    it("exists", () => {
+      expect(sut).toBeDefined();
     });
   });
 });