Only Keep Active User Alive When A View is Open (#7045)

bitwarden · Dec 5, 2023 · 3deb6ea · 3deb6ea
1 parent 2fd3030
commit 3deb6ea
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 35 deletions.
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
@@ -91,7 +91,10 @@ describe("VaultTimeoutService", () => {
         availableTimeoutActions?: VaultTimeoutAction[];
       }
     >,
-    userId?: string,
+    globalSetups?: {
+      userId?: string;
+      isViewOpen?: boolean;
+    },
   ) => {
     // Both are available by default and the specific test can change this per test
     availableVaultTimeoutActionsSubject.next([VaultTimeoutAction.Lock, VaultTimeoutAction.LogOut]);
@@ -111,7 +114,11 @@ describe("VaultTimeoutService", () => {
       return Promise.resolve(accounts[options.userId]?.lastActive);
     });
 
-    stateService.getUserId.mockResolvedValue(userId);
+    stateService.getUserId.mockResolvedValue(globalSetups?.userId);
+
+    stateService.activeAccount$ = new BehaviorSubject<string>(globalSetups?.userId);
+
+    platformUtilsService.isViewOpen.mockResolvedValue(globalSetups?.isViewOpen ?? false);
 
     vaultTimeoutSettingsService.vaultTimeoutAction$.mockImplementation((userId) => {
       return new BehaviorSubject<VaultTimeoutAction>(accounts[userId]?.timeoutAction);
@@ -217,22 +224,25 @@ describe("VaultTimeoutService", () => {
 
     it("should lock an account that isn't active and has immediate as their timeout when view is not open", async () => {
       // Arrange
-      platformUtilsService.isViewOpen.mockResolvedValue(false);
-
-      setupAccounts({
-        1: {
-          authStatus: AuthenticationStatus.Unlocked,
-          isAuthenticated: true,
-          vaultTimeout: 0, // Immediately
-          lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago
+      setupAccounts(
+        {
+          1: {
+            authStatus: AuthenticationStatus.Unlocked,
+            isAuthenticated: true,
+            vaultTimeout: 0, // Immediately
+            lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago
+          },
+          2: {
+            authStatus: AuthenticationStatus.Unlocked,
+            isAuthenticated: true,
+            vaultTimeout: 1, // One minute
+            lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago
+          },
         },
-        2: {
-          authStatus: AuthenticationStatus.Unlocked,
-          isAuthenticated: true,
-          vaultTimeout: 1, // One minute
-          lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago
+        {
+          isViewOpen: false,
         },
-      });
+      );
 
       // Act
       await vaultTimeoutService.checkVaultTimeout();
@@ -243,8 +253,6 @@ describe("VaultTimeoutService", () => {
     });
 
     it("should run action on an account that hasn't been active for greater than 1 minute and has a vault timeout for 1 minutes", async () => {
-      platformUtilsService.isViewOpen.mockResolvedValue(false);
-
       setupAccounts(
         {
           1: {
@@ -276,7 +284,7 @@ describe("VaultTimeoutService", () => {
             availableTimeoutActions: [VaultTimeoutAction.LogOut],
           },
         },
-        "2", // Treat user 2 as the active user
+        { userId: "2", isViewOpen: false }, // Treat user 2 as the active user
       );
 
       await vaultTimeoutService.checkVaultTimeout();
@@ -292,18 +300,37 @@ describe("VaultTimeoutService", () => {
       expectUserToHaveLoggedOut("4"); // They may have had lock as their chosen action but it's not available to them so logout
     });
 
-    it("should not lock any accounts as long as a view is known to be open, no matter if they haven't been active since before their timeout", async () => {
-      platformUtilsService.isViewOpen.mockResolvedValue(true);
+    it("should lock an account if they haven't been active passed their vault timeout even if a view is open when they are not the active user.", async () => {
+      setupAccounts(
+        {
+          1: {
+            // Neither of these setup values ever get called
+            authStatus: AuthenticationStatus.Unlocked,
+            isAuthenticated: true,
+            lastActive: new Date().getTime() - 80 * 1000, // Last active 80 seconds ago
+            vaultTimeout: 1, // Vault timeout of 1 minute
+          },
+        },
+        { userId: "2", isViewOpen: true },
+      );
+
+      await vaultTimeoutService.checkVaultTimeout();
+
+      expectUserToHaveLocked("1");
+    });
 
-      setupAccounts({
-        1: {
-          // Neither of these setup values ever get called
-          authStatus: AuthenticationStatus.Unlocked,
-          isAuthenticated: true,
-          lastActive: new Date().getTime() - 80 * 1000, // Last active 80 seconds ago
-          vaultTimeout: 1, // Vault timeout of 1 minute ago
+    it("should not lock an account that is active and we know that a view is open, even if they haven't been active passed their timeout", async () => {
+      setupAccounts(
+        {
+          1: {
+            authStatus: AuthenticationStatus.Unlocked,
+            isAuthenticated: true,
+            lastActive: new Date().getTime() - 80 * 1000, // Last active 80 seconds ago
+            vaultTimeout: 1, // Vault timeout of 1 minute
+          },
         },
-      });
+        { userId: "1", isViewOpen: true }, // They are the currently active user
+      );
 
       await vaultTimeoutService.checkVaultTimeout();
 

diff --git 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 } from "rxjs";
+import { firstValueFrom, timeout } from "rxjs";
 
 import { SearchService } from "../../abstractions/search.service";
 import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service";
@@ -52,13 +52,14 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
   }
 
   async checkVaultTimeout(): Promise<void> {
-    if (await this.platformUtilsService.isViewOpen()) {
-      return;
-    }
+    // 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))) {
+      if (userId != null && (await this.shouldLock(userId, activeUserId, isViewOpen))) {
         await this.executeTimeoutAction(userId);
       }
     }
@@ -108,7 +109,18 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
     }
   }
 
-  private async shouldLock(userId: string): Promise<boolean> {
+  private async shouldLock(
+    userId: string,
+    activeUserId: string,
+    isViewOpen: boolean,
+  ): Promise<boolean> {
+    if (isViewOpen && userId === activeUserId) {
+      // We know a view is open and this is the currently active user
+      // which means they are likely looking at their vault
+      // and they should not lock.
+      return false;
+    }
+
     const authStatus = await this.authService.getAuthStatus(userId);
     if (
       authStatus === AuthenticationStatus.Locked ||