refactor(active-user-state-refactor): [PM-12040] Remove ActiveUserStatus For SSO Login Component (#13149)

* refactor(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - First pass of work to update the state. In the middle of testing.

* fix(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - Fix for jslib-services.module.ts

* fix(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - Fix main.background.ts

* test(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - Added simple tests

* fix(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - Tiny touchups.

* fix(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - Few fixes to resolve comments.

* fix(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - Changed place where userId is loaded.

* test(active-user-state-refactor): [PM-12040] Remove ActiveUserState from SSO Service - Fixed test.

libs/common/src/auth/abstractions/sso-login.service.abstraction.ts

@@ -1,5 +1,7 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
+import { UserId } from "@bitwarden/common/types/guid";
+
 export abstract class SsoLoginServiceAbstraction {
   /**
    * Gets the code verifier used for SSO.
@@ -74,12 +76,16 @@ export abstract class SsoLoginServiceAbstraction {
    * Gets the value of the active user's organization sso identifier.
    *
    * This should only be used post successful SSO login once the user is initialized.
+   * @param userId The user id for retrieving the org identifier state.
    */
-  getActiveUserOrganizationSsoIdentifier: () => Promise<string>;
+  getActiveUserOrganizationSsoIdentifier: (userId: UserId) => Promise<string>;
   /**
    * Sets the value of the active user's organization sso identifier.
    *
    * This should only be used post successful SSO login once the user is initialized.
    */
-  setActiveUserOrganizationSsoIdentifier: (organizationIdentifier: string) => Promise<void>;
+  setActiveUserOrganizationSsoIdentifier: (
+    organizationIdentifier: string,
+    userId: UserId | undefined,
+  ) => Promise<void>;
 }

libs/common/src/auth/services/sso-login.service.spec.ts

@@ -0,0 +1,94 @@
+import { mock, MockProxy } from "jest-mock-extended";
+
+import {
+  CODE_VERIFIER,
+  GLOBAL_ORGANIZATION_SSO_IDENTIFIER,
+  SSO_EMAIL,
+  SSO_STATE,
+  SsoLoginService,
+  USER_ORGANIZATION_SSO_IDENTIFIER,
+} from "@bitwarden/common/auth/services/sso-login.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { UserId } from "@bitwarden/common/types/guid";
+
+import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../spec";
+
+describe("SSOLoginService ", () => {
+  let sut: SsoLoginService;
+
+  let accountService: FakeAccountService;
+  let mockSingleUserStateProvider: FakeStateProvider;
+  let mockLogService: MockProxy<LogService>;
+  let userId: UserId;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+
+    userId = Utils.newGuid() as UserId;
+    accountService = mockAccountServiceWith(userId);
+    mockSingleUserStateProvider = new FakeStateProvider(accountService);
+    mockLogService = mock<LogService>();
+
+    sut = new SsoLoginService(mockSingleUserStateProvider, mockLogService);
+  });
+
+  it("instantiates", () => {
+    expect(sut).not.toBeFalsy();
+  });
+
+  it("gets and sets code verifier", async () => {
+    const codeVerifier = "test-code-verifier";
+    await sut.setCodeVerifier(codeVerifier);
+    mockSingleUserStateProvider.getGlobal(CODE_VERIFIER);
+
+    const result = await sut.getCodeVerifier();
+    expect(result).toBe(codeVerifier);
+  });
+
+  it("gets and sets SSO state", async () => {
+    const ssoState = "test-sso-state";
+    await sut.setSsoState(ssoState);
+    mockSingleUserStateProvider.getGlobal(SSO_STATE);
+
+    const result = await sut.getSsoState();
+    expect(result).toBe(ssoState);
+  });
+
+  it("gets and sets organization SSO identifier", async () => {
+    const orgIdentifier = "test-org-identifier";
+    await sut.setOrganizationSsoIdentifier(orgIdentifier);
+    mockSingleUserStateProvider.getGlobal(GLOBAL_ORGANIZATION_SSO_IDENTIFIER);
+
+    const result = await sut.getOrganizationSsoIdentifier();
+    expect(result).toBe(orgIdentifier);
+  });
+
+  it("gets and sets SSO email", async () => {
+    const email = "test@example.com";
+    await sut.setSsoEmail(email);
+    mockSingleUserStateProvider.getGlobal(SSO_EMAIL);
+
+    const result = await sut.getSsoEmail();
+    expect(result).toBe(email);
+  });
+
+  it("gets and sets active user organization SSO identifier", async () => {
+    const userId = Utils.newGuid() as UserId;
+    const orgIdentifier = "test-active-org-identifier";
+    await sut.setActiveUserOrganizationSsoIdentifier(orgIdentifier, userId);
+    mockSingleUserStateProvider.getUser(userId, USER_ORGANIZATION_SSO_IDENTIFIER);
+
+    const result = await sut.getActiveUserOrganizationSsoIdentifier(userId);
+    expect(result).toBe(orgIdentifier);
+  });
+
+  it("logs error when setting active user organization SSO identifier with undefined userId", async () => {
+    const orgIdentifier = "test-active-org-identifier";
+    await sut.setActiveUserOrganizationSsoIdentifier(orgIdentifier, undefined);
+
+    expect(mockLogService.warning).toHaveBeenCalledWith(
+      "Tried to set a user organization sso identifier with an undefined user id.",
+    );
+  });
+});

libs/common/src/auth/services/sso-login.service.ts

@@ -2,10 +2,13 @@
 // @ts-strict-ignore
 import { firstValueFrom } from "rxjs";
 
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { UserId } from "@bitwarden/common/types/guid";
+
 import {
-  ActiveUserState,
   GlobalState,
   KeyDefinition,
+  SingleUserState,
   SSO_DISK,
   StateProvider,
   UserKeyDefinition,
@@ -15,21 +18,21 @@ import { SsoLoginServiceAbstraction } from "../abstractions/sso-login.service.ab
 /**
  * Uses disk storage so that the code verifier can be persisted across sso redirects.
  */
-const CODE_VERIFIER = new KeyDefinition<string>(SSO_DISK, "ssoCodeVerifier", {
+export const CODE_VERIFIER = new KeyDefinition<string>(SSO_DISK, "ssoCodeVerifier", {
   deserializer: (codeVerifier) => codeVerifier,
 });
 
 /**
  * Uses disk storage so that the sso state can be persisted across sso redirects.
  */
-const SSO_STATE = new KeyDefinition<string>(SSO_DISK, "ssoState", {
+export const SSO_STATE = new KeyDefinition<string>(SSO_DISK, "ssoState", {
   deserializer: (state) => state,
 });
 
 /**
  * Uses disk storage so that the organization sso identifier can be persisted across sso redirects.
  */
-const USER_ORGANIZATION_SSO_IDENTIFIER = new UserKeyDefinition<string>(
+export const USER_ORGANIZATION_SSO_IDENTIFIER = new UserKeyDefinition<string>(
   SSO_DISK,
   "organizationSsoIdentifier",
   {
@@ -41,7 +44,7 @@ const USER_ORGANIZATION_SSO_IDENTIFIER = new UserKeyDefinition<string>(
 /**
  * Uses disk storage so that the organization sso identifier can be persisted across sso redirects.
  */
-const GLOBAL_ORGANIZATION_SSO_IDENTIFIER = new KeyDefinition<string>(
+export const GLOBAL_ORGANIZATION_SSO_IDENTIFIER = new KeyDefinition<string>(
   SSO_DISK,
   "organizationSsoIdentifier",
   {
@@ -52,7 +55,7 @@ const GLOBAL_ORGANIZATION_SSO_IDENTIFIER = new KeyDefinition<string>(
 /**
  * Uses disk storage so that the user's email can be persisted across sso redirects.
  */
-const SSO_EMAIL = new KeyDefinition<string>(SSO_DISK, "ssoEmail", {
+export const SSO_EMAIL = new KeyDefinition<string>(SSO_DISK, "ssoEmail", {
   deserializer: (state) => state,
 });
 
@@ -61,16 +64,15 @@ export class SsoLoginService implements SsoLoginServiceAbstraction {
   private ssoState: GlobalState<string>;
   private orgSsoIdentifierState: GlobalState<string>;
   private ssoEmailState: GlobalState<string>;
-  private activeUserOrgSsoIdentifierState: ActiveUserState<string>;
 
-  constructor(private stateProvider: StateProvider) {
+  constructor(
+    private stateProvider: StateProvider,
+    private logService: LogService,
+  ) {
     this.codeVerifierState = this.stateProvider.getGlobal(CODE_VERIFIER);
     this.ssoState = this.stateProvider.getGlobal(SSO_STATE);
     this.orgSsoIdentifierState = this.stateProvider.getGlobal(GLOBAL_ORGANIZATION_SSO_IDENTIFIER);
     this.ssoEmailState = this.stateProvider.getGlobal(SSO_EMAIL);
-    this.activeUserOrgSsoIdentifierState = this.stateProvider.getActive(
-      USER_ORGANIZATION_SSO_IDENTIFIER,
-    );
   }
 
   getCodeVerifier(): Promise<string> {
@@ -105,11 +107,24 @@ export class SsoLoginService implements SsoLoginServiceAbstraction {
     await this.ssoEmailState.update((_) => email);
   }
 
-  getActiveUserOrganizationSsoIdentifier(): Promise<string> {
-    return firstValueFrom(this.activeUserOrgSsoIdentifierState.state$);
+  getActiveUserOrganizationSsoIdentifier(userId: UserId): Promise<string> {
+    return firstValueFrom(this.userOrgSsoIdentifierState(userId).state$);
   }
 
-  async setActiveUserOrganizationSsoIdentifier(organizationIdentifier: string): Promise<void> {
-    await this.activeUserOrgSsoIdentifierState.update((_) => organizationIdentifier);
+  async setActiveUserOrganizationSsoIdentifier(
+    organizationIdentifier: string,
+    userId: UserId | undefined,
+  ): Promise<void> {
+    if (userId === undefined) {
+      this.logService.warning(
+        "Tried to set a user organization sso identifier with an undefined user id.",
+      );
+      return;
+    }
+    await this.userOrgSsoIdentifierState(userId).update((_) => organizationIdentifier);
+  }
+
+  private userOrgSsoIdentifierState(userId: UserId): SingleUserState<string> {
+    return this.stateProvider.getUser(userId, USER_ORGANIZATION_SSO_IDENTIFIER);
   }
 }

libs/common/src/platform/state/global-state.ts

@@ -9,7 +9,7 @@ import { StateUpdateOptions } from "./state-update-options";
 export interface GlobalState<T> {
   /**
    * Method for allowing you to manipulate state in an additive way.
-   * @param configureState callback for how you want manipulate this section of state
+   * @param configureState callback for how you want to manipulate this section of state
    * @param options Defaults given by @see {module:state-update-options#DEFAULT_OPTIONS}
    * @param options.shouldUpdate A callback for determining if you want to update state. Defaults to () => true
    * @param options.combineLatestWith An observable that you want to combine with the current state for callbacks. Defaults to null

libs/common/src/platform/state/user-state.ts

@@ -16,6 +16,7 @@ export interface UserState<T> {
 }
 
 export const activeMarker: unique symbol = Symbol("active");
+
 export interface ActiveUserState<T> extends UserState<T> {
   readonly [activeMarker]: true;
 
@@ -32,7 +33,7 @@ export interface ActiveUserState<T> extends UserState<T> {
    * @param options.shouldUpdate A callback for determining if you want to update state. Defaults to () => true
    * @param options.combineLatestWith An observable that you want to combine with the current state for callbacks. Defaults to null
    * @param options.msTimeout A timeout for how long you are willing to wait for a `combineLatestWith` option to complete. Defaults to 1000ms. Only applies if `combineLatestWith` is set.
-
+   *
    * @returns A promise that must be awaited before your next action to ensure the update has been written to state.
    * Resolves to the new state. If `shouldUpdate` returns false, the promise will resolve to the current state.
    */
@@ -41,6 +42,7 @@ export interface ActiveUserState<T> extends UserState<T> {
     options?: StateUpdateOptions<T, TCombine>,
   ) => Promise<[UserId, T]>;
 }
+
 export interface SingleUserState<T> extends UserState<T> {
   readonly userId: UserId;
 
@@ -51,7 +53,7 @@ export interface SingleUserState<T> extends UserState<T> {
    * @param options.shouldUpdate A callback for determining if you want to update state. Defaults to () => true
    * @param options.combineLatestWith An observable that you want to combine with the current state for callbacks. Defaults to null
    * @param options.msTimeout A timeout for how long you are willing to wait for a `combineLatestWith` option to complete. Defaults to 1000ms. Only applies if `combineLatestWith` is set.
-
+   *
    * @returns A promise that must be awaited before your next action to ensure the update has been written to state.
    * Resolves to the new state. If `shouldUpdate` returns false, the promise will resolve to the current state.
    */