From 474ec4907f62fe67461cd073e1233c1b76006239 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 14 Aug 2025 13:21:24 -0500 Subject: [PATCH] PM-24726: Update MDM functionality (#5694) --- .../x8bit/bitwarden/BitwardenApplication.kt | 12 ++ .../manager/di/PlatformManagerModule.kt | 5 - .../manager/restriction/RestrictionManager.kt | 7 +- .../restriction/RestrictionManagerImpl.kt | 105 ++--------------- .../repository/EnvironmentRepository.kt | 5 + .../repository/EnvironmentRepositoryImpl.kt | 14 +-- .../restriction/RestrictionManagerTest.kt | 107 +++++------------- .../repository/EnvironmentRepositoryTest.kt | 6 +- .../util/FakeEnvironmentRepository.kt | 3 + 9 files changed, 75 insertions(+), 189 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/BitwardenApplication.kt b/app/src/main/kotlin/com/x8bit/bitwarden/BitwardenApplication.kt index 7f1b71a38e..5e03b3869d 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/BitwardenApplication.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/BitwardenApplication.kt @@ -8,6 +8,7 @@ import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager import com.x8bit.bitwarden.data.platform.manager.network.NetworkConfigManager import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager import com.x8bit.bitwarden.data.platform.manager.restriction.RestrictionManager +import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import dagger.hilt.android.HiltAndroidApp import timber.log.Timber import javax.inject.Inject @@ -38,6 +39,17 @@ class BitwardenApplication : Application() { @Inject lateinit var restrictionManager: RestrictionManager + @Inject + lateinit var environmentRepository: EnvironmentRepository + + override fun onCreate() { + super.onCreate() + // These must be initialized in order to ensure that the restrictionManager does not + // override the environmentRepository values. + restrictionManager.initialize() + environmentRepository.initialize() + } + override fun onLowMemory() { super.onLowMemory() Timber.w("onLowMemory") diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt index ddfff45e9c..d960600ce0 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt @@ -335,13 +335,8 @@ object PlatformManagerModule { @Singleton fun provideRestrictionManager( @ApplicationContext context: Context, - appStateManager: AppStateManager, - dispatcherManager: DispatcherManager, environmentRepository: EnvironmentRepository, ): RestrictionManager = RestrictionManagerImpl( - appStateManager = appStateManager, - dispatcherManager = dispatcherManager, - context = context, environmentRepository = environmentRepository, restrictionsManager = requireNotNull(context.getSystemService()), ) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManager.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManager.kt index ff687d03ac..84e7f80262 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManager.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManager.kt @@ -3,4 +3,9 @@ package com.x8bit.bitwarden.data.platform.manager.restriction /** * A manager for handling restrictions. */ -interface RestrictionManager +interface RestrictionManager { + /** + * Initializes the [RestrictionManager]. + */ + fun initialize() +} diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerImpl.kt index 7f688c55f9..4631523ef3 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerImpl.kt @@ -1,57 +1,19 @@ package com.x8bit.bitwarden.data.platform.manager.restriction -import android.content.BroadcastReceiver -import android.content.Context -import android.content.Intent -import android.content.IntentFilter import android.content.RestrictionsManager -import android.os.Bundle -import com.bitwarden.data.manager.DispatcherManager +import com.bitwarden.data.datasource.disk.model.EnvironmentUrlDataJson import com.bitwarden.data.repository.model.Environment -import com.x8bit.bitwarden.data.platform.manager.AppStateManager -import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach /** * The default implementation of the [RestrictionManager]. */ class RestrictionManagerImpl( - appStateManager: AppStateManager, - dispatcherManager: DispatcherManager, - private val context: Context, private val environmentRepository: EnvironmentRepository, private val restrictionsManager: RestrictionsManager, ) : RestrictionManager { - private val mainScope = CoroutineScope(dispatcherManager.main) - private val intentFilter = IntentFilter(Intent.ACTION_APPLICATION_RESTRICTIONS_CHANGED) - private val restrictionsChangedReceiver = RestrictionsChangedReceiver() - private var isReceiverRegistered = false - init { - appStateManager - .appForegroundStateFlow - .onEach { - when (it) { - AppForegroundState.BACKGROUNDED -> handleBackground() - AppForegroundState.FOREGROUNDED -> handleForeground() - } - } - .launchIn(mainScope) - } - - private fun handleBackground() { - if (isReceiverRegistered) { - context.unregisterReceiver(restrictionsChangedReceiver) - } - isReceiverRegistered = false - } - - private fun handleForeground() { - context.registerReceiver(restrictionsChangedReceiver, intentFilter) - isReceiverRegistered = true + override fun initialize() { updatePreconfiguredRestrictionSettings() } @@ -59,65 +21,22 @@ class RestrictionManagerImpl( restrictionsManager .applicationRestrictions ?.takeUnless { it.isEmpty } - ?.let { setPreconfiguredSettings(it) } - } - - private fun setPreconfiguredSettings(bundle: Bundle) { - bundle - .getString(BASE_ENVIRONMENT_URL_RESTRICTION_KEY) + ?.getString(BASE_ENVIRONMENT_URL_RESTRICTION_KEY) ?.let { url -> setPreconfiguredUrl(baseEnvironmentUrl = url) } } private fun setPreconfiguredUrl(baseEnvironmentUrl: String) { - environmentRepository.environment = when (val current = environmentRepository.environment) { - Environment.Us -> { - when (baseEnvironmentUrl) { - // If the base matches the predefined US environment, leave it alone - Environment.Us.environmentUrlData.base -> current - // If the base does not match the predefined US environment, create a - // self-hosted environment with the new base - else -> current.toSelfHosted(base = baseEnvironmentUrl) - } - } - - Environment.Eu -> { - when (baseEnvironmentUrl) { - // If the base matches the predefined EU environment, leave it alone - Environment.Eu.environmentUrlData.base -> current - // If the base does not match the predefined EU environment, create a - // self-hosted environment with the new base - else -> current.toSelfHosted(base = baseEnvironmentUrl) - } - } - - is Environment.SelfHosted -> current.toSelfHosted(base = baseEnvironmentUrl) - } - } - - /** - * A [BroadcastReceiver] used to listen for [Intent.ACTION_APPLICATION_RESTRICTIONS_CHANGED] - * updates. - * - * Note: The `Intent.ACTION_APPLICATION_RESTRICTIONS_CHANGED` will only be received if the - * `BroadcastReceiver` is dynamically registered, so this cannot be registered in the manifest. - */ - private inner class RestrictionsChangedReceiver : BroadcastReceiver() { - override fun onReceive(context: Context, intent: Intent) { - if (intent.action == Intent.ACTION_APPLICATION_RESTRICTIONS_CHANGED) { - updatePreconfiguredRestrictionSettings() - } + environmentRepository.environment = when (baseEnvironmentUrl) { + // If the baseEnvironmentUrl matches the predefined US environment, assume it is the + // default US environment. + Environment.Us.environmentUrlData.base -> Environment.Us + // If the baseEnvironmentUrl matches the predefined EU environment, assume it is the + // default EU environment. + Environment.Eu.environmentUrlData.base -> Environment.Eu + // Otherwise make a custom self-host environment. + else -> Environment.SelfHosted(EnvironmentUrlDataJson(baseEnvironmentUrl)) } } } private const val BASE_ENVIRONMENT_URL_RESTRICTION_KEY: String = "baseEnvironmentUrl" - -/** - * Helper method for creating a new [Environment.SelfHosted] with a new base. - */ -private fun Environment.toSelfHosted( - base: String, -): Environment.SelfHosted = - Environment.SelfHosted( - environmentUrlData = environmentUrlData.copy(base = base), - ) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepository.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepository.kt index 98115d77b4..cf14f9c224 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepository.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepository.kt @@ -17,6 +17,11 @@ interface EnvironmentRepository { */ val environmentStateFlow: StateFlow + /** + * Initializes the [EnvironmentRepository]. + */ + fun initialize() + /** * Stores the current environment for the given [userEmail]. */ diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryImpl.kt index 21d3fba12f..aa99221810 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryImpl.kt @@ -11,6 +11,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import timber.log.Timber @@ -20,7 +21,7 @@ import timber.log.Timber */ class EnvironmentRepositoryImpl( private val environmentDiskSource: EnvironmentDiskSource, - authDiskSource: AuthDiskSource, + private val authDiskSource: AuthDiskSource, dispatcherManager: DispatcherManager, ) : EnvironmentRepository { @@ -44,16 +45,13 @@ class EnvironmentRepositoryImpl( initialValue = environment, ) - init { + override fun initialize() { authDiskSource .userStateFlow - .onEach { userState -> + .mapNotNull { userState -> userState?.activeAccount?.settings?.environmentUrlData } + .onEach { environmentUrlDataJson -> // If the active account has environment data, set that as the current value. - userState - ?.activeAccount - ?.settings - ?.environmentUrlData - ?.let { environmentDiskSource.preAuthEnvironmentUrlData = it } + environmentDiskSource.preAuthEnvironmentUrlData = environmentUrlDataJson } .launchIn(scope) } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerTest.kt index 2ab3cbd0f3..ec24401a6a 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/restriction/RestrictionManagerTest.kt @@ -1,118 +1,91 @@ package com.x8bit.bitwarden.data.platform.manager.restriction -import android.annotation.SuppressLint -import android.content.Context import android.content.RestrictionsManager import android.os.Bundle -import com.bitwarden.data.datasource.disk.base.FakeDispatcherManager import com.bitwarden.data.datasource.disk.model.EnvironmentUrlDataJson import com.bitwarden.data.repository.model.Environment -import com.x8bit.bitwarden.data.platform.manager.model.AppForegroundState -import com.x8bit.bitwarden.data.platform.manager.util.FakeAppStateManager import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository -import io.mockk.clearMocks import io.mockk.every -import io.mockk.just import io.mockk.mockk -import io.mockk.runs import io.mockk.verify -import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test -@SuppressLint("UnspecifiedRegisterReceiverFlag") class RestrictionManagerTest { - private val context = mockk { - every { registerReceiver(any(), any()) } returns null - every { unregisterReceiver(any()) } just runs - } - private val fakeAppStateManager = FakeAppStateManager() - private val fakeDispatcherManager = FakeDispatcherManager().apply { - setMain(unconfined) - } private val fakeEnvironmentRepository = FakeEnvironmentRepository() private val restrictionsManager = mockk() private val restrictionManager: RestrictionManager = RestrictionManagerImpl( - appStateManager = fakeAppStateManager, - dispatcherManager = fakeDispatcherManager, - context = context, environmentRepository = fakeEnvironmentRepository, restrictionsManager = restrictionsManager, ) - @AfterEach - fun tearDown() { - fakeDispatcherManager.resetMain() - } - @Test - fun `on app foreground with a null bundle should register receiver and do nothing else`() { + fun `initialize with a null bundle should do nothing`() { every { restrictionsManager.applicationRestrictions } returns null - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals(Environment.Us, fakeEnvironmentRepository.environment) } @Test - fun `on app foreground with an empty bundle should register receiver and do nothing else`() { + fun `initialize with an empty bundle should do nothing`() { every { restrictionsManager.applicationRestrictions } returns mockBundle() - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals(Environment.Us, fakeEnvironmentRepository.environment) } - @Suppress("MaxLineLength") @Test - fun `on app foreground with unknown bundle data should register receiver and do nothing else`() { + fun `initialize with unknown bundle data should do nothing`() { every { restrictionsManager.applicationRestrictions } returns mockBundle("key" to "unknown") - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals(Environment.Us, fakeEnvironmentRepository.environment) } @Suppress("MaxLineLength") @Test - fun `on app foreground with baseEnvironmentUrl bundle data matching the current US environment should register receiver and set the environment to US`() { + fun `initialize with baseEnvironmentUrl bundle data matching the current US environment should set the environment to US`() { every { restrictionsManager.applicationRestrictions } returns mockBundle("baseEnvironmentUrl" to "https://vault.bitwarden.com") - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals(Environment.Us, fakeEnvironmentRepository.environment) } @Suppress("MaxLineLength") @Test - fun `on app foreground with baseEnvironmentUrl bundle data not matching the current US environment should register receiver and set the environment to self-hosted`() { + fun `initialize with baseEnvironmentUrl bundle data not matching the current US environment should set the environment to self-hosted`() { val baseUrl = "https://other.bitwarden.com" every { restrictionsManager.applicationRestrictions } returns mockBundle("baseEnvironmentUrl" to baseUrl) - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals( Environment.SelfHosted( @@ -124,33 +97,33 @@ class RestrictionManagerTest { @Suppress("MaxLineLength") @Test - fun `on app foreground with baseEnvironmentUrl bundle data matching the current EU environment should register receiver and set the environment to EU`() { + fun `initialize with baseEnvironmentUrl bundle data matching the current EU environment should set the environment to EU`() { fakeEnvironmentRepository.environment = Environment.Eu every { restrictionsManager.applicationRestrictions } returns mockBundle("baseEnvironmentUrl" to "https://vault.bitwarden.eu") - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals(Environment.Eu, fakeEnvironmentRepository.environment) } @Suppress("MaxLineLength") @Test - fun `on app foreground with baseEnvironmentUrl bundle data not matching the current EU environment should register receiver and set the environment to self-hosted`() { + fun `initialize with baseEnvironmentUrl bundle data not matching the current EU environment should set the environment to self-hosted`() { val baseUrl = "https://other.bitwarden.eu" fakeEnvironmentRepository.environment = Environment.Eu every { restrictionsManager.applicationRestrictions } returns mockBundle("baseEnvironmentUrl" to baseUrl) - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals( Environment.SelfHosted( @@ -162,7 +135,7 @@ class RestrictionManagerTest { @Suppress("MaxLineLength") @Test - fun `on app foreground with baseEnvironmentUrl bundle data matching the current self-hosted environment should register receiver and set the environment to self-hosted`() { + fun `initialize with baseEnvironmentUrl bundle data matching the current self-hosted environment should set the environment to self-hosted`() { val baseUrl = "https://vault.qa.bitwarden.pw" val environment = Environment.SelfHosted( environmentUrlData = EnvironmentUrlDataJson(base = baseUrl), @@ -172,17 +145,17 @@ class RestrictionManagerTest { restrictionsManager.applicationRestrictions } returns mockBundle("baseEnvironmentUrl" to baseUrl) - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals(environment, fakeEnvironmentRepository.environment) } @Suppress("MaxLineLength") @Test - fun `on app foreground with baseEnvironmentUrl bundle data not matching the current self-hosted environment should register receiver and set the environment to self-hosted`() { + fun `initialize with baseEnvironmentUrl bundle data not matching the current self-hosted environment should set the environment to self-hosted`() { val baseUrl = "https://other.qa.bitwarden.pw" val environment = Environment.SelfHosted( environmentUrlData = EnvironmentUrlDataJson(base = "https://vault.qa.bitwarden.pw"), @@ -192,10 +165,10 @@ class RestrictionManagerTest { restrictionsManager.applicationRestrictions } returns mockBundle("baseEnvironmentUrl" to baseUrl) - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED + restrictionManager.initialize() verify(exactly = 1) { - context.registerReceiver(any(), any()) + restrictionsManager.applicationRestrictions } assertEquals( Environment.SelfHosted( @@ -204,32 +177,6 @@ class RestrictionManagerTest { fakeEnvironmentRepository.environment, ) } - - @Test - fun `on app background when not foregrounded should do nothing`() { - fakeAppStateManager.appForegroundState = AppForegroundState.BACKGROUNDED - - verify(exactly = 0) { - context.unregisterReceiver(any()) - restrictionsManager.applicationRestrictions - } - } - - @Test - fun `on app background after foreground should unregister receiver`() { - every { restrictionsManager.applicationRestrictions } returns null - fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED - clearMocks(context, restrictionsManager, answers = false) - - fakeAppStateManager.appForegroundState = AppForegroundState.BACKGROUNDED - - verify(exactly = 1) { - context.unregisterReceiver(any()) - } - verify(exactly = 0) { - restrictionsManager.applicationRestrictions - } - } } /** diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryTest.kt index 9cb0cfbc7e..5d35bacbd8 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryTest.kt @@ -29,7 +29,7 @@ class EnvironmentRepositoryTest { private val fakeEnvironmentDiskSource = FakeEnvironmentDiskSource() private val fakeAuthDiskSource = FakeAuthDiskSource() - private val repository = EnvironmentRepositoryImpl( + private val repository: EnvironmentRepository = EnvironmentRepositoryImpl( environmentDiskSource = fakeEnvironmentDiskSource, authDiskSource = fakeAuthDiskSource, dispatcherManager = dispatcherManager, @@ -46,7 +46,7 @@ class EnvironmentRepositoryTest { } @Test - fun `changes to the active user should update the environment if necessary`() { + fun `after initialize changes to the active user should update the environment if necessary`() { assertEquals( Environment.Us, repository.environment, @@ -56,6 +56,8 @@ class EnvironmentRepositoryTest { fakeEnvironmentDiskSource.preAuthEnvironmentUrlData, ) + repository.initialize() + // Updating the environment for the active user to a non-null value triggers an update // in the saved environment. fakeAuthDiskSource.userState = getMockUserState( diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/util/FakeEnvironmentRepository.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/util/FakeEnvironmentRepository.kt index e50dc40813..6cd3e6367f 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/util/FakeEnvironmentRepository.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/util/FakeEnvironmentRepository.kt @@ -10,6 +10,9 @@ import kotlinx.coroutines.flow.asStateFlow * A faked implementation of [EnvironmentRepository] based on in-memory caching. */ class FakeEnvironmentRepository : EnvironmentRepository { + + override fun initialize() = Unit + override var environment: Environment get() = mutableEnvironmentStateFlow.value set(value) {