Skip to content

Commit 6223f36

Browse files
dseverns-livefrontdseverns-livefront
authored
PM-16062 Prevent account locks for ongoing autofill requests (#4498)
1 parent 1148e48 commit 6223f36

File tree

7 files changed

+204
-152
lines changed

7 files changed

+204
-152
lines changed

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

+10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
package com.x8bit.bitwarden.data.autofill.util
44

5+
import android.app.Activity
56
import android.app.PendingIntent
67
import android.app.assist.AssistStructure
78
import android.content.Context
@@ -147,3 +148,12 @@ fun Intent.getAutofillSelectionDataOrNull(): AutofillSelectionData? =
147148
fun Intent.getTotpCopyIntentOrNull(): AutofillTotpCopyData? =
148149
getBundleExtra(AUTOFILL_BUNDLE_KEY)
149150
?.getSafeParcelableExtra(AUTOFILL_TOTP_COPY_DATA_KEY)
151+
152+
/**
153+
* Checks if the given [Activity] was created for Autofill. This is useful to avoid locking the
154+
* vault if one of the Autofill services starts the only only instance of the [MainActivity].
155+
*/
156+
val Activity.createdForAutofill: Boolean
157+
get() = intent.getAutofillSelectionDataOrNull() != null ||
158+
intent.getAutofillSaveItemOrNull() != null ||
159+
intent.getAutofillAssistStructureOrNull() != null

‎app/src/main/java/com/x8bit/bitwarden/data/platform/manager/AppStateManagerImpl.kt

+7-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import android.os.Bundle
66
import androidx.lifecycle.DefaultLifecycleObserver
77
import androidx.lifecycle.LifecycleOwner
88
import androidx.lifecycle.ProcessLifecycleOwner
9+
import com.x8bit.bitwarden.data.autofill.util.createdForAutofill
910
import com.x8bit.bitwarden.data.platform.manager.model.AppCreationState
1011
import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState
1112
import kotlinx.coroutines.flow.MutableStateFlow
@@ -19,7 +20,8 @@ class AppStateManagerImpl(
1920
application: Application,
2021
processLifecycleOwner: LifecycleOwner = ProcessLifecycleOwner.get(),
2122
) : AppStateManager {
22-
private val mutableAppCreationStateFlow = MutableStateFlow(AppCreationState.DESTROYED)
23+
private val mutableAppCreationStateFlow =
24+
MutableStateFlow<AppCreationState>(AppCreationState.Destroyed)
2325
private val mutableAppForegroundStateFlow = MutableStateFlow(AppForegroundState.BACKGROUNDED)
2426

2527
override val appCreatedStateFlow: StateFlow<AppCreationState>
@@ -49,13 +51,15 @@ class AppStateManagerImpl(
4951
override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
5052
activityCount++
5153
// Always be in a created state if we have an activity
52-
mutableAppCreationStateFlow.value = AppCreationState.CREATED
54+
mutableAppCreationStateFlow.value = AppCreationState.Created(
55+
isAutoFill = activity.createdForAutofill,
56+
)
5357
}
5458

5559
override fun onActivityDestroyed(activity: Activity) {
5660
activityCount--
5761
if (activityCount == 0 && !activity.isChangingConfigurations) {
58-
mutableAppCreationStateFlow.value = AppCreationState.DESTROYED
62+
mutableAppCreationStateFlow.value = AppCreationState.Destroyed
5963
}
6064
}
6165

‎app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/AppCreationState.kt

+5-3
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@ package com.x8bit.bitwarden.data.platform.manager.model
33
/**
44
* Represents the creation state of the app.
55
*/
6-
enum class AppCreationState {
6+
sealed class AppCreationState {
77
/**
88
* Denotes that the app is currently created.
9+
*
10+
* @param isAutoFill Whether the app was created for autofill.
911
*/
10-
CREATED,
12+
data class Created(val isAutoFill: Boolean) : AppCreationState()
1113

1214
/**
1315
* Denotes that the app is currently destroyed.
1416
*/
15-
DESTROYED,
17+
data object Destroyed : AppCreationState()
1618
}

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

+71-40
Original file line numberDiff line numberDiff line change
@@ -305,29 +305,40 @@ class VaultLockManagerImpl(
305305
}
306306

307307
private fun observeAppCreationChanges() {
308+
var isFirstCreated = true
308309
appStateManager
309310
.appCreatedStateFlow
310311
.onEach { appCreationState ->
311312
when (appCreationState) {
312-
AppCreationState.CREATED -> Unit
313-
AppCreationState.DESTROYED -> handleOnDestroyed()
313+
is AppCreationState.Created -> {
314+
handleOnCreated(
315+
createdForAutofill = appCreationState.isAutoFill,
316+
isFirstCreated = isFirstCreated,
317+
)
318+
isFirstCreated = false
319+
}
320+
321+
AppCreationState.Destroyed -> Unit
314322
}
315323
}
316324
.launchIn(unconfinedScope)
317325
}
318326

319-
private fun handleOnDestroyed() {
320-
activeUserId?.let { userId ->
321-
checkForVaultTimeout(
322-
userId = userId,
323-
checkTimeoutReason = CheckTimeoutReason.APP_RESTARTED,
324-
)
325-
}
327+
private fun handleOnCreated(
328+
createdForAutofill: Boolean,
329+
isFirstCreated: Boolean,
330+
) {
331+
val userId = activeUserId ?: return
332+
checkForVaultTimeout(
333+
userId = userId,
334+
checkTimeoutReason = CheckTimeoutReason.AppCreated(
335+
firstTimeCreation = isFirstCreated,
336+
createdForAutofill = createdForAutofill,
337+
),
338+
)
326339
}
327340

328341
private fun observeAppForegroundChanges() {
329-
var isFirstForeground = true
330-
331342
appStateManager
332343
.appForegroundStateFlow
333344
.onEach { appForegroundState ->
@@ -336,10 +347,7 @@ class VaultLockManagerImpl(
336347
handleOnBackground()
337348
}
338349

339-
AppForegroundState.FOREGROUNDED -> {
340-
handleOnForeground(isFirstForeground = isFirstForeground)
341-
isFirstForeground = false
342-
}
350+
AppForegroundState.FOREGROUNDED -> handleOnForeground()
343351
}
344352
}
345353
.launchIn(unconfinedScope)
@@ -349,19 +357,13 @@ class VaultLockManagerImpl(
349357
val userId = activeUserId ?: return
350358
checkForVaultTimeout(
351359
userId = userId,
352-
checkTimeoutReason = CheckTimeoutReason.APP_BACKGROUNDED,
360+
checkTimeoutReason = CheckTimeoutReason.AppBackgrounded,
353361
)
354362
}
355363

356-
private fun handleOnForeground(isFirstForeground: Boolean) {
364+
private fun handleOnForeground() {
357365
val userId = activeUserId ?: return
358366
userIdTimerJobMap[userId]?.cancel()
359-
if (isFirstForeground) {
360-
checkForVaultTimeout(
361-
userId = userId,
362-
checkTimeoutReason = CheckTimeoutReason.APP_RESTARTED,
363-
)
364-
}
365367
}
366368

367369
private fun observeUserSwitchingChanges() {
@@ -461,7 +463,7 @@ class VaultLockManagerImpl(
461463
// Check if the user's timeout action should be performed as we switch away.
462464
checkForVaultTimeout(
463465
userId = previousActiveUserId,
464-
checkTimeoutReason = CheckTimeoutReason.USER_CHANGED,
466+
checkTimeoutReason = CheckTimeoutReason.UserChanged,
465467
)
466468
}
467469

@@ -491,27 +493,38 @@ class VaultLockManagerImpl(
491493

492494
VaultTimeout.OnAppRestart -> {
493495
// If this is an app restart, trigger the timeout action; otherwise ignore.
494-
if (checkTimeoutReason == CheckTimeoutReason.APP_RESTARTED) {
495-
// On restart the vault should be locked already but we may need to soft-logout
496-
// the user.
497-
handleTimeoutAction(userId = userId, vaultTimeoutAction = vaultTimeoutAction)
496+
if (checkTimeoutReason is CheckTimeoutReason.AppCreated) {
497+
// We need to check the timeout action on the first time creation no matter what
498+
// for all subsequent creations we should check if this is for autofill and
499+
// and if it is we skip checking the timeout action.
500+
if (
501+
checkTimeoutReason.firstTimeCreation ||
502+
!checkTimeoutReason.createdForAutofill
503+
) {
504+
handleTimeoutAction(
505+
userId = userId,
506+
vaultTimeoutAction = vaultTimeoutAction,
507+
)
508+
}
498509
}
499510
}
500511

501512
else -> {
502513
when (checkTimeoutReason) {
503514
// Always preform the timeout action on app restart to ensure the user is
504515
// in the correct state.
505-
CheckTimeoutReason.APP_RESTARTED -> {
506-
handleTimeoutAction(
507-
userId = userId,
508-
vaultTimeoutAction = vaultTimeoutAction,
509-
)
516+
is CheckTimeoutReason.AppCreated -> {
517+
if (checkTimeoutReason.firstTimeCreation) {
518+
handleTimeoutAction(
519+
userId = userId,
520+
vaultTimeoutAction = vaultTimeoutAction,
521+
)
522+
}
510523
}
511524

512525
// User no longer active or engaging with the app.
513-
CheckTimeoutReason.APP_BACKGROUNDED,
514-
CheckTimeoutReason.USER_CHANGED,
526+
CheckTimeoutReason.AppBackgrounded,
527+
CheckTimeoutReason.UserChanged,
515528
-> {
516529
handleTimeoutActionWithDelay(
517530
userId = userId,
@@ -589,11 +602,29 @@ class VaultLockManagerImpl(
589602
}
590603

591604
/**
592-
* Helper enum that indicates the reason we are checking for timeout.
605+
* Helper sealed class which denotes the reason to check the vault timeout.
593606
*/
594-
private enum class CheckTimeoutReason {
595-
APP_BACKGROUNDED,
596-
APP_RESTARTED,
597-
USER_CHANGED,
607+
private sealed class CheckTimeoutReason {
608+
/**
609+
* Indicates the app has been backgrounded but is still running.
610+
*/
611+
data object AppBackgrounded : CheckTimeoutReason()
612+
613+
/**
614+
* Indicates the app has entered a Created state.
615+
*
616+
* @param firstTimeCreation if this is the first time the process is being created.
617+
* @param createdForAutofill if the the creation event is due to an activity being launched
618+
* for autofill.
619+
*/
620+
data class AppCreated(
621+
val firstTimeCreation: Boolean,
622+
val createdForAutofill: Boolean,
623+
) : CheckTimeoutReason()
624+
625+
/**
626+
* Indicates that the current user has changed.
627+
*/
628+
data object UserChanged : CheckTimeoutReason()
598629
}
599630
}

‎app/src/test/java/com/x8bit/bitwarden/data/platform/manager/AppStateManagerTest.kt

+9-3
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,17 @@ package com.x8bit.bitwarden.data.platform.manager
33
import android.app.Activity
44
import android.app.Application
55
import app.cash.turbine.test
6+
import com.x8bit.bitwarden.data.autofill.util.createdForAutofill
67
import com.x8bit.bitwarden.data.platform.manager.model.AppCreationState
78
import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState
89
import com.x8bit.bitwarden.data.util.FakeLifecycleOwner
910
import io.mockk.every
1011
import io.mockk.just
1112
import io.mockk.mockk
13+
import io.mockk.mockkStatic
1214
import io.mockk.runs
1315
import io.mockk.slot
16+
import io.mockk.unmockkStatic
1417
import kotlinx.coroutines.test.runTest
1518
import org.junit.jupiter.api.Assertions.assertEquals
1619
import org.junit.jupiter.api.Test
@@ -59,15 +62,17 @@ class AppStateManagerTest {
5962
@Test
6063
fun `appCreatedStateFlow should emit whenever the underlying activities are all destroyed or a creation event occurs`() =
6164
runTest {
65+
mockkStatic(Activity::createdForAutofill)
6266
val activity = mockk<Activity> {
6367
every { isChangingConfigurations } returns false
68+
every { createdForAutofill } returns false
6469
}
6570
appStateManager.appCreatedStateFlow.test {
6671
// Initial state is DESTROYED
67-
assertEquals(AppCreationState.DESTROYED, awaitItem())
72+
assertEquals(AppCreationState.Destroyed, awaitItem())
6873

6974
activityLifecycleCallbacks.captured.onActivityCreated(activity, null)
70-
assertEquals(AppCreationState.CREATED, awaitItem())
75+
assertEquals(AppCreationState.Created(isAutoFill = false), awaitItem())
7176

7277
activityLifecycleCallbacks.captured.onActivityCreated(activity, null)
7378
expectNoEvents()
@@ -76,7 +81,8 @@ class AppStateManagerTest {
7681
expectNoEvents()
7782

7883
activityLifecycleCallbacks.captured.onActivityDestroyed(activity)
79-
assertEquals(AppCreationState.DESTROYED, awaitItem())
84+
assertEquals(AppCreationState.Destroyed, awaitItem())
8085
}
86+
unmockkStatic(Activity::createdForAutofill)
8187
}
8288
}

‎app/src/test/java/com/x8bit/bitwarden/data/platform/manager/util/FakeAppStateManager.kt

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import kotlinx.coroutines.flow.asStateFlow
1111
* A faked implementation of [AppStateManager]
1212
*/
1313
class FakeAppStateManager : AppStateManager {
14-
private val mutableAppCreationStateFlow = MutableStateFlow(AppCreationState.DESTROYED)
14+
private val mutableAppCreationStateFlow =
15+
MutableStateFlow<AppCreationState>(AppCreationState.Destroyed)
1516
private val mutableAppForegroundStateFlow = MutableStateFlow(AppForegroundState.BACKGROUNDED)
1617

1718
override val appCreatedStateFlow: StateFlow<AppCreationState>

0 commit comments

Comments
 (0)