Auth/PM-19555 - Fix multi account logout on lock screens not redirecting properly

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

@@ -4,7 +4,7 @@
 import { Subject, firstValueFrom, map, of, startWith, switchMap } from "rxjs";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
-import { LockService } from "@bitwarden/auth/common";
+import { LockService, LogoutService } from "@bitwarden/auth/common";
 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";
@@ -70,6 +70,7 @@
     private vaultTimeoutSettingsService: VaultTimeoutSettingsService,
     private authService: AuthService,
     private lockService: LockService,
+    private logoutService: LogoutService,
   ) {}
 
   get accountLimit() {
@@ -141,12 +142,9 @@
     });
 
     if (confirmed) {
-      const result = await this.accountSwitcherService.logoutAccount(userId);
-      // unlocked logout responses need to be navigated out of the account switcher.
-      // other responses will be handled by background and app.component
-      if (result?.status === AuthenticationStatus.Unlocked) {
-        this.location.back();
-      }
+      await this.logoutService.logout(userId);
+      // navigate to root so redirect guard can properly route next active user or null user to correct page
+      await this.router.navigate(["/"]);
     }
     this.loading = false;
   }

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

@@ -1,7 +1,8 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
-import { CommonModule, Location } from "@angular/common";
+import { CommonModule } from "@angular/common";
 import { Component, EventEmitter, Input, Output } from "@angular/core";
+import { Router } from "@angular/router";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
@@ -24,7 +25,7 @@
 
   constructor(
     private accountSwitcherService: AccountSwitcherService,
-    private location: Location,
+    private router: Router,
     private i18nService: I18nService,
     private logService: LogService,
     private biometricsService: BiometricsService,
@@ -45,8 +46,8 @@
 
     // Navigate out of account switching for unlocked accounts
     // locked or logged out account statuses are handled by background and app.component
-    if (result?.status === AuthenticationStatus.Unlocked) {
-      this.location.back();
+    if (result?.authenticationStatus === AuthenticationStatus.Unlocked) {
+      await this.router.navigate(["vault"]);
       await this.biometricsService.setShouldAutopromptNow(false);
     } else {
       await this.biometricsService.setShouldAutopromptNow(true);

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

@@ -207,35 +207,4 @@ describe("AccountSwitcherService", () => {
       expect(removeListenerSpy).toBeCalledTimes(1);
     });
   });
-
-  describe("logout", () => {
-    const userId1 = "1" as UserId;
-    const userId2 = "2" as UserId;
-    it("initiates logout", async () => {
-      let listener: (
-        message: { command: string; userId: UserId; status: AuthenticationStatus },
-        sender: unknown,
-        sendResponse: unknown,
-      ) => void;
-      jest.spyOn(chrome.runtime.onMessage, "addListener").mockImplementation((addedListener) => {
-        listener = addedListener;
-      });
-
-      const removeListenerSpy = jest.spyOn(chrome.runtime.onMessage, "removeListener");
-
-      const logoutPromise = accountSwitcherService.logoutAccount(userId1);
-
-      listener(
-        { command: "switchAccountFinish", userId: userId2, status: AuthenticationStatus.Unlocked },
-        undefined,
-        undefined,
-      );
-
-      const result = await logoutPromise;
-
-      expect(messagingService.send).toHaveBeenCalledWith("logout", { userId: userId1 });
-      expect(result).toEqual({ newUserId: userId2, status: AuthenticationStatus.Unlocked });
-      expect(removeListenerSpy).toBeCalledTimes(1);
-    });
-  });
 });

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

@@ -12,6 +12,7 @@ import {
   timeout,
 } from "rxjs";
 
+import { NewActiveUser } from "@bitwarden/auth/common";
 import { 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";
@@ -43,7 +44,7 @@ export class AccountSwitcherService {
   SPECIAL_ADD_ACCOUNT_ID = "addAccount";
   availableAccounts$: Observable<AvailableAccount[]>;
 
-  switchAccountFinished$: Observable<{ userId: UserId; status: AuthenticationStatus }>;
+  switchAccountFinished$: Observable<NewActiveUser | null>;
 
   constructor(
     private accountService: AccountService,
@@ -118,7 +119,7 @@ export class AccountSwitcherService {
       [message: { command: string; userId: UserId; status: AuthenticationStatus }]
     >(chrome.runtime.onMessage).pipe(
       filter(([message]) => message.command === "switchAccountFinish"),
-      map(([message]) => ({ userId: message.userId, status: message.status })),
+      map(([message]) => ({ userId: message.userId, authenticationStatus: message.status })),
     );
   }
 
@@ -143,29 +144,9 @@ export class AccountSwitcherService {
     return await switchAccountFinishedPromise;
   }
 
-  /**
-   *
-   * @param userId the user id to logout
-   * @returns the userId and status of the that has been switch to due to the logout. null on errors.
-   */
-  async logoutAccount(
-    userId: UserId,
-  ): Promise<{ newUserId: UserId; status: AuthenticationStatus } | null> {
-    // logout creates an account switch to the next up user, which may be null
-    const switchPromise = this.listenForSwitchAccountFinish(null);
-
-    await this.messagingService.send("logout", { userId });
-
-    // wait for account switch to happen, the result will be the new user id and status
-    const result = await switchPromise;
-    return { newUserId: result.userId, status: result.status };
-  }
-
   // Listens for the switchAccountFinish message and returns the userId from the message
   // Optionally filters switchAccountFinish to an expected userId
-  private listenForSwitchAccountFinish(
-    expectedUserId: UserId | null,
-  ): Promise<{ userId: UserId; status: AuthenticationStatus } | null> {
+  listenForSwitchAccountFinish(expectedUserId: UserId | null): Promise<NewActiveUser | null> {
     return firstValueFrom(
       this.switchAccountFinished$.pipe(
         filter(({ userId }) => (expectedUserId ? userId === expectedUserId : true)),

apps/browser/src/auth/popup/logout/extension-logout.service.spec.ts

@@ -0,0 +1,101 @@
+import { MockProxy, mock } from "jest-mock-extended";
+
+import { LogoutReason, LogoutService } from "@bitwarden/auth/common";
+import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
+import { UserId } from "@bitwarden/common/types/guid";
+
+import { AccountSwitcherService } from "../account-switching/services/account-switcher.service";
+
+import { ExtensionLogoutService } from "./extension-logout.service";
+
+describe("ExtensionLogoutService", () => {
+  let logoutService: LogoutService;
+  let messagingService: MockProxy<MessagingService>;
+  let accountSwitcherService: MockProxy<AccountSwitcherService>;
+
+  let primaryUserId: UserId;
+  let secondaryUserId: UserId;
+  let logoutReason: LogoutReason;
+
+  beforeEach(() => {
+    primaryUserId = "1" as UserId;
+    secondaryUserId = "2" as UserId;
+    logoutReason = "vaultTimeout";
+
+    messagingService = mock<MessagingService>();
+    accountSwitcherService = mock<AccountSwitcherService>();
+    logoutService = new ExtensionLogoutService(messagingService, accountSwitcherService);
+  });
+
+  it("instantiates", () => {
+    expect(logoutService).not.toBeFalsy();
+  });
+
+  describe("logout", () => {
+    describe("No new active user", () => {
+      beforeEach(() => {
+        accountSwitcherService.listenForSwitchAccountFinish.mockResolvedValue(null);
+      });
+
+      it("sends logout message without a logout reason when not provided", async () => {
+        const result = await logoutService.logout(primaryUserId);
+
+        expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1);
+        expect(messagingService.send).toHaveBeenCalledWith("logout", { userId: primaryUserId });
+
+        expect(result).toBeUndefined();
+      });
+
+      it("sends logout message with a logout reason when provided", async () => {
+        const result = await logoutService.logout(primaryUserId, logoutReason);
+
+        expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1);
+        expect(messagingService.send).toHaveBeenCalledWith("logout", {
+          userId: primaryUserId,
+          logoutReason,
+        });
+        expect(result).toBeUndefined();
+      });
+    });
+
+    describe("New active user", () => {
+      const newActiveUserAuthenticationStatus = AuthenticationStatus.Unlocked;
+
+      beforeEach(() => {
+        accountSwitcherService.listenForSwitchAccountFinish.mockResolvedValue({
+          userId: secondaryUserId,
+          authenticationStatus: newActiveUserAuthenticationStatus,
+        });
+      });
+
+      it("sends logout message without a logout reason when not provided and returns the new active  user", async () => {
+        const result = await logoutService.logout(primaryUserId);
+
+        expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1);
+
+        expect(messagingService.send).toHaveBeenCalledWith("logout", { userId: primaryUserId });
+
+        expect(result).toEqual({
+          userId: secondaryUserId,
+          authenticationStatus: newActiveUserAuthenticationStatus,
+        });
+      });
+
+      it("sends logout message with a logout reason when provided and returns the new active  user", async () => {
+        const result = await logoutService.logout(primaryUserId, logoutReason);
+
+        expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1);
+
+        expect(messagingService.send).toHaveBeenCalledWith("logout", {
+          userId: primaryUserId,
+          logoutReason,
+        });
+        expect(result).toEqual({
+          userId: secondaryUserId,
+          authenticationStatus: newActiveUserAuthenticationStatus,
+        });
+      });
+    });
+  });
+});