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

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<keyof AccountInfo>;
+  for (const key of keys) {
+    if (a[key] !== b[key]) {
+      return false;
+    }
+  }
+  return true;
 }
 
 export abstract class AccountService {
   accounts$: Observable<Record<UserId, AccountInfo>>;
   activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>;
+
+  /**
+   * Observable of the last activity time for each account.
+   */
+  accountActivity$: Observable<Record<UserId, Date>>;
+  /** Account list in order of descending recency */
+  sortedUserIds$: Observable<UserId[]>;
+  /** 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<void>;
+  /**
+   * updates the `accounts$` observable with the new email verification status for the account.
+   * @param userId
+   * @param emailVerified
+   */
+  abstract setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise<void>;
   /**
    * Updates the `activeAccount$` observable with the new active account.
    * @param userId
    */
   abstract switchAccount(userId: UserId): Promise<void>;
+  /**
+   * 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<void>;
+  /**
+   * Updates the given user's last activity time.
+   * @param userId
+   * @param lastActivity
+   */
+  abstract setAccountActivity(userId: UserId, lastActivity: Date): Promise<void>;
 }
 
 export abstract class InternalAccountService extends AccountService {

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<MessagingService>;
   let logService: MockProxy<LogService>;
@@ -22,8 +69,8 @@ describe("accountService", () => {
   let sut: AccountServiceImplementation;
   let accountsState: FakeGlobalState<Record<UserId, AccountInfo>>;
   let activeAccountIdState: FakeGlobalState<UserId>;
-  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<Record<UserId, Date>>;
+
+    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;
+}

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<AccountInfo, UserId>(
-  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<Date, UserId>(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<UserId>();
-  private logout = new Subject<UserId>();
   private accountsState: GlobalState<Record<UserId, AccountInfo>>;
   private activeAccountIdState: GlobalState<UserId | undefined>;
 
   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<void> {
+    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<void> {
@@ -71,6 +109,15 @@ export class AccountServiceImplementation implements InternalAccountService {
     await this.setAccountInfo(userId, { email });
   }
 
+  async setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise<void> {
+    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<void> {
     await this.activeAccountIdState.update(
       (_, accounts) => {
@@ -94,6 +141,37 @@ export class AccountServiceImplementation implements InternalAccountService {
     );
   }
 
+  async setAccountActivity(userId: UserId, lastActivity: Date): Promise<void> {
+    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<void> {
+    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<void> {
     try {

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));

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<AuthenticationStatus> {
-    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<AuthenticationStatus> {
-    // 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) {

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 }));