mirror of
https://github.com/bitwarden/android.git
synced 2025-02-23 00:59:16 +03:00
PM-15804, PM-17130: Add logic to monitor when the screen on state to ensure the vault locks properly (#4618)
This commit is contained in:
parent
e63a549485
commit
54456d3d4f
3 changed files with 131 additions and 12 deletions
app/src
main/java/com/x8bit/bitwarden/data/vault/manager
test/java/com/x8bit/bitwarden/data/vault/manager
|
@ -1,5 +1,9 @@
|
||||||
package com.x8bit.bitwarden.data.vault.manager
|
package com.x8bit.bitwarden.data.vault.manager
|
||||||
|
|
||||||
|
import android.content.BroadcastReceiver
|
||||||
|
import android.content.Context
|
||||||
|
import android.content.Intent
|
||||||
|
import android.content.IntentFilter
|
||||||
import com.bitwarden.core.InitOrgCryptoRequest
|
import com.bitwarden.core.InitOrgCryptoRequest
|
||||||
import com.bitwarden.core.InitUserCryptoMethod
|
import com.bitwarden.core.InitUserCryptoMethod
|
||||||
import com.bitwarden.core.InitUserCryptoRequest
|
import com.bitwarden.core.InitUserCryptoRequest
|
||||||
|
@ -50,6 +54,8 @@ import kotlinx.coroutines.flow.onCompletion
|
||||||
import kotlinx.coroutines.flow.onEach
|
import kotlinx.coroutines.flow.onEach
|
||||||
import kotlinx.coroutines.flow.update
|
import kotlinx.coroutines.flow.update
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
|
import java.time.Clock
|
||||||
|
import java.util.concurrent.ConcurrentHashMap
|
||||||
import kotlin.time.Duration.Companion.minutes
|
import kotlin.time.Duration.Companion.minutes
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -62,6 +68,7 @@ private const val MAXIMUM_INVALID_UNLOCK_ATTEMPTS = 5
|
||||||
*/
|
*/
|
||||||
@Suppress("TooManyFunctions", "LongParameterList")
|
@Suppress("TooManyFunctions", "LongParameterList")
|
||||||
class VaultLockManagerImpl(
|
class VaultLockManagerImpl(
|
||||||
|
private val clock: Clock,
|
||||||
private val authDiskSource: AuthDiskSource,
|
private val authDiskSource: AuthDiskSource,
|
||||||
private val authSdkSource: AuthSdkSource,
|
private val authSdkSource: AuthSdkSource,
|
||||||
private val vaultSdkSource: VaultSdkSource,
|
private val vaultSdkSource: VaultSdkSource,
|
||||||
|
@ -70,13 +77,15 @@ class VaultLockManagerImpl(
|
||||||
private val userLogoutManager: UserLogoutManager,
|
private val userLogoutManager: UserLogoutManager,
|
||||||
private val trustedDeviceManager: TrustedDeviceManager,
|
private val trustedDeviceManager: TrustedDeviceManager,
|
||||||
dispatcherManager: DispatcherManager,
|
dispatcherManager: DispatcherManager,
|
||||||
|
context: Context,
|
||||||
) : VaultLockManager {
|
) : VaultLockManager {
|
||||||
private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined)
|
private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This [Map] tracks all active timeout [Job]s that are running using the user ID as the key.
|
* This [Map] tracks all active timeout [Job]s that are running and their associated data using
|
||||||
|
* the user ID as the key.
|
||||||
*/
|
*/
|
||||||
private val userIdTimerJobMap = mutableMapOf<String, Job>()
|
private val userIdTimerJobMap: MutableMap<String, TimeoutJobData> = ConcurrentHashMap()
|
||||||
|
|
||||||
private val activeUserId: String? get() = authDiskSource.userState?.activeUserId
|
private val activeUserId: String? get() = authDiskSource.userState?.activeUserId
|
||||||
|
|
||||||
|
@ -96,6 +105,10 @@ class VaultLockManagerImpl(
|
||||||
observeUserSwitchingChanges()
|
observeUserSwitchingChanges()
|
||||||
observeVaultTimeoutChanges()
|
observeVaultTimeoutChanges()
|
||||||
observeUserLogoutResults()
|
observeUserLogoutResults()
|
||||||
|
context.registerReceiver(
|
||||||
|
ScreenStateBroadcastReceiver(),
|
||||||
|
IntentFilter(Intent.ACTION_SCREEN_ON),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun isVaultUnlocked(userId: String): Boolean =
|
override fun isVaultUnlocked(userId: String): Boolean =
|
||||||
|
@ -363,7 +376,7 @@ class VaultLockManagerImpl(
|
||||||
|
|
||||||
private fun handleOnForeground() {
|
private fun handleOnForeground() {
|
||||||
val userId = activeUserId ?: return
|
val userId = activeUserId ?: return
|
||||||
userIdTimerJobMap[userId]?.cancel()
|
userIdTimerJobMap.remove(key = userId)?.job?.cancel()
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun observeUserSwitchingChanges() {
|
private fun observeUserSwitchingChanges() {
|
||||||
|
@ -459,7 +472,7 @@ class VaultLockManagerImpl(
|
||||||
currentActiveUserId: String,
|
currentActiveUserId: String,
|
||||||
) {
|
) {
|
||||||
// Make sure to clear the now-active user's timeout job.
|
// Make sure to clear the now-active user's timeout job.
|
||||||
userIdTimerJobMap[currentActiveUserId]?.cancel()
|
userIdTimerJobMap.remove(key = currentActiveUserId)?.job?.cancel()
|
||||||
// Check if the user's timeout action should be performed as we switch away.
|
// Check if the user's timeout action should be performed as we switch away.
|
||||||
checkForVaultTimeout(
|
checkForVaultTimeout(
|
||||||
userId = previousActiveUserId,
|
userId = previousActiveUserId,
|
||||||
|
@ -529,7 +542,7 @@ class VaultLockManagerImpl(
|
||||||
handleTimeoutActionWithDelay(
|
handleTimeoutActionWithDelay(
|
||||||
userId = userId,
|
userId = userId,
|
||||||
vaultTimeoutAction = vaultTimeoutAction,
|
vaultTimeoutAction = vaultTimeoutAction,
|
||||||
delayInMs = vaultTimeout
|
delayMs = vaultTimeout
|
||||||
.vaultTimeoutInMinutes
|
.vaultTimeoutInMinutes
|
||||||
?.minutes
|
?.minutes
|
||||||
?.inWholeMilliseconds
|
?.inWholeMilliseconds
|
||||||
|
@ -542,20 +555,26 @@ class VaultLockManagerImpl(
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Performs the [VaultTimeoutAction] for the given [userId] after the [delayInMs] has passed.
|
* Performs the [VaultTimeoutAction] for the given [userId] after the [delayMs] has passed.
|
||||||
*
|
*
|
||||||
* @see handleTimeoutAction
|
* @see handleTimeoutAction
|
||||||
*/
|
*/
|
||||||
private fun handleTimeoutActionWithDelay(
|
private fun handleTimeoutActionWithDelay(
|
||||||
userId: String,
|
userId: String,
|
||||||
vaultTimeoutAction: VaultTimeoutAction,
|
vaultTimeoutAction: VaultTimeoutAction,
|
||||||
delayInMs: Long,
|
delayMs: Long,
|
||||||
) {
|
) {
|
||||||
userIdTimerJobMap[userId]?.cancel()
|
userIdTimerJobMap.remove(key = userId)?.job?.cancel()
|
||||||
userIdTimerJobMap[userId] = unconfinedScope.launch {
|
userIdTimerJobMap[userId] = TimeoutJobData(
|
||||||
delay(timeMillis = delayInMs)
|
job = unconfinedScope.launch {
|
||||||
handleTimeoutAction(userId = userId, vaultTimeoutAction = vaultTimeoutAction)
|
delay(timeMillis = delayMs)
|
||||||
}
|
userIdTimerJobMap.remove(key = userId)
|
||||||
|
handleTimeoutAction(userId = userId, vaultTimeoutAction = vaultTimeoutAction)
|
||||||
|
},
|
||||||
|
vaultTimeoutAction = vaultTimeoutAction,
|
||||||
|
startTimeMs = clock.millis(),
|
||||||
|
durationMs = delayMs,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -601,6 +620,37 @@ class VaultLockManagerImpl(
|
||||||
return (accounts.find { it.userId == userId }?.isLoggedIn) == false
|
return (accounts.find { it.userId == userId }?.isLoggedIn) == false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A custom [BroadcastReceiver] that listens for when the screen is powered on and restarts the
|
||||||
|
* vault timeout jobs to ensure they complete at the correct time.
|
||||||
|
*
|
||||||
|
* This is necessary because the [delay] function in a coroutine will not keep accurate time
|
||||||
|
* when the screen is off. We do not cancel the job when the screen is off, this allows the
|
||||||
|
* job to complete as-soon-as possible if the screen is powered off for an extended period.
|
||||||
|
*/
|
||||||
|
private inner class ScreenStateBroadcastReceiver : BroadcastReceiver() {
|
||||||
|
override fun onReceive(context: Context, intent: Intent) {
|
||||||
|
userIdTimerJobMap.map { (userId, data) ->
|
||||||
|
handleTimeoutActionWithDelay(
|
||||||
|
userId = userId,
|
||||||
|
vaultTimeoutAction = data.vaultTimeoutAction,
|
||||||
|
delayMs = data.durationMs - (clock.millis() - data.startTimeMs)
|
||||||
|
.coerceAtLeast(minimumValue = 0L),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A wrapper class containing all relevant data concerning a timeout action [Job].
|
||||||
|
*/
|
||||||
|
private data class TimeoutJobData(
|
||||||
|
val job: Job,
|
||||||
|
val vaultTimeoutAction: VaultTimeoutAction,
|
||||||
|
val startTimeMs: Long,
|
||||||
|
val durationMs: Long,
|
||||||
|
)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Helper sealed class which denotes the reason to check the vault timeout.
|
* Helper sealed class which denotes the reason to check the vault timeout.
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -71,6 +71,8 @@ object VaultManagerModule {
|
||||||
@Provides
|
@Provides
|
||||||
@Singleton
|
@Singleton
|
||||||
fun provideVaultLockManager(
|
fun provideVaultLockManager(
|
||||||
|
@ApplicationContext context: Context,
|
||||||
|
clock: Clock,
|
||||||
authDiskSource: AuthDiskSource,
|
authDiskSource: AuthDiskSource,
|
||||||
authSdkSource: AuthSdkSource,
|
authSdkSource: AuthSdkSource,
|
||||||
vaultSdkSource: VaultSdkSource,
|
vaultSdkSource: VaultSdkSource,
|
||||||
|
@ -81,6 +83,8 @@ object VaultManagerModule {
|
||||||
trustedDeviceManager: TrustedDeviceManager,
|
trustedDeviceManager: TrustedDeviceManager,
|
||||||
): VaultLockManager =
|
): VaultLockManager =
|
||||||
VaultLockManagerImpl(
|
VaultLockManagerImpl(
|
||||||
|
context = context,
|
||||||
|
clock = clock,
|
||||||
authDiskSource = authDiskSource,
|
authDiskSource = authDiskSource,
|
||||||
authSdkSource = authSdkSource,
|
authSdkSource = authSdkSource,
|
||||||
vaultSdkSource = vaultSdkSource,
|
vaultSdkSource = vaultSdkSource,
|
||||||
|
|
|
@ -1,5 +1,8 @@
|
||||||
package com.x8bit.bitwarden.data.vault.manager
|
package com.x8bit.bitwarden.data.vault.manager
|
||||||
|
|
||||||
|
import android.content.BroadcastReceiver
|
||||||
|
import android.content.Context
|
||||||
|
import android.content.Intent
|
||||||
import app.cash.turbine.test
|
import app.cash.turbine.test
|
||||||
import com.bitwarden.core.InitOrgCryptoRequest
|
import com.bitwarden.core.InitOrgCryptoRequest
|
||||||
import com.bitwarden.core.InitUserCryptoMethod
|
import com.bitwarden.core.InitUserCryptoMethod
|
||||||
|
@ -36,6 +39,7 @@ import io.mockk.every
|
||||||
import io.mockk.just
|
import io.mockk.just
|
||||||
import io.mockk.mockk
|
import io.mockk.mockk
|
||||||
import io.mockk.runs
|
import io.mockk.runs
|
||||||
|
import io.mockk.slot
|
||||||
import io.mockk.verify
|
import io.mockk.verify
|
||||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||||
import kotlinx.coroutines.async
|
import kotlinx.coroutines.async
|
||||||
|
@ -49,11 +53,18 @@ import org.junit.jupiter.api.Assertions.assertEquals
|
||||||
import org.junit.jupiter.api.Assertions.assertFalse
|
import org.junit.jupiter.api.Assertions.assertFalse
|
||||||
import org.junit.jupiter.api.Assertions.assertTrue
|
import org.junit.jupiter.api.Assertions.assertTrue
|
||||||
import org.junit.jupiter.api.Test
|
import org.junit.jupiter.api.Test
|
||||||
|
import java.time.Clock
|
||||||
|
import java.time.Instant
|
||||||
|
import java.time.ZoneOffset
|
||||||
import java.time.ZonedDateTime
|
import java.time.ZonedDateTime
|
||||||
|
|
||||||
@OptIn(ExperimentalCoroutinesApi::class)
|
@OptIn(ExperimentalCoroutinesApi::class)
|
||||||
@Suppress("LargeClass")
|
@Suppress("LargeClass")
|
||||||
class VaultLockManagerTest {
|
class VaultLockManagerTest {
|
||||||
|
private val broadcastReceiver = slot<BroadcastReceiver>()
|
||||||
|
private val context: Context = mockk {
|
||||||
|
every { registerReceiver(capture(broadcastReceiver), any()) } returns null
|
||||||
|
}
|
||||||
private val fakeAuthDiskSource = FakeAuthDiskSource()
|
private val fakeAuthDiskSource = FakeAuthDiskSource()
|
||||||
private val fakeAppStateManager = FakeAppStateManager()
|
private val fakeAppStateManager = FakeAppStateManager()
|
||||||
private val authSdkSource: AuthSdkSource = mockk {
|
private val authSdkSource: AuthSdkSource = mockk {
|
||||||
|
@ -88,6 +99,8 @@ class VaultLockManagerTest {
|
||||||
private val fakeDispatcherManager = FakeDispatcherManager(unconfined = testDispatcher)
|
private val fakeDispatcherManager = FakeDispatcherManager(unconfined = testDispatcher)
|
||||||
|
|
||||||
private val vaultLockManager: VaultLockManager = VaultLockManagerImpl(
|
private val vaultLockManager: VaultLockManager = VaultLockManagerImpl(
|
||||||
|
context = context,
|
||||||
|
clock = FIXED_CLOCK,
|
||||||
authDiskSource = fakeAuthDiskSource,
|
authDiskSource = fakeAuthDiskSource,
|
||||||
authSdkSource = authSdkSource,
|
authSdkSource = authSdkSource,
|
||||||
vaultSdkSource = vaultSdkSource,
|
vaultSdkSource = vaultSdkSource,
|
||||||
|
@ -98,6 +111,53 @@ class VaultLockManagerTest {
|
||||||
dispatcherManager = fakeDispatcherManager,
|
dispatcherManager = fakeDispatcherManager,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `broadcast receiver should be registered on initialization`() {
|
||||||
|
verify(exactly = 1) {
|
||||||
|
context.registerReceiver(any(), any())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `broadcast intent should reset active job`() {
|
||||||
|
setAccountTokens()
|
||||||
|
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||||
|
|
||||||
|
// Setup state as unlocked
|
||||||
|
mutableVaultTimeoutStateFlow.value = VaultTimeout.OneMinute
|
||||||
|
mutableVaultTimeoutActionStateFlow.value = VaultTimeoutAction.LOCK
|
||||||
|
fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED
|
||||||
|
verifyUnlockedVaultBlocking(userId = USER_ID)
|
||||||
|
assertTrue(vaultLockManager.isVaultUnlocked(USER_ID))
|
||||||
|
|
||||||
|
// Background the app
|
||||||
|
fakeAppStateManager.appForegroundState = AppForegroundState.BACKGROUNDED
|
||||||
|
|
||||||
|
// Advance by 30 seconds (half of what is required to lock the app)
|
||||||
|
testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 30 * 1000L)
|
||||||
|
|
||||||
|
// Still unlocked
|
||||||
|
assertTrue(vaultLockManager.isVaultUnlocked(USER_ID))
|
||||||
|
|
||||||
|
// Receive the screen on event
|
||||||
|
broadcastReceiver.captured.onReceive(context, Intent())
|
||||||
|
|
||||||
|
// Still unlocked
|
||||||
|
assertTrue(vaultLockManager.isVaultUnlocked(USER_ID))
|
||||||
|
|
||||||
|
// Because the test clock is fixed, this should mean that we need to advance the clock a
|
||||||
|
// full minute to get the vault to lock.
|
||||||
|
testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 30 * 1000L)
|
||||||
|
|
||||||
|
// Still unlocked
|
||||||
|
assertTrue(vaultLockManager.isVaultUnlocked(USER_ID))
|
||||||
|
|
||||||
|
testDispatcher.scheduler.advanceTimeBy(delayTimeMillis = 31 * 1000L)
|
||||||
|
|
||||||
|
// Finally locked
|
||||||
|
assertFalse(vaultLockManager.isVaultUnlocked(USER_ID))
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `vaultStateEventFlow should emit Locked event when vault state changes to locked`() =
|
fun `vaultStateEventFlow should emit Locked event when vault state changes to locked`() =
|
||||||
runTest {
|
runTest {
|
||||||
|
@ -1587,6 +1647,11 @@ class VaultLockManagerTest {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private val FIXED_CLOCK: Clock = Clock.fixed(
|
||||||
|
Instant.parse("2023-10-27T12:00:00Z"),
|
||||||
|
ZoneOffset.UTC,
|
||||||
|
)
|
||||||
|
|
||||||
private const val USER_ID = "mockId-1"
|
private const val USER_ID = "mockId-1"
|
||||||
|
|
||||||
private val MOCK_TIMEOUTS = VaultTimeout.Type.entries.map {
|
private val MOCK_TIMEOUTS = VaultTimeout.Type.entries.map {
|
||||||
|
|
Loading…
Add table
Reference in a new issue