diff --git 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 index 3fa71598e65..f737eb18425 100644 --- 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 @@ -307,6 +307,176 @@ describe("VaultTimeoutSettingsService", () => { }, ); }); + + describe("state persistence side effects", () => { + beforeEach(() => { + sessionTimeoutTypeService.getOrPromoteToAvailable.mockImplementation( + async (timeout) => timeout, + ); + policyService.policiesByType$.mockReturnValue(of([])); + }); + + it("resets incompatible stored action to null when only one action is available (e.g. lock stored, only LogOut available)", async () => { + // TDE user removes PIN — only LogOut available but "lock" was stored + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(false)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + + await stateProvider.setUserState(VAULT_TIMEOUT_ACTION, VaultTimeoutAction.Lock, mockUserId); + + await firstValueFrom( + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(mockUserId), + ); + + expect( + stateProvider.singleUser.getFake(mockUserId, VAULT_TIMEOUT_ACTION).nextMock, + ).toHaveBeenCalledWith(null); + }); + + it("migrates tokens to memory when stored Lock is reset because only LogOut is available", async () => { + const mockAccessToken = "mockAccessToken"; + const mockRefreshToken = "mockRefreshToken"; + const mockClientId = "mockClientId"; + const mockClientSecret = "mockClientSecret"; + + // TDE user removes PIN — only LogOut available, but "lock" was stored + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(false)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + + tokenService.getAccessToken.mockResolvedValue(mockAccessToken); + tokenService.getRefreshToken.mockResolvedValue(mockRefreshToken); + tokenService.getClientId.mockResolvedValue(mockClientId); + tokenService.getClientSecret.mockResolvedValue(mockClientSecret); + + await stateProvider.setUserState(VAULT_TIMEOUT_ACTION, VaultTimeoutAction.Lock, mockUserId); + await stateProvider.setUserState(VAULT_TIMEOUT, 15, mockUserId); + + await firstValueFrom( + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(mockUserId), + ); + + expect(tokenService.setTokens).toHaveBeenCalledWith( + mockAccessToken, + VaultTimeoutAction.LogOut, + 15, + mockRefreshToken, + [mockClientId, mockClientSecret], + ); + }); + + it("resets stored LogOut to null when only LogOut is available (master-password-less)", async () => { + // TDE user removed PIN — stored LogOut must be cleared so that re-enabling PIN later + // defaults back to Lock instead of staying on LogOut. + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(false)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + + await stateProvider.setUserState( + VAULT_TIMEOUT_ACTION, + VaultTimeoutAction.LogOut, + mockUserId, + ); + + await firstValueFrom( + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(mockUserId), + ); + + // State must be reset to null; no token migration since the effective action didn't change + expect( + stateProvider.singleUser.getFake(mockUserId, VAULT_TIMEOUT_ACTION).nextMock, + ).toHaveBeenCalledWith(null); + expect(tokenService.setTokens).not.toHaveBeenCalled(); + }); + + it("does not write when state is already null and only one action is available", async () => { + // TDE user with no unlock methods, state already null — no write needed + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(false)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + + await stateProvider.setUserState(VAULT_TIMEOUT_ACTION, null, mockUserId); + const stateFake = stateProvider.singleUser.getFake(mockUserId, VAULT_TIMEOUT_ACTION); + stateFake.nextMock.mockClear(); + + await firstValueFrom( + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(mockUserId), + ); + + expect(stateFake.nextMock).not.toHaveBeenCalled(); + }); + + it("stores policy-forced LogOut when multiple actions are available", async () => { + // User with PIN — both Lock and LogOut available; policy forces LogOut + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(true)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + policyService.policiesByType$.mockReturnValue( + of([ + { data: { action: VaultTimeoutAction.LogOut, type: "never" } }, + ] as unknown as Policy[]), + ); + + await stateProvider.setUserState(VAULT_TIMEOUT_ACTION, null, mockUserId); + + await firstValueFrom( + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(mockUserId), + ); + + expect( + stateProvider.singleUser.getFake(mockUserId, VAULT_TIMEOUT_ACTION).nextMock, + ).toHaveBeenCalledWith(VaultTimeoutAction.LogOut); + }); + + it("migrates tokens when effective action changes from null (forced LogOut reset) to Lock", async () => { + const mockAccessToken = "mockAccessToken"; + const mockRefreshToken = "mockRefreshToken"; + const mockClientId = "mockClientId"; + const mockClientSecret = "mockClientSecret"; + + // User adds PIN — Lock becomes available; stored action is null (reset by migrator or new code) + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(true)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + + tokenService.getAccessToken.mockResolvedValue(mockAccessToken); + tokenService.getRefreshToken.mockResolvedValue(mockRefreshToken); + tokenService.getClientId.mockResolvedValue(mockClientId); + tokenService.getClientSecret.mockResolvedValue(mockClientSecret); + + await stateProvider.setUserState(VAULT_TIMEOUT_ACTION, null, mockUserId); + await stateProvider.setUserState(VAULT_TIMEOUT, 15, mockUserId); + + await firstValueFrom( + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(mockUserId), + ); + + expect(tokenService.setTokens).toHaveBeenCalledWith( + mockAccessToken, + VaultTimeoutAction.Lock, + 15, + mockRefreshToken, + [mockClientId, mockClientSecret], + ); + }); + + it("does not call setTokens when accessToken is null during null to Lock transition", async () => { + // User adds PIN but accessToken is null (e.g. not yet authenticated) + userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); + pinStateService.pinSet$.mockReturnValue(of(true)); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(false)); + + tokenService.getAccessToken.mockResolvedValue(null); + + await stateProvider.setUserState(VAULT_TIMEOUT_ACTION, null, mockUserId); + + await firstValueFrom( + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(mockUserId), + ); + + expect(tokenService.setTokens).not.toHaveBeenCalled(); + }); + }); }); describe("getVaultTimeoutByUserId$", () => { @@ -803,6 +973,7 @@ describe("VaultTimeoutSettingsService", () => { // Arrange const action = VaultTimeoutAction.LogOut; const timeout = 30; + tokenService.getAccessToken.mockResolvedValue(mockAccessToken); // Act await vaultTimeoutSettingsService.setVaultTimeoutOptions(mockUserId, timeout, action); @@ -815,6 +986,7 @@ describe("VaultTimeoutSettingsService", () => { // Arrange const action = VaultTimeoutAction.LogOut; const timeout = VaultTimeoutStringType.Never; + tokenService.getAccessToken.mockResolvedValue(mockAccessToken); // Act await vaultTimeoutSettingsService.setVaultTimeoutOptions(mockUserId, timeout, action); diff --git 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 index 5384d6860b7..af14478b1f6 100644 --- 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 @@ -3,16 +3,16 @@ import { catchError, combineLatest, + concatMap, distinctUntilChanged, EMPTY, firstValueFrom, from, map, - of, Observable, + of, shareReplay, switchMap, - concatMap, } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. @@ -76,27 +76,11 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA throw new Error("Vault Timeout Action cannot be null."); } - // We swap these tokens from being on disk for lock actions, and in memory for logout actions - // Get them here to set them to their new location after changing the timeout action and clearing if needed - const accessToken = await this.tokenService.getAccessToken(userId); - const refreshToken = await this.tokenService.getRefreshToken(userId); - const clientId = await this.tokenService.getClientId(userId); - const clientSecret = await this.tokenService.getClientSecret(userId); - await this.setVaultTimeout(userId, timeout); - if (timeout != VaultTimeoutStringType.Never && action === VaultTimeoutAction.LogOut) { - // if we have a vault timeout and the action is log out, reset tokens - // as the tokens were stored on disk and now should be stored in memory - await this.tokenService.clearTokens(userId); - } - await this.setVaultTimeoutAction(userId, action); - await this.tokenService.setTokens(accessToken, action, timeout, refreshToken, [ - clientId, - clientSecret, - ]); + await this.migrateTokenStorage(userId, action, timeout); await this.keyService.refreshAdditionalKeys(userId); } @@ -259,6 +243,35 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return currentVaultTimeout; } + /** + * Re-stores tokens in the correct location (memory vs disk) for the given action and timeout. + */ + private async migrateTokenStorage( + userId: UserId, + action: VaultTimeoutAction, + timeout: VaultTimeout, + ): Promise { + // Read tokens before any clearing so they can be re-stored in the new location + const accessToken = await this.tokenService.getAccessToken(userId); + const refreshToken = await this.tokenService.getRefreshToken(userId); + const clientId = await this.tokenService.getClientId(userId); + const clientSecret = await this.tokenService.getClientSecret(userId); + + if (timeout != VaultTimeoutStringType.Never && action === VaultTimeoutAction.LogOut) { + // Switching to LogOut: clear tokens from disk before re-storing in memory + await this.tokenService.clearTokens(userId); + } + + if (!accessToken) { + return; + } + + await this.tokenService.setTokens(accessToken, action, timeout, refreshToken, [ + clientId, + clientSecret, + ]); + } + private async setVaultTimeoutAction(userId: UserId, action: VaultTimeoutAction): Promise { if (!userId) { throw new Error("User id required. Cannot set vault timeout action."); @@ -280,12 +293,14 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA this.stateProvider.getUserState$(VAULT_TIMEOUT_ACTION, userId), this.getMaxSessionTimeoutPolicyDataByUserId$(userId), this.availableVaultTimeoutActions$(userId), + this.getVaultTimeoutByUserId$(userId), ]).pipe( concatMap( async ([ currentVaultTimeoutAction, maxSessionTimeoutPolicyData, availableVaultTimeoutActions, + vaultTimeout, ]) => { const vaultTimeoutAction = this.determineVaultTimeoutAction( availableVaultTimeoutActions, @@ -293,11 +308,33 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA maxSessionTimeoutPolicyData, ); - // As a side effect, set the new value determined by determineVaultTimeout into state if it's different from the current - // We want to avoid having a null timeout action always so we set it to the default if it is null - // and if the user becomes subject to a policy that requires a specific action, we set it to that - if (vaultTimeoutAction !== currentVaultTimeoutAction) { + // As a side effect, persist the determined action back to state when needed. + const oneActionAvailable = availableVaultTimeoutActions.length === 1; + if (oneActionAvailable) { + const availableAction = availableVaultTimeoutActions[0]; + // Always reset to null when only one action is available — even if the stored action + // matches. Ensures the default (Lock) is used when an unlock method (e.g. PIN) is + // re-enabled later. + if (currentVaultTimeoutAction !== null) { + await this.stateProvider.setUserState(VAULT_TIMEOUT_ACTION, null, userId); + // Only migrate tokens if the actual action changes (e.g. Lock → LogOut). + // No migration needed when stored action already matches the only available one. + if (currentVaultTimeoutAction !== availableAction) { + await this.migrateTokenStorage(userId, availableAction, vaultTimeout); + } + } + } else if (vaultTimeoutAction !== currentVaultTimeoutAction) { await this.stateProvider.setUserState(VAULT_TIMEOUT_ACTION, vaultTimeoutAction, userId); + + // Migrate tokens when effective action changes from LogOut (memory) to Lock (disk). + // currentVaultTimeoutAction is null when forced LogOut was reset to null by the + // side effect (no unlock methods) or the state migrator. + if ( + currentVaultTimeoutAction === null && + vaultTimeoutAction === VaultTimeoutAction.Lock + ) { + await this.migrateTokenStorage(userId, vaultTimeoutAction, vaultTimeout); + } } return vaultTimeoutAction;