PM-10379: Update the timeout action logic to occur immediately after …

…requirements are met (#3652)

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].
      */

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))
 

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.
      */

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,

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<String, Job>()
+
     private val activeUserId: String? get() = authDiskSource.userState?.activeUserId
-    private val userIds: Set<String> get() = authDiskSource.userState?.accounts?.keys.orEmpty()
 
     private val mutableVaultUnlockDataStateFlow =
         MutableStateFlow<List<VaultUnlockData>>(emptyList())
@@ -307,31 +309,37 @@ 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
                     }
                 }
             }
             .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,66 +438,95 @@ 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
             }
 
             VaultTimeout.OnAppRestart -> {
                 // If this is an app restart, trigger the timeout action; otherwise ignore.
-                if (isAppRestart) 0 else return
-            }
-
-            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)
+                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)
                 }
+            }
 
-                VaultTimeoutAction.LOGOUT -> {
-                    setVaultToLocked(userId = userId)
-                    userLogoutManager.softLogout(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
+
+                    // 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,
+    }
 }