[PM-10058] Non-remembered device TDE issue in same session (#3631)

This commit is contained in:
Dave Severns 2024-08-06 13:34:04 -04:00 committed by GitHub
parent af82261fba
commit a090000826
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 66 additions and 12 deletions

View file

@ -1,9 +1,18 @@
package com.x8bit.bitwarden.data.auth.manager
import com.x8bit.bitwarden.data.auth.manager.model.LogoutEvent
import kotlinx.coroutines.flow.SharedFlow
/**
* Manages the logging out of users and clearing of their data.
*/
interface UserLogoutManager {
/**
* Observable flow of [LogoutEvent]s
*/
val logoutEventFlow: SharedFlow<LogoutEvent>
/**
* Completely logs out the given [userId], removing all data.
* If [isExpired] is true, a toast will be displayed

View file

@ -5,14 +5,19 @@ 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.manager.model.LogoutEvent
import com.x8bit.bitwarden.data.platform.datasource.disk.PushDiskSource
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.GeneratorDiskSource
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.PasswordHistoryDiskSource
import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.launch
/**
@ -33,6 +38,10 @@ class UserLogoutManagerImpl(
private val scope = CoroutineScope(dispatcherManager.unconfined)
private val mainScope = CoroutineScope(dispatcherManager.main)
private val mutableLogoutEventFlow: MutableSharedFlow<LogoutEvent> =
bufferedMutableSharedFlow()
override val logoutEventFlow: SharedFlow<LogoutEvent> = mutableLogoutEventFlow.asSharedFlow()
override fun logout(userId: String, isExpired: Boolean) {
authDiskSource.userState ?: return
@ -52,6 +61,7 @@ class UserLogoutManagerImpl(
}
clearData(userId = userId)
mutableLogoutEventFlow.tryEmit(LogoutEvent(loggedOutUserId = userId))
}
override fun softLogout(userId: String) {
@ -67,6 +77,7 @@ class UserLogoutManagerImpl(
switchUserIfAvailable(currentUserId = userId, removeCurrentUserFromAccounts = false)
clearData(userId = userId)
mutableLogoutEventFlow.tryEmit(LogoutEvent(loggedOutUserId = userId))
// Restore data that is still required
settingsDiskSource.apply {

View file

@ -0,0 +1,9 @@
package com.x8bit.bitwarden.data.auth.manager.model
/**
* Result class to share the [loggedOutUserId] of a user
* that was successfully logged out.
*/
data class LogoutEvent(
val loggedOutUserId: String,
)

View file

@ -93,6 +93,7 @@ class VaultLockManagerImpl(
observeAppForegroundChanges()
observeUserSwitchingChanges()
observeVaultTimeoutChanges()
observeUserLogoutResults()
}
override fun isVaultUnlocked(userId: String): Boolean =
@ -229,7 +230,7 @@ class VaultLockManagerImpl(
)
if (invalidUnlockAttempts >= MAXIMUM_INVALID_UNLOCK_ATTEMPTS) {
userLogoutManager.logout(userId)
userLogoutManager.logout(userId = userId)
}
}
@ -370,6 +371,15 @@ class VaultLockManagerImpl(
.launchIn(unconfinedScope)
}
private fun observeUserLogoutResults() {
userLogoutManager
.logoutEventFlow
.onEach {
setVaultToLocked(it.loggedOutUserId)
}
.launchIn(unconfinedScope)
}
private fun vaultTimeoutChangesForUserFlow(userId: String) =
settingsRepository
.getVaultTimeoutStateFlow(userId = userId)
@ -440,13 +450,11 @@ class VaultLockManagerImpl(
userId: String,
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 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
if (isUserLoggedOut(userId = userId)) return
val vaultTimeout = settingsRepository.getVaultTimeoutStateFlow(userId = userId).value
val vaultTimeoutAction = settingsRepository
.getVaultTimeoutActionStateFlow(userId = userId)
@ -523,7 +531,6 @@ class VaultLockManagerImpl(
}
VaultTimeoutAction.LOGOUT -> {
setVaultToLocked(userId = userId)
userLogoutManager.softLogout(userId = userId)
}
}
@ -548,6 +555,11 @@ class VaultLockManagerImpl(
)
}
private fun isUserLoggedOut(userId: String): Boolean {
val accounts = authDiskSource.userAccountTokens
return (accounts.find { it.userId == userId }?.isLoggedIn) == false
}
/**
* Helper enum that indicates the reason we are checking for timeout.
*/

View file

@ -12,6 +12,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource
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.manager.model.LogoutEvent
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager
import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState
@ -37,7 +38,9 @@ import io.mockk.runs
import io.mockk.verify
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runTest
@ -64,9 +67,12 @@ class VaultLockManagerTest {
private val vaultSdkSource: VaultSdkSource = mockk {
every { clearCrypto(userId = any()) } just runs
}
private val mutableLogoutResultFlow = MutableSharedFlow<LogoutEvent>()
private val userLogoutManager: UserLogoutManager = mockk {
every { logout(any()) } just runs
every { softLogout(any()) } just runs
every { logoutEventFlow } returns mutableLogoutResultFlow.asSharedFlow()
}
private val trustedDeviceManager: TrustedDeviceManager = mockk()
private val mutableVaultTimeoutStateFlow =
@ -195,7 +201,6 @@ class VaultLockManagerTest {
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)
@ -210,7 +215,6 @@ class VaultLockManagerTest {
VaultTimeout.FourHours,
is VaultTimeout.Custom,
-> {
assertTrue(vaultLockManager.isVaultUnlocked(USER_ID))
verify(exactly = 0) { userLogoutManager.softLogout(any()) }
}
@ -219,7 +223,6 @@ class VaultLockManagerTest {
VaultTimeout.OneMinute,
VaultTimeout.FiveMinutes,
-> {
assertFalse(vaultLockManager.isVaultUnlocked(USER_ID))
verify(exactly = 1) { userLogoutManager.softLogout(USER_ID) }
}
}
@ -455,8 +458,6 @@ class VaultLockManagerTest {
VaultTimeout.FourHours,
is VaultTimeout.Custom,
-> {
assertTrue(vaultLockManager.isVaultUnlocked(activeUserId))
assertTrue(vaultLockManager.isVaultUnlocked(inactiveUserId))
verify(exactly = 0) { userLogoutManager.softLogout(any()) }
}
@ -465,8 +466,6 @@ class VaultLockManagerTest {
VaultTimeout.OneMinute,
VaultTimeout.FiveMinutes,
-> {
assertTrue(vaultLockManager.isVaultUnlocked(activeUserId))
assertFalse(vaultLockManager.isVaultUnlocked(inactiveUserId))
verify(exactly = 0) { userLogoutManager.softLogout(activeUserId) }
verify(exactly = 1) { userLogoutManager.softLogout(inactiveUserId) }
}
@ -1368,6 +1367,20 @@ class VaultLockManagerTest {
}
}
@Test
fun `When new LogoutResult is observed set the vault to locked for that user`() = runTest {
verifyUnlockedVault(USER_ID)
vaultLockManager.vaultStateEventFlow.test {
mutableLogoutResultFlow.emit(LogoutEvent(loggedOutUserId = USER_ID))
assertEquals(VaultStateEvent.Locked(userId = USER_ID), awaitItem())
assertFalse(vaultLockManager.isVaultUnlocked(USER_ID))
}
verify(exactly = 1) {
vaultSdkSource.clearCrypto(USER_ID)
}
}
/**
* Resets the verification call count for the given [mock] while leaving all other mocked
* behavior in place.