From 30eb11b85ebe7dd1832621da0fcb9616dccebdee Mon Sep 17 00:00:00 2001 From: David Perez Date: Fri, 15 Nov 2024 16:15:19 -0600 Subject: [PATCH] PM-14409: Add realtime check for when the accessibility service is enabled or disabled (#4314) --- .../java/com/x8bit/bitwarden/MainActivity.kt | 4 - .../accessibility/di/AccessibilityModule.kt | 15 +++- .../di/ActivityAccessibilityModule.kt | 36 --------- .../manager/AccessibilityActivityManager.kt | 10 --- .../AccessibilityActivityManagerImpl.kt | 28 ------- .../manager/AccessibilityEnabledManager.kt | 10 +-- .../AccessibilityEnabledManagerImpl.kt | 17 ++-- .../AccessibilityActivityManagerTest.kt | 81 ------------------- .../AccessibilityEnabledManagerTest.kt | 24 ++++-- .../FakeAccessibilityEnabledManager.kt | 19 +++++ .../repository/SettingsRepositoryTest.kt | 12 ++- 11 files changed, 68 insertions(+), 188 deletions(-) delete mode 100644 app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/ActivityAccessibilityModule.kt delete mode 100644 app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManager.kt delete mode 100644 app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerImpl.kt delete mode 100644 app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerTest.kt create mode 100644 app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/FakeAccessibilityEnabledManager.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt b/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt index 50617c2ff..175487965 100644 --- a/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt +++ b/app/src/main/java/com/x8bit/bitwarden/MainActivity.kt @@ -15,7 +15,6 @@ import androidx.core.os.LocaleListCompat import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.compose.rememberNavController -import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityActivityManager import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityCompletionManager import com.x8bit.bitwarden.data.autofill.manager.AutofillActivityManager import com.x8bit.bitwarden.data.autofill.manager.AutofillCompletionManager @@ -39,9 +38,6 @@ class MainActivity : AppCompatActivity() { private val mainViewModel: MainViewModel by viewModels() - @Inject - lateinit var accessibilityActivityManager: AccessibilityActivityManager - @Inject lateinit var autofillActivityManager: AutofillActivityManager diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/AccessibilityModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/AccessibilityModule.kt index b33b229fd..ceed9f19f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/AccessibilityModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/AccessibilityModule.kt @@ -3,6 +3,7 @@ package com.x8bit.bitwarden.data.autofill.accessibility.di import android.content.Context import android.content.pm.PackageManager import android.os.PowerManager +import android.view.accessibility.AccessibilityManager import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityAutofillManager import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityAutofillManagerImpl import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityCompletionManager @@ -55,8 +56,12 @@ object AccessibilityModule { @Singleton @Provides - fun providesAccessibilityEnabledManager(): AccessibilityEnabledManager = - AccessibilityEnabledManagerImpl() + fun providesAccessibilityEnabledManager( + accessibilityManager: AccessibilityManager, + ): AccessibilityEnabledManager = + AccessibilityEnabledManagerImpl( + accessibilityManager = accessibilityManager, + ) @Singleton @Provides @@ -110,6 +115,12 @@ object AccessibilityModule { @ApplicationContext context: Context, ): PackageManager = context.packageManager + @Singleton + @Provides + fun provideAccessibilityManager( + @ApplicationContext context: Context, + ): AccessibilityManager = context.getSystemService(AccessibilityManager::class.java) + @Singleton @Provides fun providesPowerManager( diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/ActivityAccessibilityModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/ActivityAccessibilityModule.kt deleted file mode 100644 index 636a6d15d..000000000 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/di/ActivityAccessibilityModule.kt +++ /dev/null @@ -1,36 +0,0 @@ -package com.x8bit.bitwarden.data.autofill.accessibility.di - -import android.content.Context -import androidx.lifecycle.LifecycleCoroutineScope -import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityActivityManager -import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityActivityManagerImpl -import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityEnabledManager -import com.x8bit.bitwarden.data.platform.manager.AppStateManager -import dagger.Module -import dagger.Provides -import dagger.hilt.InstallIn -import dagger.hilt.android.components.ActivityComponent -import dagger.hilt.android.qualifiers.ApplicationContext -import dagger.hilt.android.scopes.ActivityScoped - -/** - * Provides dependencies within the accessibility package scoped to the activity. - */ -@Module -@InstallIn(ActivityComponent::class) -object ActivityAccessibilityModule { - @ActivityScoped - @Provides - fun providesAccessibilityActivityManager( - @ApplicationContext context: Context, - accessibilityEnabledManager: AccessibilityEnabledManager, - appStateManager: AppStateManager, - lifecycleScope: LifecycleCoroutineScope, - ): AccessibilityActivityManager = - AccessibilityActivityManagerImpl( - context = context, - accessibilityEnabledManager = accessibilityEnabledManager, - appStateManager = appStateManager, - lifecycleScope = lifecycleScope, - ) -} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManager.kt deleted file mode 100644 index 191382544..000000000 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManager.kt +++ /dev/null @@ -1,10 +0,0 @@ -package com.x8bit.bitwarden.data.autofill.accessibility.manager - -import android.app.Activity - -/** - * A helper for dealing with accessibility configuration that must be scoped to a specific - * [Activity]. In particular, this should be injected into an [Activity] to ensure that the - * [AccessibilityEnabledManager] reports correct values. - */ -interface AccessibilityActivityManager diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerImpl.kt deleted file mode 100644 index 0777298b6..000000000 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerImpl.kt +++ /dev/null @@ -1,28 +0,0 @@ -package com.x8bit.bitwarden.data.autofill.accessibility.manager - -import android.content.Context -import androidx.lifecycle.LifecycleCoroutineScope -import com.x8bit.bitwarden.data.autofill.accessibility.util.isAccessibilityServiceEnabled -import com.x8bit.bitwarden.data.platform.manager.AppStateManager -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach - -/** - * The default implementation of the [AccessibilityActivityManager]. - */ -class AccessibilityActivityManagerImpl( - private val context: Context, - private val accessibilityEnabledManager: AccessibilityEnabledManager, - appStateManager: AppStateManager, - lifecycleScope: LifecycleCoroutineScope, -) : AccessibilityActivityManager { - init { - appStateManager - .appForegroundStateFlow - .onEach { - accessibilityEnabledManager.isAccessibilityEnabled = - context.isAccessibilityServiceEnabled - } - .launchIn(lifecycleScope) - } -} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManager.kt index 4ca327247..5c498c347 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManager.kt @@ -7,15 +7,7 @@ import kotlinx.coroutines.flow.StateFlow */ interface AccessibilityEnabledManager { /** - * Whether or not the accessibility service 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 isAccessibilityEnabled: Boolean - - /** - * Emits updates that track [isAccessibilityEnabled] values. + * Emits updates that track whether the accessibility autofill service is enabled.. */ val isAccessibilityEnabledStateFlow: StateFlow } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerImpl.kt index c3f434521..c9e53d0bf 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerImpl.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.data.autofill.accessibility.manager +import android.view.accessibility.AccessibilityManager import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -7,14 +8,18 @@ import kotlinx.coroutines.flow.asStateFlow /** * The default implementation of [AccessibilityEnabledManager]. */ -class AccessibilityEnabledManagerImpl : AccessibilityEnabledManager { +class AccessibilityEnabledManagerImpl( + accessibilityManager: AccessibilityManager, +) : AccessibilityEnabledManager { private val mutableIsAccessibilityEnabledStateFlow = MutableStateFlow(value = false) - override var isAccessibilityEnabled: Boolean - get() = mutableIsAccessibilityEnabledStateFlow.value - set(value) { - mutableIsAccessibilityEnabledStateFlow.value = value - } + init { + accessibilityManager.addAccessibilityStateChangeListener( + AccessibilityManager.AccessibilityStateChangeListener { isEnabled -> + mutableIsAccessibilityEnabledStateFlow.value = isEnabled + }, + ) + } override val isAccessibilityEnabledStateFlow: StateFlow get() = mutableIsAccessibilityEnabledStateFlow.asStateFlow() diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerTest.kt deleted file mode 100644 index d8c5e63bd..000000000 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityActivityManagerTest.kt +++ /dev/null @@ -1,81 +0,0 @@ -package com.x8bit.bitwarden.data.autofill.accessibility.manager - -import android.content.Context -import androidx.lifecycle.LifecycleCoroutineScope -import app.cash.turbine.test -import com.x8bit.bitwarden.data.autofill.accessibility.util.isAccessibilityServiceEnabled -import com.x8bit.bitwarden.data.platform.manager.AppStateManager -import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState -import io.mockk.every -import io.mockk.mockk -import io.mockk.mockkStatic -import io.mockk.unmockkStatic -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.test.UnconfinedTestDispatcher -import kotlinx.coroutines.test.runTest -import org.junit.jupiter.api.AfterEach -import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertTrue -import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.Test - -@OptIn(ExperimentalCoroutinesApi::class) -class AccessibilityActivityManagerTest { - private val context: Context = mockk() - private val accessibilityEnabledManager: AccessibilityEnabledManager = - AccessibilityEnabledManagerImpl() - private val mutableAppForegroundStateFlow = MutableStateFlow(AppForegroundState.BACKGROUNDED) - private val appStateManager: AppStateManager = mockk { - every { appForegroundStateFlow } returns mutableAppForegroundStateFlow - } - private val lifecycleScope = mockk { - every { coroutineContext } returns UnconfinedTestDispatcher() - } - - // We will construct an instance here just to hook the various dependencies together internally - private lateinit var autofillActivityManager: AccessibilityActivityManager - - @BeforeEach - fun setup() { - mockkStatic(Context::isAccessibilityServiceEnabled) - every { context.isAccessibilityServiceEnabled } returns false - autofillActivityManager = AccessibilityActivityManagerImpl( - context = context, - accessibilityEnabledManager = accessibilityEnabledManager, - appStateManager = appStateManager, - lifecycleScope = lifecycleScope, - ) - } - - @AfterEach - fun tearDown() { - unmockkStatic(Context::isAccessibilityServiceEnabled) - } - - @Test - fun `changes in app foreground status should update the AutofillEnabledManager as necessary`() = - runTest { - accessibilityEnabledManager.isAccessibilityEnabledStateFlow.test { - assertFalse(awaitItem()) - - // An update is received when both the accessibility state and foreground state - // change - every { context.isAccessibilityServiceEnabled } returns 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 accessibility state changes - every { context.isAccessibilityServiceEnabled } returns false - expectNoEvents() - - // An update is received after both states have changed - mutableAppForegroundStateFlow.value = AppForegroundState.FOREGROUNDED - assertFalse(awaitItem()) - } - } -} diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerTest.kt index 716b27f0e..77a7ab45d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityEnabledManagerTest.kt @@ -1,6 +1,10 @@ package com.x8bit.bitwarden.data.autofill.accessibility.manager +import android.view.accessibility.AccessibilityManager import app.cash.turbine.test +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue @@ -8,23 +12,33 @@ import org.junit.jupiter.api.Test class AccessibilityEnabledManagerTest { + private val accessibilityStateChangeListener = + slot() + private val accessibilityManager = mockk { + every { + addAccessibilityStateChangeListener(capture(accessibilityStateChangeListener)) + } returns true + } + private val accessibilityEnabledManager: AccessibilityEnabledManager = - AccessibilityEnabledManagerImpl() + AccessibilityEnabledManagerImpl( + accessibilityManager = accessibilityManager, + ) @Suppress("MaxLineLength") @Test - fun `isAccessibilityEnabledStateFlow should emit whenever isAccessibilityEnabled is set to a unique value`() = + fun `isAccessibilityEnabledStateFlow should emit whenever accessibilityStateChangeListener emits a unique value`() = runTest { accessibilityEnabledManager.isAccessibilityEnabledStateFlow.test { assertFalse(awaitItem()) - accessibilityEnabledManager.isAccessibilityEnabled = true + accessibilityStateChangeListener.captured.onAccessibilityStateChanged(true) assertTrue(awaitItem()) - accessibilityEnabledManager.isAccessibilityEnabled = true + accessibilityStateChangeListener.captured.onAccessibilityStateChanged(true) expectNoEvents() - accessibilityEnabledManager.isAccessibilityEnabled = false + accessibilityStateChangeListener.captured.onAccessibilityStateChanged(false) assertFalse(awaitItem()) } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/FakeAccessibilityEnabledManager.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/FakeAccessibilityEnabledManager.kt new file mode 100644 index 000000000..d35d1dab1 --- /dev/null +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/FakeAccessibilityEnabledManager.kt @@ -0,0 +1,19 @@ +package com.x8bit.bitwarden.data.autofill.accessibility.manager + +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow + +class FakeAccessibilityEnabledManager : AccessibilityEnabledManager { + + private val mutableIsAccessibilityEnabledStateFlow = MutableStateFlow(value = false) + + override val isAccessibilityEnabledStateFlow: StateFlow + get() = mutableIsAccessibilityEnabledStateFlow.asStateFlow() + + var isAccessibilityEnabled: Boolean + get() = mutableIsAccessibilityEnabledStateFlow.value + set(value) { + mutableIsAccessibilityEnabledStateFlow.value = value + } +} diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt index 7674bd061..c5eb5b885 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt @@ -12,8 +12,7 @@ import com.x8bit.bitwarden.data.auth.datasource.network.model.KdfTypeJson import com.x8bit.bitwarden.data.auth.datasource.network.model.TrustedDeviceUserDecryptionOptionsJson import com.x8bit.bitwarden.data.auth.datasource.network.model.UserDecryptionOptionsJson import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult -import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityEnabledManager -import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityEnabledManagerImpl +import com.x8bit.bitwarden.data.autofill.accessibility.manager.FakeAccessibilityEnabledManager import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManagerImpl import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager @@ -55,8 +54,7 @@ class SettingsRepositoryTest { every { disableAutofillServices() } just runs } private val autofillEnabledManager: AutofillEnabledManager = AutofillEnabledManagerImpl() - private val accessibilityEnabledManager: AccessibilityEnabledManager = - AccessibilityEnabledManagerImpl() + private val fakeAccessibilityEnabledManager = FakeAccessibilityEnabledManager() private val fakeAuthDiskSource = FakeAuthDiskSource() private val fakeSettingsDiskSource = FakeSettingsDiskSource() private val vaultSdkSource: VaultSdkSource = mockk() @@ -75,7 +73,7 @@ class SettingsRepositoryTest { settingsDiskSource = fakeSettingsDiskSource, vaultSdkSource = vaultSdkSource, biometricsEncryptionManager = biometricsEncryptionManager, - accessibilityEnabledManager = accessibilityEnabledManager, + accessibilityEnabledManager = fakeAccessibilityEnabledManager, dispatcherManager = FakeDispatcherManager(), policyManager = policyManager, ) @@ -691,10 +689,10 @@ class SettingsRepositoryTest { settingsRepository.isAccessibilityEnabledStateFlow.test { assertFalse(awaitItem()) - accessibilityEnabledManager.isAccessibilityEnabled = true + fakeAccessibilityEnabledManager.isAccessibilityEnabled = true assertTrue(awaitItem()) - accessibilityEnabledManager.isAccessibilityEnabled = false + fakeAccessibilityEnabledManager.isAccessibilityEnabled = false assertFalse(awaitItem()) } }