Get autofill enabled information more reliably (#867)

This commit is contained in:
Brian Yencho 2024-01-30 09:43:55 -06:00 committed by Álison Fernandes
parent 2de2ade7a6
commit f93db195c0
14 changed files with 281 additions and 92 deletions

View file

@ -12,6 +12,7 @@ import androidx.core.os.LocaleListCompat
import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.lifecycle.lifecycleScope
import com.x8bit.bitwarden.data.autofill.manager.AutofillActivityManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillCompletionManager
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
import com.x8bit.bitwarden.ui.platform.feature.rootnav.RootNavScreen
@ -29,6 +30,9 @@ class MainActivity : AppCompatActivity() {
private val mainViewModel: MainViewModel by viewModels()
@Inject
lateinit var autofillActivityManager: AutofillActivityManager
@Inject
lateinit var autofillCompletionManager: AutofillCompletionManager

View file

@ -1,25 +1,49 @@
package com.x8bit.bitwarden.data.autofill.di
import com.x8bit.bitwarden.MainActivity
import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManagerImpl
import android.app.Activity
import android.view.autofill.AutofillManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillActivityManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillActivityManagerImpl
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.android.components.ActivityRetainedComponent
import dagger.hilt.android.scopes.ActivityRetainedScoped
import dagger.hilt.android.components.ActivityComponent
import dagger.hilt.android.scopes.ActivityScoped
/**
* Provides dependencies in the autofill package that must be scoped to a retained Activity. These
* are for dependencies that must operate independently in different application tasks that contain
* unique [MainActivity] instances.
* Provides dependencies in the autofill package that must be scoped to a single Activity. These
* are for dependencies that require a very specific Activity's context to operate.
*/
@Module
@InstallIn(ActivityRetainedComponent::class)
@InstallIn(ActivityComponent::class)
object ActivityAutofillModule {
@ActivityRetainedScoped
@ActivityScoped
@Provides
fun provideAutofillSelectionManager(): AutofillSelectionManager =
AutofillSelectionManagerImpl()
fun provideAutofillActivityManager(
@ActivityScopedManager autofillManager: AutofillManager,
appForegroundManager: AppForegroundManager,
autofillEnabledManager: AutofillEnabledManager,
dispatcherManager: DispatcherManager,
): AutofillActivityManager =
AutofillActivityManagerImpl(
autofillManager = autofillManager,
appForegroundManager = appForegroundManager,
autofillEnabledManager = autofillEnabledManager,
dispatcherManager = dispatcherManager,
)
/**
* An AutofillManager specific to the given Activity. This wll give more accurate results
* compared to the global manager.
*/
@ActivityScoped
@ActivityScopedManager
@Provides
fun provideActivityScopedAutofillManager(
activity: Activity,
): AutofillManager = activity.getSystemService(AutofillManager::class.java)
}

View file

@ -0,0 +1,25 @@
package com.x8bit.bitwarden.data.autofill.di
import com.x8bit.bitwarden.MainActivity
import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManagerImpl
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.android.components.ActivityRetainedComponent
import dagger.hilt.android.scopes.ActivityRetainedScoped
/**
* Provides dependencies in the autofill package that must be scoped to a retained Activity. These
* are for dependencies that must operate independently in different application tasks that contain
* unique [MainActivity] instances.
*/
@Module
@InstallIn(ActivityRetainedComponent::class)
object ActivityRetainedAutofillModule {
@ActivityRetainedScoped
@Provides
fun provideAutofillSelectionManager(): AutofillSelectionManager =
AutofillSelectionManagerImpl()
}

View file

@ -0,0 +1,10 @@
package com.x8bit.bitwarden.data.autofill.di
import javax.inject.Qualifier
/**
* Used to denote that the particular manager is scoped to a single Activity instance.
*/
@Qualifier
@Retention(AnnotationRetention.RUNTIME)
annotation class ActivityScopedManager

View file

@ -9,6 +9,8 @@ import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilder
import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilderImpl
import com.x8bit.bitwarden.data.autofill.manager.AutofillCompletionManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillCompletionManagerImpl
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManagerImpl
import com.x8bit.bitwarden.data.autofill.parser.AutofillParser
import com.x8bit.bitwarden.data.autofill.parser.AutofillParserImpl
import com.x8bit.bitwarden.data.autofill.processor.AutofillProcessor
@ -39,6 +41,11 @@ object AutofillModule {
@ApplicationContext context: Context,
): AutofillManager = context.getSystemService(AutofillManager::class.java)
@Singleton
@Provides
fun providesAutofillEnabledManager(): AutofillEnabledManager =
AutofillEnabledManagerImpl()
@Singleton
@Provides
fun provideAutofillCompletionManager(

View file

@ -0,0 +1,10 @@
package com.x8bit.bitwarden.data.autofill.manager
import android.app.Activity
/**
* A helper for dealing with autofill configuration that must be scoped to a specific [Activity].
* In particular, this should be injected into an [Activity] to ensure that an
* [AutofillEnabledManager] reports correct values.
*/
interface AutofillActivityManager

View file

@ -0,0 +1,34 @@
package com.x8bit.bitwarden.data.autofill.manager
import android.view.autofill.AutofillManager
import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
/**
* Primary implementation of [AutofillActivityManager].
*/
class AutofillActivityManagerImpl(
private val autofillManager: AutofillManager,
private val appForegroundManager: AppForegroundManager,
private val autofillEnabledManager: AutofillEnabledManager,
private val dispatcherManager: DispatcherManager,
) : AutofillActivityManager {
private val isAutofillEnabledAndSupported: Boolean
get() = autofillManager.isEnabled &&
autofillManager.hasEnabledAutofillServices() &&
autofillManager.isAutofillSupported
private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined)
init {
appForegroundManager
.appForegroundStateFlow
.onEach {
autofillEnabledManager.isAutofillEnabled = isAutofillEnabledAndSupported
}
.launchIn(unconfinedScope)
}
}

View file

@ -0,0 +1,22 @@
package com.x8bit.bitwarden.data.autofill.manager
import kotlinx.coroutines.flow.StateFlow
/**
* A container for values specifying whether or not autofill is enabled. These values should be
* filled by an [AutofillActivityManager].
*/
interface AutofillEnabledManager {
/**
* Whether or not autofill should be considered enabled.
*
* Note that changing this does not enable or disable autofill; it is only an indicator that
* this has occurred elsewhere.
*/
var isAutofillEnabled: Boolean
/**
* Emits updates that track [isAutofillEnabled] values.
*/
val isAutofillEnabledStateFlow: StateFlow<Boolean>
}

View file

@ -0,0 +1,21 @@
package com.x8bit.bitwarden.data.autofill.manager
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
/**
* Primary implementation of [AutofillEnabledManager].
*/
class AutofillEnabledManagerImpl : AutofillEnabledManager {
private val mutableIsAutofillEnabledStateFlow = MutableStateFlow(false)
override var isAutofillEnabled: Boolean
get() = mutableIsAutofillEnabledStateFlow.value
set(value) {
mutableIsAutofillEnabledStateFlow.value = value
}
override val isAutofillEnabledStateFlow: StateFlow<Boolean>
get() = mutableIsAutofillEnabledStateFlow.asStateFlow()
}

View file

@ -4,8 +4,8 @@ import android.view.autofill.AutofillManager
import com.x8bit.bitwarden.BuildConfig
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.platform.repository.model.BiometricsKeyResult
@ -21,13 +21,9 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import java.time.Instant
@ -40,7 +36,7 @@ private val DEFAULT_IS_SCREEN_CAPTURE_ALLOWED = BuildConfig.DEBUG
@Suppress("TooManyFunctions", "LongParameterList")
class SettingsRepositoryImpl(
private val autofillManager: AutofillManager,
private val appForegroundManager: AppForegroundManager,
private val autofillEnabledManager: AutofillEnabledManager,
private val authDiskSource: AuthDiskSource,
private val settingsDiskSource: SettingsDiskSource,
private val vaultSdkSource: VaultSdkSource,
@ -51,13 +47,6 @@ class SettingsRepositoryImpl(
private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined)
private val isAutofillEnabledAndSupported: Boolean
get() = autofillManager.isEnabled &&
autofillManager.hasEnabledAutofillServices() &&
autofillManager.isAutofillSupported
private val mutableIsAutofillEnabledStateFlow = MutableStateFlow(isAutofillEnabledAndSupported)
override var appLanguage: AppLanguage
get() = settingsDiskSource.appLanguage ?: AppLanguage.DEFAULT
set(value) {
@ -231,7 +220,7 @@ class SettingsRepositoryImpl(
)
}
override val isAutofillEnabledStateFlow: StateFlow<Boolean> =
mutableIsAutofillEnabledStateFlow.asStateFlow()
autofillEnabledManager.isAutofillEnabledStateFlow
override var isScreenCaptureAllowed: Boolean
get() = activeUserId?.let {
@ -266,16 +255,12 @@ class SettingsRepositoryImpl(
?: DEFAULT_IS_SCREEN_CAPTURE_ALLOWED,
)
init {
observeAutofillEnabledChanges()
}
override fun disableAutofill() {
autofillManager.disableAutofillServices()
// Manually indicate that autofill is no longer supported without needing a foreground state
// change.
mutableIsAutofillEnabledStateFlow.value = false
autofillEnabledManager.isAutofillEnabled = false
}
@Suppress("ReturnCount")
@ -435,21 +420,6 @@ class SettingsRepositoryImpl(
)
}
}
@OptIn(ExperimentalCoroutinesApi::class)
private fun observeAutofillEnabledChanges() {
mutableIsAutofillEnabledStateFlow
// Only observe when subscribed to.
.subscriptionCount.map { it > 0 }
.filter { hasSubscribers -> hasSubscribers }
.flatMapLatest {
appForegroundManager.appForegroundStateFlow
}
.onEach {
mutableIsAutofillEnabledStateFlow.value = isAutofillEnabledAndSupported
}
.launchIn(unconfinedScope)
}
}
/**

View file

@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.platform.repository.di
import android.view.autofill.AutofillManager
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.platform.datasource.disk.EnvironmentDiskSource
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager
@ -42,6 +43,7 @@ object PlatformRepositoryModule {
@Singleton
fun provideSettingsRepository(
autofillManager: AutofillManager,
autofillEnabledManager: AutofillEnabledManager,
appForegroundManager: AppForegroundManager,
authDiskSource: AuthDiskSource,
settingsDiskSource: SettingsDiskSource,
@ -51,7 +53,7 @@ object PlatformRepositoryModule {
): SettingsRepository =
SettingsRepositoryImpl(
autofillManager = autofillManager,
appForegroundManager = appForegroundManager,
autofillEnabledManager = autofillEnabledManager,
authDiskSource = authDiskSource,
settingsDiskSource = settingsDiskSource,
vaultSdkSource = vaultSdkSource,

View file

@ -0,0 +1,67 @@
package com.x8bit.bitwarden.data.autofill.manager
import android.view.autofill.AutofillManager
import app.cash.turbine.test
import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager
import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager
import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
class AutofillActivityManagerTest {
private val autofillManager: AutofillManager = mockk {
every { hasEnabledAutofillServices() } answers { isAutofillEnabledAndSupported }
every { isAutofillSupported } answers { isAutofillEnabledAndSupported }
every { isEnabled } answers { isAutofillEnabledAndSupported }
every { disableAutofillServices() } just runs
}
private val autofillEnabledManager: AutofillEnabledManager = AutofillEnabledManagerImpl()
private val mutableAppForegroundStateFlow = MutableStateFlow(AppForegroundState.BACKGROUNDED)
private val appForegroundManager: AppForegroundManager = mockk {
every { appForegroundStateFlow } returns mutableAppForegroundStateFlow
}
// We will construct an instance here just to hook the various dependencies together internally
private val autofillActivityManager: AutofillActivityManager = AutofillActivityManagerImpl(
autofillManager = autofillManager,
appForegroundManager = appForegroundManager,
autofillEnabledManager = autofillEnabledManager,
dispatcherManager = FakeDispatcherManager(),
)
private var isAutofillEnabledAndSupported = false
@Suppress("MaxLineLength")
@Test
fun `changes in app foreground status should update the AutofillEnabledManager as necessary`() =
runTest {
autofillEnabledManager.isAutofillEnabledStateFlow.test {
assertFalse(awaitItem())
// An update is received when both the autofill state and foreground state change
isAutofillEnabledAndSupported = true
mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
assertTrue(awaitItem())
// An update is not received when only the foreground state changes
mutableAppForegroundStateFlow.value = AppForegroundState.BACKGROUNDED
expectNoEvents()
// An update is not received when only the autofill state changes
isAutofillEnabledAndSupported = false
expectNoEvents()
// An update is received after both states have changed
mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
assertFalse(awaitItem())
}
}
}

View file

@ -0,0 +1,29 @@
package com.x8bit.bitwarden.data.autofill.manager
import app.cash.turbine.test
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
class AutofillEnabledManagerTest {
private val autofillEnabledManager: AutofillEnabledManager = AutofillEnabledManagerImpl()
@Suppress("MaxLineLength")
@Test
fun `isAutofillEnabledStateFlow should emit whenever isAutofillEnabled is set to a unique value`() =
runTest {
autofillEnabledManager.isAutofillEnabledStateFlow.test {
assertFalse(awaitItem())
autofillEnabledManager.isAutofillEnabled = true
assertTrue(awaitItem())
autofillEnabledManager.isAutofillEnabled = true
expectNoEvents()
autofillEnabledManager.isAutofillEnabled = false
assertFalse(awaitItem())
}
}
}

View file

@ -6,11 +6,11 @@ import com.bitwarden.core.DerivePinKeyResponse
import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson
import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource
import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManagerImpl
import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager
import com.x8bit.bitwarden.data.platform.datasource.disk.util.FakeSettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.AppForegroundManager
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState
import com.x8bit.bitwarden.data.platform.repository.model.BiometricsKeyResult
import com.x8bit.bitwarden.data.platform.repository.model.UriMatchType
import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeout
@ -27,7 +27,6 @@ import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
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.assertFalse
@ -39,25 +38,17 @@ import java.time.Instant
@Suppress("LargeClass")
class SettingsRepositoryTest {
private val autofillManager: AutofillManager = mockk {
every { hasEnabledAutofillServices() } answers { isAutofillEnabledAndSupported }
every { isAutofillSupported } answers { isAutofillEnabledAndSupported }
every { isEnabled } answers { isAutofillEnabledAndSupported }
every { disableAutofillServices() } just runs
}
private val mutableAppForegroundStateFlow = MutableStateFlow(AppForegroundState.BACKGROUNDED)
private val appForegroundManager: AppForegroundManager = mockk {
every { appForegroundStateFlow } returns mutableAppForegroundStateFlow
}
private val autofillEnabledManager: AutofillEnabledManager = AutofillEnabledManagerImpl()
private val fakeAuthDiskSource = FakeAuthDiskSource()
private val fakeSettingsDiskSource = FakeSettingsDiskSource()
private val vaultSdkSource: VaultSdkSource = mockk()
private val biometricsEncryptionManager: BiometricsEncryptionManager = mockk()
private var isAutofillEnabledAndSupported = false
private val settingsRepository = SettingsRepositoryImpl(
autofillManager = autofillManager,
appForegroundManager = appForegroundManager,
autofillEnabledManager = autofillEnabledManager,
authDiskSource = fakeAuthDiskSource,
settingsDiskSource = fakeSettingsDiskSource,
vaultSdkSource = vaultSdkSource,
@ -534,42 +525,15 @@ class SettingsRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `isAutofillEnabledStateFlow should emit updates if necessary when the app foreground state changes and disableAutofill is called`() =
fun `isAutofillEnabledStateFlow should emit whenever the AutofillEnabledManager does`() =
runTest {
// Updates are not received without an isAutofillEnabledStateFlow subscriber.
isAutofillEnabledAndSupported = true
mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
assertFalse(settingsRepository.isAutofillEnabledStateFlow.value)
// Revert back to initial state.
isAutofillEnabledAndSupported = false
mutableAppForegroundStateFlow.value = AppForegroundState.BACKGROUNDED
settingsRepository.isAutofillEnabledStateFlow.test {
assertFalse(awaitItem())
// An update is received when both the autofill state and foreground state change
isAutofillEnabledAndSupported = true
mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
autofillEnabledManager.isAutofillEnabled = true
assertTrue(awaitItem())
// An update is not received when only the foreground state changes
mutableAppForegroundStateFlow.value = AppForegroundState.BACKGROUNDED
expectNoEvents()
// An update is not received when only the autofill state changes
isAutofillEnabledAndSupported = false
expectNoEvents()
// An update is received after both states have changed
mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
assertFalse(awaitItem())
// Calling disableAutofill will result in an emission of false
isAutofillEnabledAndSupported = true
mutableAppForegroundStateFlow.value = AppForegroundState.BACKGROUNDED
assertTrue(awaitItem())
settingsRepository.disableAutofill()
autofillEnabledManager.isAutofillEnabled = false
assertFalse(awaitItem())
}
}
@ -579,8 +543,7 @@ class SettingsRepositoryTest {
fun `disableAutofill should trigger an emission of false from isAutofillEnabledStateFlow and disable autofill with the OS`() =
runTest {
// Start in a state where autofill is enabled
isAutofillEnabledAndSupported = true
mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED
autofillEnabledManager.isAutofillEnabled = true
settingsRepository.isAutofillEnabledStateFlow.test {
assertTrue(awaitItem())
expectNoEvents()
@ -589,6 +552,7 @@ class SettingsRepositoryTest {
settingsRepository.disableAutofill()
assertFalse(settingsRepository.isAutofillEnabledStateFlow.value)
assertFalse(autofillEnabledManager.isAutofillEnabled)
verify { autofillManager.disableAutofillServices() }
}