[PM-26049] Auto key not stored due to vault timeout write vs read race condition for cli (#17707)

* auto key not stored due to vault timeout race condition being null for cli

* fix unit test default state

* neglected electron key service test cleanup

* bad merge - fix formatting

apps/desktop/src/key-management/electron-key.service.spec.ts

@@ -13,11 +13,13 @@ import { UserKey } from "@bitwarden/common/types/key";
 import { BiometricStateService, KdfConfigService } from "@bitwarden/key-management";
 
 import {
-  makeSymmetricCryptoKey,
   FakeAccountService,
-  mockAccountServiceWith,
   FakeStateProvider,
+  makeSymmetricCryptoKey,
+  mockAccountServiceWith,
 } from "../../../../libs/common/spec";
+// eslint-disable-next-line no-restricted-imports
+import { VAULT_TIMEOUT } from "../../../../libs/common/src/key-management/vault-timeout";
 
 import { DesktopBiometricsService } from "./biometrics/desktop.biometrics.service";
 import { ElectronKeyService } from "./electron-key.service";
@@ -40,11 +42,13 @@ describe("ElectronKeyService", () => {
   let accountService: FakeAccountService;
   let masterPasswordService: FakeMasterPasswordService;
 
-  beforeEach(() => {
+  beforeEach(async () => {
     accountService = mockAccountServiceWith(mockUserId);
     masterPasswordService = new FakeMasterPasswordService();
     stateProvider = new FakeStateProvider(accountService);
 
+    await stateProvider.setUserState(VAULT_TIMEOUT, 10, mockUserId);
+
     keyService = new ElectronKeyService(
       masterPasswordService,
       keyGenerationService,
@@ -79,38 +83,17 @@ describe("ElectronKeyService", () => {
         expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(mockUserId);
       });
 
-      describe("biometric unlock enabled", () => {
-        beforeEach(() => {
-          biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(true);
-        });
+      it("sets biometric key when biometric unlock enabled", async () => {
+        biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(true);
 
-        it("sets null biometric client key half and biometric unlock key when require password on start disabled", async () => {
-          biometricStateService.getRequirePasswordOnStart.mockResolvedValue(false);
+        await keyService.setUserKey(userKey, mockUserId);
 
-          await keyService.setUserKey(userKey, mockUserId);
-
-          expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
-            mockUserId,
-            userKey,
-          );
-          expect(biometricStateService.setEncryptedClientKeyHalf).not.toHaveBeenCalled();
-          expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(mockUserId);
-        });
-
-        describe("require password on start enabled", () => {
-          beforeEach(() => {
-            biometricStateService.getRequirePasswordOnStart.mockResolvedValue(true);
-          });
-
-          it("sets biometric key", async () => {
-            await keyService.setUserKey(userKey, mockUserId);
-
-            expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
-              mockUserId,
-              userKey,
-            );
-          });
-        });
+        expect(biometricService.setBiometricProtectedUnlockKeyForUser).toHaveBeenCalledWith(
+          mockUserId,
+          userKey,
+        );
+        expect(biometricStateService.setEncryptedClientKeyHalf).not.toHaveBeenCalled();
+        expect(biometricStateService.getBiometricUnlockEnabled).toHaveBeenCalledWith(mockUserId);
       });
     });
   });

libs/common/src/key-management/vault-timeout/index.ts

@@ -10,3 +10,5 @@ export {
   VaultTimeoutNumberType,
   VaultTimeoutStringType,
 } from "./types/vault-timeout.type";
+// Only used by desktop's electron-key.service.spec.ts test
+export { VAULT_TIMEOUT } from "./services/vault-timeout-settings.state";

libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts

@@ -13,6 +13,7 @@ import {
   shareReplay,
   switchMap,
   tap,
+  concatMap,
 } from "rxjs";
 
 // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
@@ -150,7 +151,7 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA
         return from(
           this.determineVaultTimeout(currentVaultTimeout, maxSessionTimeoutPolicyData),
         ).pipe(
-          tap((vaultTimeout: VaultTimeout) => {
+          concatMap(async (vaultTimeout: VaultTimeout) => {
             this.logService.debug(
               "[VaultTimeoutSettingsService] Determined vault timeout is %o for user id %s",
               vaultTimeout,
@@ -159,8 +160,9 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA
 
             // As a side effect, set the new value determined by determineVaultTimeout into state if it's different from the current
             if (vaultTimeout !== currentVaultTimeout) {
-              return this.stateProvider.setUserState(VAULT_TIMEOUT, vaultTimeout, userId);
+              await this.stateProvider.setUserState(VAULT_TIMEOUT, vaultTimeout, userId);
             }
+            return vaultTimeout;
           }),
           catchError((error: unknown) => {
             // Protect outer observable from canceling on error by catching and returning EMPTY

libs/key-management/src/key.service.spec.ts

@@ -69,11 +69,13 @@ describe("keyService", () => {
   let accountService: FakeAccountService;
   let masterPasswordService: FakeMasterPasswordService;
 
-  beforeEach(() => {
+  beforeEach(async () => {
     accountService = mockAccountServiceWith(mockUserId);
     masterPasswordService = new FakeMasterPasswordService();
     stateProvider = new FakeStateProvider(accountService);
 
+    await stateProvider.setUserState(VAULT_TIMEOUT, VaultTimeoutStringType.Never, mockUserId);
+
     keyService = new DefaultKeyService(
       masterPasswordService,
       keyGenerationService,

libs/key-management/src/key.service.ts

@@ -691,7 +691,9 @@ export class DefaultKeyService implements KeyServiceAbstraction {
         // the VaultTimeoutSettingsSvc and this service.
         // This should be fixed as part of the PM-7082 - Auto Key Service work.
         const vaultTimeout = await firstValueFrom(
-          this.stateProvider.getUserState$(VAULT_TIMEOUT, userId),
+          this.stateProvider
+            .getUserState$(VAULT_TIMEOUT, userId)
+            .pipe(filter((timeout) => timeout != null)),
         );
 
         shouldStoreKey = vaultTimeout == VaultTimeoutStringType.Never;