[PM-26049] Always store users auto unlock key on Cli (#18477)

* Always store auto user key on CLI * update unit tests * prevent bad vault timeout state * Update libs/key-management/src/key.service.ts Co-authored-by: Bernd Schoolmann <mail@quexten.com> --------- Co-authored-by: Bernd Schoolmann <mail@quexten.com>
2026-01-28 15:23:53 +00:00 · 2026-01-23 20:30:31 +01:00
parent 3a5b31f7df
commit a59760f83b
4 changed files with 197 additions and 32 deletions
--- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts
+++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts
@@ -260,6 +260,13 @@ describe("VaultTimeoutSettingsService", () => {
  });

  describe("getVaultTimeoutByUserId$", () => {
+    beforeEach(() => {
+      // Return the input value unchanged
+      sessionTimeoutTypeService.getOrPromoteToAvailable.mockImplementation(
+        async (timeout) => timeout,
+      );
+    });
+
    it("should throw an error if no user id is provided", async () => {
      expect(() => vaultTimeoutSettingsService.getVaultTimeoutByUserId$(null)).toThrow(
        "User id required. Cannot get vault timeout.",
@@ -277,6 +284,9 @@ describe("VaultTimeoutSettingsService", () => {
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          defaultVaultTimeout,
+        );
        expect(result).toBe(defaultVaultTimeout);
      });

@@ -299,8 +309,31 @@ describe("VaultTimeoutSettingsService", () => {
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          vaultTimeout,
+        );
        expect(result).toBe(vaultTimeout);
      });
+
+      it("promotes timeout when unavailable on client", async () => {
+        const determinedTimeout = VaultTimeoutNumberType.OnMinute;
+        const promotedValue = VaultTimeoutStringType.OnRestart;
+
+        sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(promotedValue);
+        userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: true }));
+        policyService.policiesByType$.mockReturnValue(of([]));
+
+        await stateProvider.setUserState(VAULT_TIMEOUT, determinedTimeout, mockUserId);
+
+        const result = await firstValueFrom(
+          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
+        );
+
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          determinedTimeout,
+        );
+        expect(result).toBe(promotedValue);
+      });
    });

    describe("policy type: custom", () => {
@@ -327,6 +360,9 @@ describe("VaultTimeoutSettingsService", () => {
            vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
          );

+          expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+            policyMinutes,
+          );
          expect(result).toBe(policyMinutes);
        },
      );
@@ -345,6 +381,9 @@ describe("VaultTimeoutSettingsService", () => {
            vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
          );

+          expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+            vaultTimeout,
+          );
          expect(result).toBe(vaultTimeout);
        },
      );
@@ -365,8 +404,36 @@ describe("VaultTimeoutSettingsService", () => {
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          VaultTimeoutNumberType.Immediately,
+        );
        expect(result).toBe(VaultTimeoutNumberType.Immediately);
      });
+
+      it("promotes policy minutes when unavailable on client", async () => {
+        const promotedValue = VaultTimeoutStringType.Never;
+
+        sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(promotedValue);
+        userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: true }));
+        policyService.policiesByType$.mockReturnValue(
+          of([{ data: { type: "custom", minutes: policyMinutes } }] as unknown as Policy[]),
+        );
+
+        await stateProvider.setUserState(
+          VAULT_TIMEOUT,
+          VaultTimeoutNumberType.EightHours,
+          mockUserId,
+        );
+
+        const result = await firstValueFrom(
+          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
+        );
+
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          policyMinutes,
+        );
+        expect(result).toBe(promotedValue);
+      });
    });

    describe("policy type: immediately", () => {
@@ -383,7 +450,6 @@ describe("VaultTimeoutSettingsService", () => {
        "when current timeout is %s, returns immediately or promoted value",
        async (currentTimeout) => {
          const expectedTimeout = VaultTimeoutNumberType.Immediately;
-          sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(expectedTimeout);
          policyService.policiesByType$.mockReturnValue(
            of([{ data: { type: "immediately" } }] as unknown as Policy[]),
          );
@@ -400,6 +466,26 @@ describe("VaultTimeoutSettingsService", () => {
          expect(result).toBe(expectedTimeout);
        },
      );
+
+      it("promotes immediately when unavailable on client", async () => {
+        const promotedValue = VaultTimeoutNumberType.OnMinute;
+
+        sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(promotedValue);
+        policyService.policiesByType$.mockReturnValue(
+          of([{ data: { type: "immediately" } }] as unknown as Policy[]),
+        );
+
+        await stateProvider.setUserState(VAULT_TIMEOUT, VaultTimeoutStringType.Never, mockUserId);
+
+        const result = await firstValueFrom(
+          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
+        );
+
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          VaultTimeoutNumberType.Immediately,
+        );
+        expect(result).toBe(promotedValue);
+      });
    });

    describe("policy type: onSystemLock", () => {
@@ -413,7 +499,6 @@ describe("VaultTimeoutSettingsService", () => {
        "when current timeout is %s, returns onLocked or promoted value",
        async (currentTimeout) => {
          const expectedTimeout = VaultTimeoutStringType.OnLocked;
-          sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(expectedTimeout);
          policyService.policiesByType$.mockReturnValue(
            of([{ data: { type: "onSystemLock" } }] as unknown as Policy[]),
          );
@@ -446,9 +531,31 @@ describe("VaultTimeoutSettingsService", () => {
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

-        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).not.toHaveBeenCalled();
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          currentTimeout,
+        );
        expect(result).toBe(currentTimeout);
      });
+
+      it("promotes onLocked when unavailable on client", async () => {
+        const promotedValue = VaultTimeoutStringType.OnRestart;
+
+        sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(promotedValue);
+        policyService.policiesByType$.mockReturnValue(
+          of([{ data: { type: "onSystemLock" } }] as unknown as Policy[]),
+        );
+
+        await stateProvider.setUserState(VAULT_TIMEOUT, VaultTimeoutStringType.Never, mockUserId);
+
+        const result = await firstValueFrom(
+          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
+        );
+
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          VaultTimeoutStringType.OnLocked,
+        );
+        expect(result).toBe(promotedValue);
+      });
    });

    describe("policy type: onAppRestart", () => {
@@ -468,7 +575,9 @@ describe("VaultTimeoutSettingsService", () => {
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

-        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).not.toHaveBeenCalled();
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          VaultTimeoutStringType.OnRestart,
+        );
        expect(result).toBe(VaultTimeoutStringType.OnRestart);
      });

@@ -488,32 +597,40 @@ describe("VaultTimeoutSettingsService", () => {
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

-        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).not.toHaveBeenCalled();
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          currentTimeout,
+        );
        expect(result).toBe(currentTimeout);
      });
-    });

-    describe("policy type: never", () => {
-      it("when current timeout is never, returns never or promoted value", async () => {
-        const expectedTimeout = VaultTimeoutStringType.Never;
-        sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(expectedTimeout);
+      it("promotes onRestart when unavailable on client", async () => {
+        const promotedValue = VaultTimeoutStringType.Never;
+
+        sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(promotedValue);
        policyService.policiesByType$.mockReturnValue(
-          of([{ data: { type: "never" } }] as unknown as Policy[]),
+          of([{ data: { type: "onAppRestart" } }] as unknown as Policy[]),
        );

-        await stateProvider.setUserState(VAULT_TIMEOUT, VaultTimeoutStringType.Never, mockUserId);
+        await stateProvider.setUserState(
+          VAULT_TIMEOUT,
+          VaultTimeoutStringType.OnLocked,
+          mockUserId,
+        );

        const result = await firstValueFrom(
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
-          VaultTimeoutStringType.Never,
+          VaultTimeoutStringType.OnRestart,
        );
-        expect(result).toBe(expectedTimeout);
+        expect(result).toBe(promotedValue);
      });
+    });

+    describe("policy type: never", () => {
      it.each([
+        VaultTimeoutStringType.Never,
        VaultTimeoutStringType.OnRestart,
        VaultTimeoutStringType.OnLocked,
        VaultTimeoutStringType.OnIdle,
@@ -532,9 +649,32 @@ describe("VaultTimeoutSettingsService", () => {
          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
        );

-        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).not.toHaveBeenCalled();
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          currentTimeout,
+        );
        expect(result).toBe(currentTimeout);
      });
+
+      it("promotes timeout when unavailable on client", async () => {
+        const determinedTimeout = VaultTimeoutStringType.Never;
+        const promotedValue = VaultTimeoutStringType.OnRestart;
+
+        sessionTimeoutTypeService.getOrPromoteToAvailable.mockResolvedValue(promotedValue);
+        policyService.policiesByType$.mockReturnValue(
+          of([{ data: { type: "never" } }] as unknown as Policy[]),
+        );
+
+        await stateProvider.setUserState(VAULT_TIMEOUT, determinedTimeout, mockUserId);
+
+        const result = await firstValueFrom(
+          vaultTimeoutSettingsService.getVaultTimeoutByUserId$(mockUserId),
+        );
+
+        expect(sessionTimeoutTypeService.getOrPromoteToAvailable).toHaveBeenCalledWith(
+          determinedTimeout,
+        );
+        expect(result).toBe(promotedValue);
+      });
    });
  });

--- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts
+++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts
@@ -179,7 +179,20 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA
  private async determineVaultTimeout(
    currentVaultTimeout: VaultTimeout | null,
    maxSessionTimeoutPolicyData: MaximumSessionTimeoutPolicyData | null,
-  ): Promise<VaultTimeout | null> {
+  ): Promise<VaultTimeout> {
+    const determinedTimeout = await this.determineVaultTimeoutInternal(
+      currentVaultTimeout,
+      maxSessionTimeoutPolicyData,
+    );
+
+    // Ensures the timeout is available on this client
+    return await this.sessionTimeoutTypeService.getOrPromoteToAvailable(determinedTimeout);
+  }
+
+  private async determineVaultTimeoutInternal(
+    currentVaultTimeout: VaultTimeout | null,
+    maxSessionTimeoutPolicyData: MaximumSessionTimeoutPolicyData | null,
+  ): Promise<VaultTimeout> {
    // if current vault timeout is null, apply the client specific default
    currentVaultTimeout = currentVaultTimeout ?? this.defaultVaultTimeout;

@@ -190,9 +203,7 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA

    switch (maxSessionTimeoutPolicyData.type) {
      case "immediately":
-        return await this.sessionTimeoutTypeService.getOrPromoteToAvailable(
-          VaultTimeoutNumberType.Immediately,
-        );
+        return VaultTimeoutNumberType.Immediately;
      case "custom":
      case null:
      case undefined:
@@ -211,9 +222,7 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA
          currentVaultTimeout === VaultTimeoutStringType.OnIdle ||
          currentVaultTimeout === VaultTimeoutStringType.OnSleep
        ) {
-          return await this.sessionTimeoutTypeService.getOrPromoteToAvailable(
-            VaultTimeoutStringType.OnLocked,
-          );
+          return VaultTimeoutStringType.OnLocked;
        }
        break;
      case "onAppRestart":
@@ -227,11 +236,7 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA
        }
        break;
      case "never":
-        if (currentVaultTimeout === VaultTimeoutStringType.Never) {
-          return await this.sessionTimeoutTypeService.getOrPromoteToAvailable(
-            VaultTimeoutStringType.Never,
-          );
-        }
+        // Policy doesn't override user preference for "never"
        break;
    }
    return currentVaultTimeout;
--- a/libs/key-management/src/key.service.spec.ts
+++ b/libs/key-management/src/key.service.spec.ts
@@ -1,6 +1,7 @@
 import { mock } from "jest-mock-extended";
 import { BehaviorSubject, bufferCount, firstValueFrom, lastValueFrom, of, take } from "rxjs";

+import { ClientType } from "@bitwarden/client-type";
 import { EncryptedOrganizationKeyData } from "@bitwarden/common/admin-console/models/data/encrypted-organization-key.data";
 import { KeyGenerationService } from "@bitwarden/common/key-management/crypto";
 import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service";
@@ -259,7 +260,18 @@ describe("keyService", () => {
        });
      });

-      it("clears the Auto key if vault timeout is set to anything other than null", async () => {
+      it("sets an Auto key if vault timeout is set to 10 minutes and is Cli", async () => {
+        await stateProvider.setUserState(VAULT_TIMEOUT, 10, mockUserId);
+        platformUtilService.getClientType.mockReturnValue(ClientType.Cli);
+
+        await keyService.setUserKey(mockUserKey, mockUserId);
+
+        expect(stateService.setUserKeyAutoUnlock).toHaveBeenCalledWith(mockUserKey.keyB64, {
+          userId: mockUserId,
+        });
+      });
+
+      it("clears the Auto key if vault timeout is set to 10 minutes", async () => {
        await stateProvider.setUserState(VAULT_TIMEOUT, 10, mockUserId);

        await keyService.setUserKey(mockUserKey, mockUserId);
--- a/libs/key-management/src/key.service.ts
+++ b/libs/key-management/src/key.service.ts
@@ -14,6 +14,7 @@ import {
  switchMap,
 } from "rxjs";

+import { ClientType } from "@bitwarden/client-type";
 import { EncryptedOrganizationKeyData } from "@bitwarden/common/admin-console/models/data/encrypted-organization-key.data";
 import { BaseEncryptedOrganizationKey } from "@bitwarden/common/admin-console/models/domain/encrypted-organization-key";
 import { ProfileOrganizationResponse } from "@bitwarden/common/admin-console/models/response/profile-organization.response";
@@ -671,9 +672,13 @@ export class DefaultKeyService implements KeyServiceAbstraction {
  }

  protected async shouldStoreKey(keySuffix: KeySuffixOptions, userId: UserId) {
-    let shouldStoreKey = false;
    switch (keySuffix) {
      case KeySuffixOptions.Auto: {
+        // Cli has fixed Never vault timeout, and it should not be affected by a policy.
+        if (this.platformUtilService.getClientType() == ClientType.Cli) {
+          return true;
+        }
+
        // TODO: Sharing the UserKeyDefinition is temporary to get around a circ dep issue between
        // the VaultTimeoutSettingsSvc and this service.
        // This should be fixed as part of the PM-7082 - Auto Key Service work.
@@ -683,11 +688,14 @@ export class DefaultKeyService implements KeyServiceAbstraction {
            .pipe(filter((timeout) => timeout != null)),
        );

-        shouldStoreKey = vaultTimeout == VaultTimeoutStringType.Never;
-        break;
+        this.logService.debug(
+          `[KeyService] Should store auto key for vault timeout ${vaultTimeout}`,
+        );
+
+        return vaultTimeout == VaultTimeoutStringType.Never;
      }
    }
-    return shouldStoreKey;
+    return false;
  }

  protected async getKeyFromStorage(