From 6d22ee9550b2b6ea5807f87ca42584f8ff25c50f Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 30 Jul 2024 17:43:54 -0500 Subject: [PATCH] PM-10379: Update the timeout action logic to occur immediately after requirements are met (#3652) --- .../auth/datasource/disk/AuthDiskSource.kt | 19 -- .../datasource/disk/AuthDiskSourceImpl.kt | 15 - .../data/auth/repository/AuthRepository.kt | 7 +- .../auth/repository/AuthRepositoryImpl.kt | 12 +- .../vault/manager/VaultLockManagerImpl.kt | 174 ++++++---- .../platform/feature/rootnav/RootNavScreen.kt | 11 - .../feature/rootnav/RootNavViewModel.kt | 12 +- .../VaultUnlockedNavBarScreen.kt | 11 - .../VaultUnlockedNavBarViewModel.kt | 10 - .../datasource/disk/AuthDiskSourceTest.kt | 62 ---- .../disk/util/FakeAuthDiskSource.kt | 19 -- .../auth/repository/AuthRepositoryTest.kt | 18 - .../vault/manager/VaultLockManagerTest.kt | 317 +++++++----------- .../feature/rootnav/RootNavViewModelTest.kt | 11 - .../VaultUnlockedNavBarViewModelTest.kt | 8 - 15 files changed, 239 insertions(+), 467 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt index d115a58d1..18e374c00 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt @@ -60,25 +60,6 @@ interface AuthDiskSource { */ fun storeShouldTrustDevice(userId: String, shouldTrustDevice: Boolean?) - /** - * Retrieves the "last active time" for the given [userId], in milliseconds. - * - * This time is intended to be derived from a call to - * [SystemClock.elapsedRealtime()](https://developer.android.com/reference/android/os/SystemClock#elapsedRealtime()) - */ - fun getLastActiveTimeMillis(userId: String): Long? - - /** - * Stores the [lastActiveTimeMillis] for the given [userId]. - * - * This time is intended to be derived from a call to - * [SystemClock.elapsedRealtime()](https://developer.android.com/reference/android/os/SystemClock#elapsedRealtime()) - */ - fun storeLastActiveTimeMillis( - userId: String, - lastActiveTimeMillis: Long?, - ) - /** * Retrieves the number of consecutive invalid lock attempts for the given [userId]. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt index fb31bf546..8c03054da 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt @@ -28,7 +28,6 @@ private const val UNIQUE_APP_ID_KEY = "appId" private const val REMEMBERED_EMAIL_ADDRESS_KEY = "rememberedEmail" private const val REMEMBERED_ORG_IDENTIFIER_KEY = "rememberedOrgIdentifier" private const val STATE_KEY = "state" -private const val LAST_ACTIVE_TIME_KEY = "lastActiveTime" private const val INVALID_UNLOCK_ATTEMPTS_KEY = "invalidUnlockAttempts" private const val MASTER_KEY_ENCRYPTION_USER_KEY = "masterKeyEncryptedUserKey" private const val MASTER_KEY_ENCRYPTION_PRIVATE_KEY = "encPrivateKey" @@ -111,7 +110,6 @@ class AuthDiskSourceImpl( .onSubscription { emit(userState) } override fun clearData(userId: String) { - storeLastActiveTimeMillis(userId = userId, lastActiveTimeMillis = null) storeInvalidUnlockAttempts(userId = userId, invalidUnlockAttempts = null) storeUserKey(userId = userId, userKey = null) storeUserAutoUnlockKey(userId = userId, userAutoUnlockKey = null) @@ -138,19 +136,6 @@ class AuthDiskSourceImpl( putBoolean(SHOULD_TRUST_DEVICE_KEY.appendIdentifier(userId), shouldTrustDevice) } - override fun getLastActiveTimeMillis(userId: String): Long? = - getLong(key = LAST_ACTIVE_TIME_KEY.appendIdentifier(userId)) - - override fun storeLastActiveTimeMillis( - userId: String, - lastActiveTimeMillis: Long?, - ) { - putLong( - key = LAST_ACTIVE_TIME_KEY.appendIdentifier(userId), - value = lastActiveTimeMillis, - ) - } - override fun getInvalidUnlockAttempts(userId: String): Int? = getInt(key = INVALID_UNLOCK_ATTEMPTS_KEY.appendIdentifier(userId)) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt index 5773a6273..2fe15b09c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt @@ -22,8 +22,8 @@ import com.x8bit.bitwarden.data.auth.repository.model.ResetPasswordResult import com.x8bit.bitwarden.data.auth.repository.model.SetPasswordResult import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult import com.x8bit.bitwarden.data.auth.repository.model.UserState -import com.x8bit.bitwarden.data.auth.repository.model.ValidatePinResult import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult +import com.x8bit.bitwarden.data.auth.repository.model.ValidatePinResult import com.x8bit.bitwarden.data.auth.repository.model.VerifyOtpResult import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult import com.x8bit.bitwarden.data.auth.repository.util.DuoCallbackTokenResult @@ -239,11 +239,6 @@ interface AuthRepository : AuthenticatorProvider, AuthRequestManager { */ fun switchAccount(userId: String): SwitchAccountResult - /** - * Updates the "last active time" for the current user. - */ - fun updateLastActiveTime() - /** * Attempt to register a new account with the given parameters. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 5e5b01e9f..ce8b93d2a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -1,6 +1,5 @@ package com.x8bit.bitwarden.data.auth.repository -import android.os.SystemClock import com.bitwarden.core.AuthRequestMethod import com.bitwarden.core.InitUserCryptoMethod import com.bitwarden.core.InitUserCryptoRequest @@ -55,8 +54,8 @@ import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult import com.x8bit.bitwarden.data.auth.repository.model.UserAccountTokens import com.x8bit.bitwarden.data.auth.repository.model.UserOrganizations import com.x8bit.bitwarden.data.auth.repository.model.UserState -import com.x8bit.bitwarden.data.auth.repository.model.ValidatePinResult import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult +import com.x8bit.bitwarden.data.auth.repository.model.ValidatePinResult import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType import com.x8bit.bitwarden.data.auth.repository.model.VerifyOtpResult import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult @@ -140,7 +139,6 @@ class AuthRepositoryImpl( private val policyManager: PolicyManager, pushManager: PushManager, dispatcherManager: DispatcherManager, - private val elapsedRealtimeMillisProvider: () -> Long = { SystemClock.elapsedRealtime() }, ) : AuthRepository, AuthRequestManager by authRequestManager { /** @@ -707,14 +705,6 @@ class AuthRepositoryImpl( return SwitchAccountResult.AccountSwitched } - override fun updateLastActiveTime() { - val userId = activeUserId ?: return - authDiskSource.storeLastActiveTimeMillis( - userId = userId, - lastActiveTimeMillis = elapsedRealtimeMillisProvider(), - ) - } - @Suppress("LongMethod") override suspend fun register( email: String, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt index 82122d099..9aa7da196 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt @@ -1,6 +1,5 @@ package com.x8bit.bitwarden.data.vault.manager -import android.os.SystemClock import com.bitwarden.core.InitOrgCryptoRequest import com.bitwarden.core.InitUserCryptoMethod import com.bitwarden.core.InitUserCryptoRequest @@ -32,6 +31,8 @@ import com.x8bit.bitwarden.data.vault.repository.util.toVaultUnlockResult import com.x8bit.bitwarden.data.vault.repository.util.update import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -48,9 +49,7 @@ import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch - -private const val SECONDS_PER_MINUTE = 60 -private const val MILLISECONDS_PER_SECOND = 1000 +import kotlin.time.Duration.Companion.minutes /** * The number of times a user may fail to unlock before they are automatically logged out. @@ -70,12 +69,15 @@ class VaultLockManagerImpl( private val userLogoutManager: UserLogoutManager, private val trustedDeviceManager: TrustedDeviceManager, dispatcherManager: DispatcherManager, - private val elapsedRealtimeMillisProvider: () -> Long = { SystemClock.elapsedRealtime() }, ) : VaultLockManager { private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) + /** + * This [Map] tracks all active timeout [Job]s that are running using the user ID as the key. + */ + private val userIdTimerJobMap = mutableMapOf() + private val activeUserId: String? get() = authDiskSource.userState?.activeUserId - private val userIds: Set get() = authDiskSource.userState?.accounts?.keys.orEmpty() private val mutableVaultUnlockDataStateFlow = MutableStateFlow>(emptyList()) @@ -307,24 +309,11 @@ class VaultLockManagerImpl( .onEach { appForegroundState -> when (appForegroundState) { AppForegroundState.BACKGROUNDED -> { - activeUserId?.let { updateLastActiveTime(userId = it) } + handleOnBackground() } AppForegroundState.FOREGROUNDED -> { - userIds.forEach { userId -> - // If first foreground, clear the elapsed values so the timeout action - // is always performed. - if (isFirstForeground) { - authDiskSource.storeLastActiveTimeMillis( - userId = userId, - lastActiveTimeMillis = null, - ) - } - checkForVaultTimeout( - userId = userId, - isAppRestart = isFirstForeground, - ) - } + handleOnForeground(isFirstForeground = isFirstForeground) isFirstForeground = false } } @@ -332,6 +321,25 @@ class VaultLockManagerImpl( .launchIn(unconfinedScope) } + private fun handleOnBackground() { + val userId = activeUserId ?: return + checkForVaultTimeout( + userId = userId, + checkTimeoutReason = CheckTimeoutReason.APP_BACKGROUNDED, + ) + } + + private fun handleOnForeground(isFirstForeground: Boolean) { + val userId = activeUserId ?: return + userIdTimerJobMap[userId]?.cancel() + if (isFirstForeground) { + checkForVaultTimeout( + userId = userId, + checkTimeoutReason = CheckTimeoutReason.APP_RESTARTED, + ) + } + } + private fun observeUserSwitchingChanges() { authDiskSource .userSwitchingChangesFlow @@ -415,17 +423,13 @@ class VaultLockManagerImpl( previousActiveUserId: String, currentActiveUserId: String, ) { + // Make sure to clear the now-active user's timeout job. + userIdTimerJobMap[currentActiveUserId]?.cancel() // Check if the user's timeout action should be performed as we switch away. - checkForVaultTimeout(userId = previousActiveUserId) - - // Set the last active time for the previous user. - updateLastActiveTime(userId = previousActiveUserId) - - // Check if the vault timeout action should be performed for the current user - checkForVaultTimeout(userId = currentActiveUserId) - - // Set the last active time for the current user. - updateLastActiveTime(userId = currentActiveUserId) + checkForVaultTimeout( + userId = previousActiveUserId, + checkTimeoutReason = CheckTimeoutReason.USER_CHANGED, + ) } /** @@ -434,27 +438,21 @@ class VaultLockManagerImpl( */ private fun checkForVaultTimeout( userId: String, - isAppRestart: Boolean = false, + checkTimeoutReason: CheckTimeoutReason, ) { val accounts = authDiskSource.userAccountTokens - /** - * Check if the user is already logged out. If this is the case no need to check timeout. - * This is required in the case that an account has been "soft logged out" and has an - * immediate time interval time out. Without this check it would be automatically switch - * the active user back to an authenticated user if one exists. - */ - if ((accounts.find { it.userId == userId }?.isLoggedIn) == false) { - return - } + // Check if the user is already logged out. If this is the case no need to check timeout. + // This is required in the case that an account has been "soft logged out" and has an + // immediate time interval timeout. Without this check it would be automatically switch + // the active user back to an authenticated user if one exists. + if ((accounts.find { it.userId == userId }?.isLoggedIn) == false) return - val currentTimeMillis = elapsedRealtimeMillisProvider() - val lastActiveTimeMillis = authDiskSource.getLastActiveTimeMillis(userId = userId) ?: 0 val vaultTimeout = settingsRepository.getVaultTimeoutStateFlow(userId = userId).value val vaultTimeoutAction = settingsRepository .getVaultTimeoutActionStateFlow(userId = userId) .value - val vaultTimeoutInMinutes = when (vaultTimeout) { + when (vaultTimeout) { VaultTimeout.Never -> { // No action to take for Never timeout. return @@ -462,38 +460,73 @@ class VaultLockManagerImpl( VaultTimeout.OnAppRestart -> { // If this is an app restart, trigger the timeout action; otherwise ignore. - if (isAppRestart) 0 else return + if (checkTimeoutReason == CheckTimeoutReason.APP_RESTARTED) { + // On restart the vault should be locked already but we may need to soft-logout + // the user. + handleTimeoutAction(userId = userId, vaultTimeoutAction = vaultTimeoutAction) + } } - else -> vaultTimeout.vaultTimeoutInMinutes ?: return - } - val vaultTimeoutInMillis = vaultTimeoutInMinutes * - SECONDS_PER_MINUTE * - MILLISECONDS_PER_SECOND - if (currentTimeMillis - lastActiveTimeMillis >= vaultTimeoutInMillis) { - // Perform lock / logout! - when (vaultTimeoutAction) { - VaultTimeoutAction.LOCK -> { - setVaultToLocked(userId = userId) - } + else -> { + // Only perform action for users losing "fully active" status in some way. + when (checkTimeoutReason) { + // Don't perform delayed actions when first starting the app + CheckTimeoutReason.APP_RESTARTED -> Unit - VaultTimeoutAction.LOGOUT -> { - setVaultToLocked(userId = userId) - userLogoutManager.softLogout(userId = userId) + // User no longer active or engaging with the app. + CheckTimeoutReason.APP_BACKGROUNDED, + CheckTimeoutReason.USER_CHANGED, + -> { + handleTimeoutActionWithDelay( + userId = userId, + vaultTimeoutAction = vaultTimeoutAction, + delayInMs = vaultTimeout + .vaultTimeoutInMinutes + ?.minutes + ?.inWholeMilliseconds + ?: 0L, + ) + } } } } } /** - * Sets the "last active time" for the given [userId] to the current time. + * Performs the [VaultTimeoutAction] for the given [userId] after the [delayInMs] has passed. + * + * @see handleTimeoutAction */ - private fun updateLastActiveTime(userId: String) { - val elapsedRealtimeMillis = elapsedRealtimeMillisProvider() - authDiskSource.storeLastActiveTimeMillis( - userId = userId, - lastActiveTimeMillis = elapsedRealtimeMillis, - ) + private fun handleTimeoutActionWithDelay( + userId: String, + vaultTimeoutAction: VaultTimeoutAction, + delayInMs: Long, + ) { + userIdTimerJobMap[userId]?.cancel() + userIdTimerJobMap[userId] = unconfinedScope.launch { + delay(timeMillis = delayInMs) + handleTimeoutAction(userId = userId, vaultTimeoutAction = vaultTimeoutAction) + } + } + + /** + * Performs a lock or soft-logout operation for the given [userId] based on the provided + * [VaultTimeoutAction]. + */ + private fun handleTimeoutAction( + userId: String, + vaultTimeoutAction: VaultTimeoutAction, + ) { + when (vaultTimeoutAction) { + VaultTimeoutAction.LOCK -> { + setVaultToLocked(userId = userId) + } + + VaultTimeoutAction.LOGOUT -> { + setVaultToLocked(userId = userId) + userLogoutManager.softLogout(userId = userId) + } + } } private suspend fun unlockVaultForUser( @@ -514,4 +547,13 @@ class VaultLockManagerImpl( organizationKeys = organizationKeys, ) } + + /** + * Helper enum that indicates the reason we are checking for timeout. + */ + private enum class CheckTimeoutReason { + APP_BACKGROUNDED, + APP_RESTARTED, + USER_CHANGED, + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt index 2c1aef504..22f2d72d7 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt @@ -47,8 +47,6 @@ import com.x8bit.bitwarden.ui.vault.feature.itemlisting.navigateToVaultItemListi import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType import com.x8bit.bitwarden.ui.vault.model.VaultItemCipherType import com.x8bit.bitwarden.ui.vault.model.VaultItemListingType -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach import java.util.concurrent.atomic.AtomicReference /** @@ -69,15 +67,6 @@ fun RootNavScreen( if (isNotSplashScreen) onSplashScreenRemoved() } - LaunchedEffect(Unit) { - navController - .currentBackStackEntryFlow - .onEach { - viewModel.trySendAction(RootNavAction.BackStackUpdate) - } - .launchIn(this) - } - NavHost( navController = navController, startDestination = SPLASH_ROUTE, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModel.kt index 355d97f0d..5e05fab38 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModel.kt @@ -25,7 +25,7 @@ import javax.inject.Inject */ @HiltViewModel class RootNavViewModel @Inject constructor( - private val authRepository: AuthRepository, + authRepository: AuthRepository, specialCircumstanceManager: SpecialCircumstanceManager, ) : BaseViewModel( initialState = RootNavState.Splash, @@ -48,15 +48,10 @@ class RootNavViewModel @Inject constructor( override fun handleAction(action: RootNavAction) { when (action) { - is RootNavAction.BackStackUpdate -> handleBackStackUpdate() is RootNavAction.Internal.UserStateUpdateReceive -> handleUserStateUpdateReceive(action) } } - private fun handleBackStackUpdate() { - authRepository.updateLastActiveTime() - } - @Suppress("CyclomaticComplexMethod", "MaxLineLength") private fun handleUserStateUpdateReceive( action: RootNavAction.Internal.UserStateUpdateReceive, @@ -247,11 +242,6 @@ sealed class RootNavState : Parcelable { */ sealed class RootNavAction { - /** - * Indicates the backstack has changed. - */ - data object BackStackUpdate : RootNavAction() - /** * Internal ViewModel actions. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarScreen.kt index cb343b007..920fe2f44 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarScreen.kt @@ -20,7 +20,6 @@ import androidx.compose.material3.NavigationBarItemDefaults import androidx.compose.material3.ScaffoldDefaults import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf @@ -64,8 +63,6 @@ import com.x8bit.bitwarden.ui.vault.feature.vault.VAULT_GRAPH_ROUTE import com.x8bit.bitwarden.ui.vault.feature.vault.navigateToVaultGraph import com.x8bit.bitwarden.ui.vault.feature.vault.vaultGraph import com.x8bit.bitwarden.ui.vault.model.VaultItemCipherType -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach import kotlinx.parcelize.Parcelize /** @@ -113,14 +110,6 @@ fun VaultUnlockedNavBarScreen( } } } - LaunchedEffect(Unit) { - navController - .currentBackStackEntryFlow - .onEach { - viewModel.trySendAction(VaultUnlockedNavBarAction.BackStackUpdate) - } - .launchIn(this) - } VaultUnlockedNavBarScaffold( state = state, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt index 70796ea7b..13ff5894b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt @@ -54,7 +54,6 @@ class VaultUnlockedNavBarViewModel @Inject constructor( VaultUnlockedNavBarAction.SendTabClick -> handleSendTabClicked() VaultUnlockedNavBarAction.SettingsTabClick -> handleSettingsTabClicked() VaultUnlockedNavBarAction.VaultTabClick -> handleVaultTabClicked() - VaultUnlockedNavBarAction.BackStackUpdate -> handleBackStackUpdate() } } // #region BottomTabViewModel Action Handlers @@ -85,10 +84,6 @@ class VaultUnlockedNavBarViewModel @Inject constructor( private fun handleSettingsTabClicked() { sendEvent(VaultUnlockedNavBarEvent.NavigateToSettingsScreen) } - - private fun handleBackStackUpdate() { - authRepository.updateLastActiveTime() - } // #endregion BottomTabViewModel Action Handlers } @@ -123,11 +118,6 @@ sealed class VaultUnlockedNavBarAction { * click Settings tab. */ data object SettingsTabClick : VaultUnlockedNavBarAction() - - /** - * Indicates the backstack has changed. - */ - data object BackStackUpdate : VaultUnlockedNavBarAction() } /** diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt index cef8b96d2..d5815b287 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt @@ -206,10 +206,6 @@ class AuthDiskSourceTest { userId = userId, pinProtectedUserKey = "pinProtectedUserKey", ) - authDiskSource.storeLastActiveTimeMillis( - userId = userId, - lastActiveTimeMillis = 123456789L, - ) authDiskSource.storeInvalidUnlockAttempts( userId = userId, invalidUnlockAttempts = 1, @@ -252,7 +248,6 @@ class AuthDiskSourceTest { // These should be cleared assertNull(authDiskSource.getUserBiometricUnlockKey(userId = userId)) assertNull(authDiskSource.getPinProtectedUserKey(userId = userId)) - assertNull(authDiskSource.getLastActiveTimeMillis(userId = userId)) assertNull(authDiskSource.getInvalidUnlockAttempts(userId = userId)) assertNull(authDiskSource.getUserKey(userId = userId)) assertNull(authDiskSource.getUserAutoUnlockKey(userId = userId)) @@ -265,63 +260,6 @@ class AuthDiskSourceTest { assertNull(authDiskSource.getMasterPasswordHash(userId = userId)) } - @Test - fun `getLastActiveTimeMillis should pull from SharedPreferences`() { - val lastActiveTimeBaseKey = "bwPreferencesStorage:lastActiveTime" - val mockUserId = "mockUserId" - val mockLastActiveTime = 123456789L - fakeSharedPreferences - .edit { - putLong( - "${lastActiveTimeBaseKey}_$mockUserId", - mockLastActiveTime, - ) - } - val actual = authDiskSource.getLastActiveTimeMillis(userId = mockUserId) - assertEquals( - mockLastActiveTime, - actual, - ) - } - - @Test - fun `storeLastActiveTimeMillis for non-null values should update SharedPreferences`() { - val lastActiveTimeBaseKey = "bwPreferencesStorage:lastActiveTime" - val mockUserId = "mockUserId" - val mockLastActiveTime = 123456789L - authDiskSource.storeLastActiveTimeMillis( - userId = mockUserId, - lastActiveTimeMillis = mockLastActiveTime, - ) - val actual = fakeSharedPreferences - .getLong( - "${lastActiveTimeBaseKey}_$mockUserId", - 0L, - ) - assertEquals( - mockLastActiveTime, - actual, - ) - } - - @Test - fun `storeLastActiveTimeMillis for null values should clear SharedPreferences`() { - val lastActiveTimeBaseKey = "bwPreferencesStorage:lastActiveTime" - val mockUserId = "mockUserId" - val mockLastActiveTime = 123456789L - val lastActiveTimeKey = "${lastActiveTimeBaseKey}_$mockUserId" - fakeSharedPreferences - .edit { - putLong(lastActiveTimeKey, mockLastActiveTime) - } - assertTrue(fakeSharedPreferences.contains(lastActiveTimeKey)) - authDiskSource.storeLastActiveTimeMillis( - userId = mockUserId, - lastActiveTimeMillis = null, - ) - assertFalse(fakeSharedPreferences.contains(lastActiveTimeKey)) - } - @Test fun `getInvalidUnlockAttempts should pull from SharedPreferences`() { val lastActiveTimeBaseKey = "bwPreferencesStorage:invalidUnlockAttempts" diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt index ac7becb7e..7f0cb7cc5 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/util/FakeAuthDiskSource.kt @@ -27,7 +27,6 @@ class FakeAuthDiskSource : AuthDiskSource { private val mutableUserStateFlow = bufferedMutableSharedFlow(replay = 1) private val storedShouldTrustDevice = mutableMapOf() - private val storedLastActiveTimeMillis = mutableMapOf() private val storedInvalidUnlockAttempts = mutableMapOf() private val storedUserKeys = mutableMapOf() private val storedPrivateKeys = mutableMapOf() @@ -55,7 +54,6 @@ class FakeAuthDiskSource : AuthDiskSource { get() = mutableUserStateFlow.onSubscription { emit(userState) } override fun clearData(userId: String) { - storedLastActiveTimeMillis.remove(userId) storedInvalidUnlockAttempts.remove(userId) storedUserKeys.remove(userId) storedPrivateKeys.remove(userId) @@ -81,16 +79,6 @@ class FakeAuthDiskSource : AuthDiskSource { storedShouldTrustDevice[userId] = shouldTrustDevice } - override fun getLastActiveTimeMillis(userId: String): Long? = - storedLastActiveTimeMillis[userId] - - override fun storeLastActiveTimeMillis( - userId: String, - lastActiveTimeMillis: Long?, - ) { - storedLastActiveTimeMillis[userId] = lastActiveTimeMillis - } - override fun getInvalidUnlockAttempts(userId: String): Int? = storedInvalidUnlockAttempts[userId] @@ -240,13 +228,6 @@ class FakeAuthDiskSource : AuthDiskSource { assertEquals(accountTokens, this.storedAccountTokens[userId]) } - /** - * Assert that the [lastActiveTimeMillis] was stored successfully using the [userId]. - */ - fun assertLastActiveTimeMillis(userId: String, lastActiveTimeMillis: Long?) { - assertEquals(lastActiveTimeMillis, storedLastActiveTimeMillis[userId]) - } - /** * Assert that the [invalidUnlockAttempts] was stored successfully using the [userId]. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 082c6c14b..0ab3ac95c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -218,8 +218,6 @@ class AuthRepositoryTest { } returns mutableActivePolicyFlow } - private var elapsedRealtimeMillis = 123456789L - private val repository = AuthRepositoryImpl( accountsService = accountsService, devicesService = devicesService, @@ -238,7 +236,6 @@ class AuthRepositoryTest { dispatcherManager = dispatcherManager, pushManager = pushManager, policyManager = policyManager, - elapsedRealtimeMillisProvider = { elapsedRealtimeMillis }, ) @BeforeEach @@ -4372,21 +4369,6 @@ class AuthRepositoryTest { assertFalse(repository.hasPendingAccountAddition) } - @Test - fun `updateLastActiveTime should update the last active time for the current user`() { - val userId = USER_ID_1 - fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 - - assertNull(fakeAuthDiskSource.getLastActiveTimeMillis(userId = userId)) - - repository.updateLastActiveTime() - - assertEquals( - elapsedRealtimeMillis, - fakeAuthDiskSource.getLastActiveTimeMillis(userId = userId), - ) - } - @Test fun `getIsKnownDevice should return failure when service returns failure`() = runTest { coEvery { diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt index da68c367a..0335d7b6a 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt @@ -35,15 +35,18 @@ import io.mockk.just import io.mockk.mockk import io.mockk.runs import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test +@OptIn(ExperimentalCoroutinesApi::class) @Suppress("LargeClass") class VaultLockManagerTest { private val fakeAuthDiskSource = FakeAuthDiskSource() @@ -73,8 +76,8 @@ class VaultLockManagerTest { every { getVaultTimeoutStateFlow(any()) } returns mutableVaultTimeoutStateFlow every { getVaultTimeoutActionStateFlow(any()) } returns mutableVaultTimeoutActionStateFlow } - - private var elapsedRealtimeMillis = 123456789L + private val testDispatcher = UnconfinedTestDispatcher() + private val fakeDispatcherManager = FakeDispatcherManager(unconfined = testDispatcher) private val vaultLockManager: VaultLockManager = VaultLockManagerImpl( authDiskSource = fakeAuthDiskSource, @@ -84,8 +87,7 @@ class VaultLockManagerTest { appForegroundManager = fakeAppForegroundManager, userLogoutManager = userLogoutManager, trustedDeviceManager = trustedDeviceManager, - dispatcherManager = FakeDispatcherManager(), - elapsedRealtimeMillisProvider = { elapsedRealtimeMillis }, + dispatcherManager = fakeDispatcherManager, ) @Test @@ -139,97 +141,138 @@ class VaultLockManagerTest { } @Test - fun `app going into background should update the current user's last active time`() { + fun `app coming into background subsequent times should perform timeout action if necessary`() { + setAccountTokens() fakeAuthDiskSource.userState = MOCK_USER_STATE // Start in a foregrounded state fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = null, - ) - elapsedRealtimeMillis = 123L - fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED + // Will be used within each loop to reset the test to a suitable initial state. + fun resetTest(vaultTimeout: VaultTimeout) { + clearVerifications(userLogoutManager) + mutableVaultTimeoutStateFlow.value = vaultTimeout + fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED + verifyUnlockedVaultBlocking(userId = USER_ID) + assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) + } - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = 123L, - ) + // Test Lock action + mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK + MOCK_TIMEOUTS.forEach { vaultTimeout -> + resetTest(vaultTimeout = vaultTimeout) + + fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED + // Advance by 6 minutes. Only actions with a timeout less than this will be triggered. + testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 6 * 60 * 1000L) + + when (vaultTimeout) { + // After 6 minutes (or action should not be performed) + VaultTimeout.Never, + VaultTimeout.OnAppRestart, + VaultTimeout.FifteenMinutes, + VaultTimeout.ThirtyMinutes, + VaultTimeout.OneHour, + VaultTimeout.FourHours, + is VaultTimeout.Custom, + -> { + assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) + } + + // Before 6 minutes + VaultTimeout.Immediately, + VaultTimeout.OneMinute, + VaultTimeout.FiveMinutes, + -> { + assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) + } + } + + verify(exactly = 0) { userLogoutManager.softLogout(any()) } + } + + // Test Logout action + mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOGOUT + MOCK_TIMEOUTS.forEach { vaultTimeout -> + resetTest(vaultTimeout = vaultTimeout) + + fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED + // Advance by 6 minutes. Only actions with a timeout less than this will be triggered. + testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 6 * 60 * 1000L) + + when (vaultTimeout) { + // After 6 minutes (or action should not be performed) + VaultTimeout.Never, + VaultTimeout.OnAppRestart, + VaultTimeout.FifteenMinutes, + VaultTimeout.ThirtyMinutes, + VaultTimeout.OneHour, + VaultTimeout.FourHours, + is VaultTimeout.Custom, + -> { + assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) + verify(exactly = 0) { userLogoutManager.softLogout(any()) } + } + + // Before 6 minutes + VaultTimeout.Immediately, + VaultTimeout.OneMinute, + VaultTimeout.FiveMinutes, + -> { + assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) + verify(exactly = 1) { userLogoutManager.softLogout(USER_ID) } + } + } + } } @Suppress("MaxLineLength") @Test - fun `app coming into foreground for the first time for Never timeout should clear existing times and not perform timeout action`() { + fun `app coming into foreground for the first time for Never timeout should not perform timeout action`() { fakeAuthDiskSource.userState = MOCK_USER_STATE mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK mutableVaultTimeoutStateFlow.value = VaultTimeout.Never fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = 123L, - ) verifyUnlockedVaultBlocking(userId = USER_ID) assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = null, - ) } @Suppress("MaxLineLength") @Test - fun `app coming into foreground for the first time for OnAppRestart timeout should clear existing times and lock vaults if necessary`() { + fun `app coming into foreground for the first time for OnAppRestart timeout should lock vaults if necessary`() { setAccountTokens() fakeAuthDiskSource.userState = MOCK_USER_STATE mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK mutableVaultTimeoutStateFlow.value = VaultTimeout.OnAppRestart fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = 123L, - ) verifyUnlockedVaultBlocking(userId = USER_ID) assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = null, - ) } - @Suppress("MaxLineLength") @Test - fun `app coming into foreground for the first time for other timeout should clear existing times and lock vaults if necessary`() { + fun `app coming into foreground for the first time for other timeout should do nothing`() { setAccountTokens() fakeAuthDiskSource.userState = MOCK_USER_STATE mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK mutableVaultTimeoutStateFlow.value = VaultTimeout.ThirtyMinutes fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = 123L, - ) verifyUnlockedVaultBlocking(userId = USER_ID) assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED - assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = null, - ) + assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) } @Suppress("MaxLineLength") @@ -240,24 +283,11 @@ class VaultLockManagerTest { mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK mutableVaultTimeoutStateFlow.value = VaultTimeout.ThirtyMinutes - fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = 123L, - ) - verifyUnlockedVaultBlocking(userId = USER_ID) - assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) - fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = null, - ) } - @Suppress("MaxLineLength") @Test fun `Verify Checking for timeout should take place for a user with logged in state`() { setAccountTokens() @@ -266,10 +296,6 @@ class VaultLockManagerTest { mutableVaultTimeoutStateFlow.value = VaultTimeout.ThirtyMinutes fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = 123L, - ) verifyUnlockedVaultBlocking(userId = USER_ID) assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) @@ -286,10 +312,6 @@ class VaultLockManagerTest { mutableVaultTimeoutStateFlow.value = VaultTimeout.ThirtyMinutes fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = 123L, - ) verifyUnlockedVaultBlocking(userId = USER_ID) assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) @@ -298,33 +320,19 @@ class VaultLockManagerTest { verify(exactly = 0) { settingsRepository.getVaultTimeoutActionStateFlow(USER_ID) } } - @Suppress("MaxLineLength") @Test - fun `app coming into foreground subsequent times should perform timeout action if necessary and not clear existing times`() { + fun `app coming into foreground subsequent times should do nothing`() { setAccountTokens() fakeAuthDiskSource.userState = MOCK_USER_STATE // Start in a foregrounded state - fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = null, - ) - - // Set the last active time to 2 minutes and the current time to 8 minutes, so only times - // beyond 6 minutes perform their action. - val lastActiveTime = 2 * 60 * 1000L - elapsedRealtimeMillis = 8 * 60 * 1000L + fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED // Will be used within each loop to reset the test to a suitable initial state. fun resetTest(vaultTimeout: VaultTimeout) { - clearVerifications(userLogoutManager) mutableVaultTimeoutStateFlow.value = vaultTimeout fakeAppForegroundManager.appForegroundState = AppForegroundState.BACKGROUNDED - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = lastActiveTime, - ) + clearVerifications(userLogoutManager) verifyUnlockedVaultBlocking(userId = USER_ID) assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) } @@ -335,34 +343,12 @@ class VaultLockManagerTest { resetTest(vaultTimeout = vaultTimeout) fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED + // Advance by 6 minutes. Only actions with a timeout less than this will be triggered. + testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 6 * 60 * 1000L) - when (vaultTimeout) { - // After 6 minutes (or action should not be performed) - VaultTimeout.Never, - VaultTimeout.OnAppRestart, - VaultTimeout.FifteenMinutes, - VaultTimeout.ThirtyMinutes, - VaultTimeout.OneHour, - VaultTimeout.FourHours, - is VaultTimeout.Custom, - -> { - assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) - } - - // Before 6 minutes - VaultTimeout.Immediately, - VaultTimeout.OneMinute, - VaultTimeout.FiveMinutes, - -> { - assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) - } - } - + // Vault is never locked while foregrounded, no matter the timeout. + assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) verify(exactly = 0) { userLogoutManager.softLogout(any()) } - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = lastActiveTime, - ) } // Test Logout action @@ -371,43 +357,21 @@ class VaultLockManagerTest { resetTest(vaultTimeout = vaultTimeout) fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED + // Advance by 6 minutes. Only actions with a timeout less than this will be triggered. + testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 6 * 60 * 1000L) - when (vaultTimeout) { - // After 6 minutes (or action should not be performed) - VaultTimeout.Never, - VaultTimeout.OnAppRestart, - VaultTimeout.FifteenMinutes, - VaultTimeout.ThirtyMinutes, - VaultTimeout.OneHour, - VaultTimeout.FourHours, - is VaultTimeout.Custom, - -> { - assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) - verify(exactly = 0) { userLogoutManager.softLogout(any()) } - } - - // Before 6 minutes - VaultTimeout.Immediately, - VaultTimeout.OneMinute, - VaultTimeout.FiveMinutes, - -> { - assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) - verify(exactly = 1) { userLogoutManager.softLogout(USER_ID) } - } - } - - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = lastActiveTime, - ) + // Vault is never locked while foregrounded, no matter the timeout. + assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) + verify(exactly = 0) { userLogoutManager.softLogout(any()) } } } @Suppress("MaxLineLength") @Test - fun `switching users should perform lock actions for each user if necessary and reset their last active times`() { + fun `switching users should perform lock actions or start a timer for each user if necessary`() { val userId2 = "mockId-2" setAccountTokens(listOf(USER_ID, userId2)) + fakeAppForegroundManager.appForegroundState = AppForegroundState.FOREGROUNDED fakeAuthDiskSource.userState = UserStateJson( activeUserId = USER_ID, accounts = mapOf( @@ -416,23 +380,10 @@ class VaultLockManagerTest { ), ) - // Set the last active time to 2 minutes and the current time to 8 minutes, so only times - // beyond 6 minutes perform their action. - val lastActiveTime = 2 * 60 * 1000L - elapsedRealtimeMillis = 8 * 60 * 1000L - // Will be used within each loop to reset the test to a suitable initial state. fun resetTest(vaultTimeout: VaultTimeout) { clearVerifications(userLogoutManager) mutableVaultTimeoutStateFlow.value = vaultTimeout - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = lastActiveTime, - ) - fakeAuthDiskSource.storeLastActiveTimeMillis( - userId = userId2, - lastActiveTimeMillis = lastActiveTime, - ) verifyUnlockedVaultBlocking(userId = USER_ID) verifyUnlockedVaultBlocking(userId = userId2) assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) @@ -444,13 +395,14 @@ class VaultLockManagerTest { MOCK_TIMEOUTS.forEach { vaultTimeout -> resetTest(vaultTimeout = vaultTimeout) + val activeUserCheck = fakeAuthDiskSource.userState?.activeUserId == USER_ID + val activeUserId = if (activeUserCheck) userId2 else USER_ID + val inactiveUserId = if (activeUserCheck) USER_ID else userId2 fakeAuthDiskSource.userState = fakeAuthDiskSource.userState?.copy( - activeUserId = if (fakeAuthDiskSource.userState?.activeUserId == USER_ID) { - userId2 - } else { - USER_ID - }, + activeUserId = activeUserId, ) + // Advance by 6 minutes. Only actions with a timeout less than this will be triggered. + testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 6 * 60 * 1000L) when (vaultTimeout) { // After 6 minutes (or action should not be performed) @@ -462,8 +414,8 @@ class VaultLockManagerTest { VaultTimeout.FourHours, is VaultTimeout.Custom, -> { - assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) - assertTrue(vaultLockManager.isVaultUnlocked(userId2)) + assertTrue(vaultLockManager.isVaultUnlocked(activeUserId)) + assertTrue(vaultLockManager.isVaultUnlocked(inactiveUserId)) } // Before 6 minutes @@ -471,20 +423,12 @@ class VaultLockManagerTest { VaultTimeout.OneMinute, VaultTimeout.FiveMinutes, -> { - assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) - assertFalse(vaultLockManager.isVaultUnlocked(userId2)) + assertTrue(vaultLockManager.isVaultUnlocked(activeUserId)) + assertFalse(vaultLockManager.isVaultUnlocked(inactiveUserId)) } } verify(exactly = 0) { userLogoutManager.softLogout(any()) } - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = elapsedRealtimeMillis, - ) - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = userId2, - lastActiveTimeMillis = elapsedRealtimeMillis, - ) } // Test Logout action @@ -492,13 +436,14 @@ class VaultLockManagerTest { MOCK_TIMEOUTS.forEach { vaultTimeout -> resetTest(vaultTimeout = vaultTimeout) + val activeUserCheck = fakeAuthDiskSource.userState?.activeUserId == USER_ID + val activeUserId = if (activeUserCheck) userId2 else USER_ID + val inactiveUserId = if (activeUserCheck) USER_ID else userId2 fakeAuthDiskSource.userState = fakeAuthDiskSource.userState?.copy( - activeUserId = if (fakeAuthDiskSource.userState?.activeUserId == USER_ID) { - userId2 - } else { - USER_ID - }, + activeUserId = activeUserId, ) + // Advance by 6 minutes. Only actions with a timeout less than this will be triggered. + testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 6 * 60 * 1000L) when (vaultTimeout) { // After 6 minutes (or action should not be performed) @@ -510,8 +455,8 @@ class VaultLockManagerTest { VaultTimeout.FourHours, is VaultTimeout.Custom, -> { - assertTrue(vaultLockManager.isVaultUnlocked(USER_ID)) - assertTrue(vaultLockManager.isVaultUnlocked(userId2)) + assertTrue(vaultLockManager.isVaultUnlocked(activeUserId)) + assertTrue(vaultLockManager.isVaultUnlocked(inactiveUserId)) verify(exactly = 0) { userLogoutManager.softLogout(any()) } } @@ -520,21 +465,12 @@ class VaultLockManagerTest { VaultTimeout.OneMinute, VaultTimeout.FiveMinutes, -> { - assertFalse(vaultLockManager.isVaultUnlocked(USER_ID)) - assertFalse(vaultLockManager.isVaultUnlocked(userId2)) - verify(exactly = 1) { userLogoutManager.softLogout(USER_ID) } - verify(exactly = 1) { userLogoutManager.softLogout(userId2) } + assertTrue(vaultLockManager.isVaultUnlocked(activeUserId)) + assertFalse(vaultLockManager.isVaultUnlocked(inactiveUserId)) + verify(exactly = 0) { userLogoutManager.softLogout(activeUserId) } + verify(exactly = 1) { userLogoutManager.softLogout(inactiveUserId) } } } - - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = USER_ID, - lastActiveTimeMillis = elapsedRealtimeMillis, - ) - fakeAuthDiskSource.assertLastActiveTimeMillis( - userId = userId2, - lastActiveTimeMillis = elapsedRealtimeMillis, - ) } } @@ -1558,8 +1494,11 @@ class VaultLockManagerTest { private fun setAccountTokens(userIds: List = listOf(USER_ID)) { userIds.forEach { userId -> fakeAuthDiskSource.storeAccountTokens( - userId, - accountTokens = AccountTokensJson("access-$userId", "refresh-$userId"), + userId = userId, + accountTokens = AccountTokensJson( + accessToken = "access-$userId", + refreshToken = "refresh-$userId", + ), ) } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt index f545a048b..0b714557f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt @@ -13,10 +13,7 @@ import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import io.mockk.every -import io.mockk.just import io.mockk.mockk -import io.mockk.runs -import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test @@ -25,7 +22,6 @@ class RootNavViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(null) private val authRepository = mockk { every { userStateFlow } returns mutableUserStateFlow - every { updateLastActiveTime() } just runs } private val specialCircumstanceManager = SpecialCircumstanceManagerImpl() @@ -557,13 +553,6 @@ class RootNavViewModelTest : BaseViewModelTest() { assertEquals(RootNavState.VaultLocked, viewModel.stateFlow.value) } - @Test - fun `BackStackUpdate should call updateLastActiveTime`() { - val viewModel = createViewModel() - viewModel.trySendAction(RootNavAction.BackStackUpdate) - verify { authRepository.updateLastActiveTime() } - } - private fun createViewModel(): RootNavViewModel = RootNavViewModel( authRepository = authRepository, diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModelTest.kt index bc150a944..f4e6e9936 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModelTest.kt @@ -21,7 +21,6 @@ class VaultUnlockedNavBarViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(null) private val authRepository: AuthRepository = mockk { every { userStateFlow } returns mutableUserStateFlow - every { updateLastActiveTime() } just runs } private val specialCircumstancesManager: SpecialCircumstanceManager = mockk { every { specialCircumstance = null } just runs @@ -160,13 +159,6 @@ class VaultUnlockedNavBarViewModelTest : BaseViewModelTest() { } } - @Test - fun `BackStackUpdate should call updateLastActiveTime`() { - val viewModel = createViewModel() - viewModel.trySendAction(VaultUnlockedNavBarAction.BackStackUpdate) - verify { authRepository.updateLastActiveTime() } - } - private fun createViewModel() = VaultUnlockedNavBarViewModel( authRepository = authRepository,