PM-8534 update the active account after a "soft logout" (#3456)

app/src/main/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt

@@ -34,37 +34,19 @@ class UserLogoutManagerImpl(
     private val mainScope = CoroutineScope(dispatcherManager.main)
 
     override fun logout(userId: String, isExpired: Boolean) {
-        val currentUserState = authDiskSource.userState ?: return
+        authDiskSource.userState ?: return
 
         if (isExpired) {
             showToast(message = R.string.login_expired)
         }
 
-        // Remove the active user from the accounts map
-        val updatedAccounts = currentUserState
-            .accounts
-            .filterKeys { it != userId }
-
-        // Check if there is a new active user
-        if (updatedAccounts.isNotEmpty()) {
-            if (userId == currentUserState.activeUserId && !isExpired) {
-                showToast(message = R.string.account_switched_automatically)
-            }
-
-            // If we logged out a non-active user, we want to leave the active user unchanged.
-            // If we logged out the active user, we want to set the active user to the first one
-            // in the list.
-            val updatedActiveUserId = currentUserState
-                .activeUserId
-                .takeUnless { it == userId }
-                ?: updatedAccounts.entries.first().key
-
-            // Update the user information and emit an updated token
-            authDiskSource.userState = currentUserState.copy(
-                activeUserId = updatedActiveUserId,
-                accounts = updatedAccounts,
+        val ableToSwitchToNewAccount = switchUserIfAvailable(
+            currentUserId = userId,
+            isExpired = isExpired,
+            removeCurrentUserFromAccounts = true,
         )
-        } else {
+
+        if (!ableToSwitchToNewAccount) {
             // Update the user information and log out
             authDiskSource.userState = null
         }
@@ -82,6 +64,8 @@ class UserLogoutManagerImpl(
         val vaultTimeoutInMinutes = settingsDiskSource.getVaultTimeoutInMinutes(userId = userId)
         val vaultTimeoutAction = settingsDiskSource.getVaultTimeoutAction(userId = userId)
 
+        switchUserIfAvailable(currentUserId = userId, removeCurrentUserFromAccounts = false)
+
         clearData(userId = userId)
 
         // Restore data that is still required
@@ -112,4 +96,46 @@ class UserLogoutManagerImpl(
     private fun showToast(@StringRes message: Int) {
         mainScope.launch { Toast.makeText(context, message, Toast.LENGTH_SHORT).show() }
     }
+
+    private fun switchUserIfAvailable(
+        currentUserId: String,
+        removeCurrentUserFromAccounts: Boolean,
+        isExpired: Boolean = false,
+    ): Boolean {
+        val currentUserState = authDiskSource.userState ?: return false
+
+        val currentAccountsMap = currentUserState.accounts
+
+        // Remove the active user from the accounts map
+        val updatedAccounts = currentAccountsMap
+            .filterKeys { it != currentUserId }
+
+        // Check if there is a new active user
+        return if (updatedAccounts.isNotEmpty()) {
+            if (currentUserId == currentUserState.activeUserId && !isExpired) {
+                showToast(message = R.string.account_switched_automatically)
+            }
+
+            // If we logged out a non-active user, we want to leave the active user unchanged.
+            // If we logged out the active user, we want to set the active user to the first one
+            // in the list.
+            val updatedActiveUserId = currentUserState
+                .activeUserId
+                .takeUnless { it == currentUserId }
+                ?: updatedAccounts.entries.first().key
+
+            // Update the user information and emit an updated token
+            authDiskSource.userState = currentUserState.copy(
+                activeUserId = updatedActiveUserId,
+                accounts = if (removeCurrentUserFromAccounts) {
+                    updatedAccounts
+                } else {
+                    currentAccountsMap
+                },
+            )
+            true
+        } else {
+            false
+        }
+    }
 }

app/src/main/java/com/x8bit/bitwarden/data/autofill/util/CipherViewExtensions.kt

@@ -49,4 +49,4 @@ fun CipherView.toAutofillCipherProvider(): AutofillCipherProvider =
  * Returns true when the cipher is not deleted and contains at least one FIDO 2 credential.
  */
 val CipherView.isActiveWithFido2Credentials: Boolean
-    get() = deletedDate == null && login?.fido2Credentials.isNullOrEmpty().not()
+    get() = deletedDate == null && !(login?.fido2Credentials.isNullOrEmpty())

app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt

@@ -11,6 +11,7 @@ import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSource
 import com.x8bit.bitwarden.data.auth.manager.TrustedDeviceManager
 import com.x8bit.bitwarden.data.auth.manager.UserLogoutManager
 import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
+import com.x8bit.bitwarden.data.auth.repository.util.userAccountTokens
 import com.x8bit.bitwarden.data.auth.repository.util.userSwitchingChangesFlow
 import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager
 import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
@@ -435,6 +436,17 @@ class VaultLockManagerImpl(
         userId: String,
         isAppRestart: Boolean = false,
     ) {
+        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
+        }
+
         val currentTimeMillis = elapsedRealtimeMillisProvider()
         val lastActiveTimeMillis = authDiskSource.getLastActiveTimeMillis(userId = userId) ?: 0
         val vaultTimeout = settingsRepository.getVaultTimeoutStateFlow(userId = userId).value

app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/landing/LandingViewModel.kt

@@ -5,6 +5,7 @@ import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.viewModelScope
 import com.x8bit.bitwarden.R
 import com.x8bit.bitwarden.data.auth.repository.AuthRepository
+import com.x8bit.bitwarden.data.auth.repository.model.UserState
 import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
 import com.x8bit.bitwarden.data.platform.repository.model.Environment
 import com.x8bit.bitwarden.data.vault.repository.VaultRepository
@@ -16,6 +17,7 @@ import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary
 import com.x8bit.bitwarden.ui.vault.feature.vault.util.toAccountSummaries
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.update
 import kotlinx.parcelize.Parcelize
@@ -76,6 +78,16 @@ class LandingViewModel @Inject constructor(
                 )
             }
             .launchIn(viewModelScope)
+
+        authRepository
+            .userStateFlow
+            .map { userState ->
+                userState?.activeAccount?.let(::mapToInternalActionOrNull)
+            }
+            .onEach { action ->
+                action?.let(::handleAction)
+            }
+            .launchIn(viewModelScope)
     }
 
     override fun handleAction(action: LandingAction) {
@@ -91,8 +103,15 @@ class LandingViewModel @Inject constructor(
             LandingAction.CreateAccountClick -> handleCreateAccountClicked()
             is LandingAction.DialogDismiss -> handleDialogDismiss()
             is LandingAction.RememberMeToggle -> handleRememberMeToggled(action)
-            is LandingAction.EmailInputChanged -> handleEmailInputUpdated(action)
+            is LandingAction.EmailInputChanged -> handleEmailInputChanged(action)
             is LandingAction.EnvironmentTypeSelect -> handleEnvironmentTypeSelect(action)
+            is LandingAction.Internal -> handleInternalActions(action)
+        }
+    }
+
+    private fun handleInternalActions(action: LandingAction.Internal) {
+        when (action) {
+            is LandingAction.Internal.UpdateEmailState -> handleInternalEmailStateUpdate(action)
             is LandingAction.Internal.UpdatedEnvironmentReceive -> {
                 handleUpdatedEnvironmentReceive(action)
             }
@@ -117,12 +136,19 @@ class LandingViewModel @Inject constructor(
         authRepository.switchAccount(userId = action.accountSummary.userId)
     }
 
-    private fun handleEmailInputUpdated(action: LandingAction.EmailInputChanged) {
-        val email = action.input
+    private fun handleEmailInputChanged(action: LandingAction.EmailInputChanged) {
+        updateEmailInput(action.input)
+    }
+
+    private fun handleInternalEmailStateUpdate(action: LandingAction.Internal.UpdateEmailState) {
+        updateEmailInput(action.emailInput)
+    }
+
+    private fun updateEmailInput(updatedInput: String) {
         mutableStateFlow.update {
             it.copy(
-                emailInput = email,
-                isContinueButtonEnabled = email.isNotBlank(),
+                emailInput = updatedInput,
+                isContinueButtonEnabled = updatedInput.isNotBlank(),
             )
         }
     }
@@ -198,6 +224,19 @@ class LandingViewModel @Inject constructor(
             )
         }
     }
+
+    /**
+     * If the user state account is changed to an active but not "logged in" account we can
+     * pre-populate the email field with this account.
+     */
+    private fun mapToInternalActionOrNull(
+        activeAccount: UserState.Account,
+    ): LandingAction.Internal.UpdateEmailState? {
+        val activeUserNotLoggedIn = !activeAccount.isLoggedIn
+        val noPendingAdditions = !authRepository.hasPendingAccountAddition
+        return LandingAction.Internal.UpdateEmailState(activeAccount.email)
+            .takeIf { activeUserNotLoggedIn && noPendingAdditions }
+    }
 }
 
 /**
@@ -341,5 +380,10 @@ sealed class LandingAction {
         data class UpdatedEnvironmentReceive(
             val environment: Environment,
         ) : Internal()
+
+        /**
+         * Internal action to update the email input state from a non-user action
+         */
+        data class UpdateEmailState(val emailInput: String) : Internal()
     }
 }

app/src/test/java/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt

@@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.auth.manager
 
 import android.content.Context
 import android.widget.Toast
+import androidx.annotation.StringRes
 import com.x8bit.bitwarden.R
 import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
 import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson
@@ -92,17 +93,7 @@ class UserLogoutManagerTest {
     @Suppress("MaxLineLength")
     @Test
     fun `logout for multiple accounts should clear data associated with the given user and change to the new active user`() {
-        mockkStatic(Toast::class)
-
-        every {
-            Toast
-                .makeText(
-                    context,
-                    R.string.account_switched_automatically,
-                    Toast.LENGTH_SHORT,
-                )
-                .show()
-        } just runs
+        mockToast(R.string.account_switched_automatically)
 
         val userId = USER_ID_1
         every { authDiskSource.userState } returns MULTI_USER_STATE
@@ -130,7 +121,11 @@ class UserLogoutManagerTest {
     fun `softLogout should clear most data associated with the given user and remove token data in the authDiskSource`() {
         val userId = USER_ID_1
         val vaultTimeoutInMinutes = 360
-        val vaultTimeoutAction = VaultTimeoutAction.LOCK
+        val vaultTimeoutAction = VaultTimeoutAction.LOGOUT
+
+        mockToast(R.string.account_switched_automatically)
+
+        every { authDiskSource.userState } returns MULTI_USER_STATE
         every {
             settingsDiskSource.getVaultTimeoutInMinutes(userId = userId)
         } returns vaultTimeoutInMinutes
@@ -157,6 +152,32 @@ class UserLogoutManagerTest {
         }
     }
 
+    @Suppress("MaxLineLength")
+    @Test
+    fun `softLogout should switch active user but keep previous user in accounts list`() {
+        val userId = USER_ID_1
+        val vaultTimeoutInMinutes = 360
+        val vaultTimeoutAction = VaultTimeoutAction.LOGOUT
+
+        mockToast(R.string.account_switched_automatically)
+
+        every { authDiskSource.userState } returns MULTI_USER_STATE
+        every {
+            settingsDiskSource.getVaultTimeoutInMinutes(userId = userId)
+        } returns vaultTimeoutInMinutes
+        every {
+            settingsDiskSource.getVaultTimeoutAction(userId = userId)
+        } returns vaultTimeoutAction
+
+        userLogoutManager.softLogout(userId = userId)
+
+        verify { authDiskSource.storeAccountTokens(userId = USER_ID_1, accountTokens = null) }
+        verify {
+            authDiskSource.userState =
+                UserStateJson(activeUserId = USER_ID_2, accounts = MULTI_USER_STATE.accounts)
+        }
+    }
+
     private fun assertDataCleared(userId: String) {
         verify { vaultSdkSource.clearCrypto(userId = userId) }
         verify { authDiskSource.clearData(userId = userId) }
@@ -168,6 +189,15 @@ class UserLogoutManagerTest {
             vaultDiskSource.deleteVaultData(userId = userId)
         }
     }
+
+    private fun mockToast(@StringRes res: Int) {
+        mockkStatic(Toast::class)
+        every {
+            Toast
+                .makeText(context, res, Toast.LENGTH_SHORT)
+                .show()
+        } just runs
+    }
 }
 
 private const val EMAIL_2 = "test2@bitwarden.com"

app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt

@@ -185,6 +185,7 @@ class VaultLockManagerTest {
     @Suppress("MaxLineLength")
     @Test
     fun `app coming into foreground for the first time for OnAppRestart timeout should clear existing times and lock vaults if necessary`() {
+        setAccountTokens()
         fakeAuthDiskSource.userState = MOCK_USER_STATE
         mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK
         mutableVaultTimeoutStateFlow.value = VaultTimeout.OnAppRestart
@@ -209,6 +210,7 @@ class VaultLockManagerTest {
     @Suppress("MaxLineLength")
     @Test
     fun `app coming into foreground for the first time for other timeout should clear existing times and lock vaults if necessary`() {
+        setAccountTokens()
         fakeAuthDiskSource.userState = MOCK_USER_STATE
         mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK
         mutableVaultTimeoutStateFlow.value = VaultTimeout.ThirtyMinutes
@@ -233,6 +235,7 @@ class VaultLockManagerTest {
     @Suppress("MaxLineLength")
     @Test
     fun `app coming into foreground for the first time for non-Never timeout should clear existing times and perform timeout action`() {
+        setAccountTokens()
         fakeAuthDiskSource.userState = MOCK_USER_STATE
         mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK
         mutableVaultTimeoutStateFlow.value = VaultTimeout.ThirtyMinutes
@@ -254,9 +257,51 @@ class VaultLockManagerTest {
         )
     }
 
+    @Suppress("MaxLineLength")
+    @Test
+    fun `Verify Checking for timeout should take place for a user with logged in state`() {
+        setAccountTokens()
+        fakeAuthDiskSource.userState = MOCK_USER_STATE
+        mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOGOUT
+        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
+
+        verify(exactly = 1) { settingsRepository.getVaultTimeoutActionStateFlow(USER_ID) }
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `Verify Checking for timeout should not take place for a user who is already in the soft logged out state`() {
+        fakeAuthDiskSource.userState = MOCK_USER_STATE
+        mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOGOUT
+        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
+
+        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`() {
+        setAccountTokens()
         fakeAuthDiskSource.userState = MOCK_USER_STATE
 
         // Start in a foregrounded state
@@ -362,6 +407,7 @@ class VaultLockManagerTest {
     @Test
     fun `switching users should perform lock actions for each user if necessary and reset their last active times`() {
         val userId2 = "mockId-2"
+        setAccountTokens(listOf(USER_ID, userId2))
         fakeAuthDiskSource.userState = UserStateJson(
             activeUserId = USER_ID,
             accounts = mapOf(
@@ -1507,6 +1553,16 @@ class VaultLockManagerTest {
     private fun verifyUnlockedVaultBlocking(userId: String) {
         runBlocking { verifyUnlockedVault(userId = userId) }
     }
+
+    // region helper functions
+    private fun setAccountTokens(userIds: List<String> = listOf(USER_ID)) {
+        userIds.forEach { userId ->
+            fakeAuthDiskSource.storeAccountTokens(
+                userId,
+                accountTokens = AccountTokensJson("access-$userId", "refresh-$userId"),
+            )
+        }
+    }
 }
 
 private const val USER_ID = "mockId-1"

app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/landing/LandingViewModelTest.kt

@@ -5,6 +5,7 @@ import app.cash.turbine.test
 import com.x8bit.bitwarden.R
 import com.x8bit.bitwarden.data.auth.repository.AuthRepository
 import com.x8bit.bitwarden.data.auth.repository.model.UserState
+import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType
 import com.x8bit.bitwarden.data.platform.repository.model.Environment
 import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository
 import com.x8bit.bitwarden.data.vault.repository.VaultRepository
@@ -21,6 +22,7 @@ import io.mockk.verify
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
 
 class LandingViewModelTest : BaseViewModelTest() {
@@ -399,6 +401,72 @@ class LandingViewModelTest : BaseViewModelTest() {
         }
     }
 
+    @Test
+    fun `Active Logged Out user causes email field to prepopulate`() = runTest {
+        val expectedEmail = "frodo@hobbit.on"
+        val userId = "1"
+
+        val userAccount: UserState.Account = UserState.Account(
+            userId = userId,
+            name = null,
+            email = expectedEmail,
+            avatarColorHex = "lorem",
+            environment = Environment.Us,
+            isPremium = false,
+            isLoggedIn = false,
+            isVaultUnlocked = false,
+            needsPasswordReset = false,
+            needsMasterPassword = false,
+            trustedDevice = null,
+            organizations = listOf(),
+            isBiometricsEnabled = false,
+            vaultUnlockType = VaultUnlockType.MASTER_PASSWORD,
+        )
+
+        val userState = UserState(
+            activeUserId = userId,
+            accounts = listOf(userAccount),
+        )
+
+        val viewModel = createViewModel(userState = userState)
+
+        assertEquals(expectedEmail, viewModel.stateFlow.value.emailInput)
+    }
+
+    @Test
+    fun `Email input will not change based on active user when adding new account`() = runTest {
+        val expectedEmail = "frodo@hobbit.on"
+        val userId = "1"
+
+        val userAccount: UserState.Account = UserState.Account(
+            userId = userId,
+            name = null,
+            email = expectedEmail,
+            avatarColorHex = "lorem",
+            environment = Environment.Us,
+            isPremium = false,
+            isLoggedIn = false,
+            isVaultUnlocked = false,
+            needsPasswordReset = false,
+            needsMasterPassword = false,
+            trustedDevice = null,
+            organizations = listOf(),
+            isBiometricsEnabled = false,
+            vaultUnlockType = VaultUnlockType.MASTER_PASSWORD,
+        )
+
+        val userState = UserState(
+            activeUserId = userId,
+            accounts = listOf(userAccount),
+        )
+
+        every { authRepository.hasPendingAccountAddition } returns true
+
+        val viewModel = createViewModel(userState = userState)
+
+        assertTrue(viewModel.stateFlow.value.emailInput.isEmpty())
+    }
+
     //region Helper methods
 
     private fun createViewModel(