PM-15110: Ensure all network requests always use the current environment data (#4344)

app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/BaseUrlInterceptor.kt

@@ -6,37 +6,27 @@ import okhttp3.Interceptor
 import okhttp3.Response
 
 /**
- * A [Interceptor] that optionally takes the current base URL of a request and replaces it with
+ * An [Interceptor] that optionally takes the current base URL of a request and replaces it with
- * the currently set [baseUrl]
+ * the currently set base URL from the [baseUrlProvider].
  */
-class BaseUrlInterceptor : Interceptor {
+class BaseUrlInterceptor(
+    private val baseUrlProvider: () -> String?,
+) : Interceptor {
 
-    /**
+    private val baseHttpUrl: HttpUrl?
-     * The base URL to use as an override, or `null` if no override should be performed.
+        get() = baseUrlProvider()?.let { requireNotNull(it.toHttpUrlOrNull()) }
-     */
-    var baseUrl: String? = null
-        set(value) {
-            field = value
-            baseHttpUrl = baseUrl?.let { requireNotNull(it.toHttpUrlOrNull()) }
-        }
-
-    private var baseHttpUrl: HttpUrl? = null
 
     override fun intercept(chain: Interceptor.Chain): Response {
         val request = chain.request()
 
         // If no base URL is set, we can simply skip
-        val base = baseHttpUrl ?: return chain.proceed(request)
+        val base = baseHttpUrl ?: return chain.proceed(request = request)
 
         // Update the base URL used.
         return chain.proceed(
-            request
+            request = request
                 .newBuilder()
-                .url(
+                .url(url = request.url.replaceBaseUrlWith(baseUrl = base))
-                    request
-                        .url
-                        .replaceBaseUrlWith(base),
-                )
                 .build(),
         )
     }

app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/BaseUrlInterceptors.kt

@@ -1,47 +1,44 @@
 package com.x8bit.bitwarden.data.platform.datasource.network.interceptor
 
+import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage
+import com.x8bit.bitwarden.data.platform.datasource.disk.EnvironmentDiskSource
 import com.x8bit.bitwarden.data.platform.repository.model.Environment
 import com.x8bit.bitwarden.data.platform.repository.util.baseApiUrl
 import com.x8bit.bitwarden.data.platform.repository.util.baseEventsUrl
 import com.x8bit.bitwarden.data.platform.repository.util.baseIdentityUrl
+import com.x8bit.bitwarden.data.platform.repository.util.toEnvironmentUrlsOrDefault
 import javax.inject.Inject
 import javax.inject.Singleton
 
 /**
  * An overall container for various [BaseUrlInterceptor] implementations for different API groups.
  */
+@OmitFromCoverage
 @Singleton
-class BaseUrlInterceptors @Inject constructor() {
+class BaseUrlInterceptors @Inject constructor(
-    var environment: Environment = Environment.Us
+    private val environmentDiskSource: EnvironmentDiskSource,
-        set(value) {
+) {
-            field = value
+    private val environment: Environment
-            updateBaseUrls(environment = value)
+        get() = environmentDiskSource.preAuthEnvironmentUrlData.toEnvironmentUrlsOrDefault()
-        }
 
     /**
      * An interceptor for "/api" calls.
      */
-    val apiInterceptor: BaseUrlInterceptor = BaseUrlInterceptor()
+    val apiInterceptor: BaseUrlInterceptor = BaseUrlInterceptor {
+        environment.environmentUrlData.baseApiUrl
+    }
 
     /**
      * An interceptor for "/identity" calls.
      */
-    val identityInterceptor: BaseUrlInterceptor = BaseUrlInterceptor()
+    val identityInterceptor: BaseUrlInterceptor = BaseUrlInterceptor {
+        environment.environmentUrlData.baseIdentityUrl
+    }
 
     /**
      * An interceptor for "/events" calls.
      */
-    val eventsInterceptor: BaseUrlInterceptor = BaseUrlInterceptor()
+    val eventsInterceptor: BaseUrlInterceptor = BaseUrlInterceptor {
-
+        environment.environmentUrlData.baseEventsUrl
-    init {
-        // Ensure all interceptors begin with a default value
-        environment = Environment.Us
-    }
-
-    private fun updateBaseUrls(environment: Environment) {
-        val environmentUrlData = environment.environmentUrlData
-        apiInterceptor.baseUrl = environmentUrlData.baseApiUrl
-        identityInterceptor.baseUrl = environmentUrlData.baseIdentityUrl
-        eventsInterceptor.baseUrl = environmentUrlData.baseEventsUrl
     }
 }

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

@@ -2,7 +2,6 @@ package com.x8bit.bitwarden.data.platform.manager
 
 import com.x8bit.bitwarden.data.auth.repository.AuthRepository
 import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.RefreshAuthenticator
-import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors
 import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
 import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
 import com.x8bit.bitwarden.data.platform.repository.ServerConfigRepository
@@ -20,7 +19,6 @@ class NetworkConfigManagerImpl(
     authRepository: AuthRepository,
     environmentRepository: EnvironmentRepository,
     serverConfigRepository: ServerConfigRepository,
-    private val baseUrlInterceptors: BaseUrlInterceptors,
     refreshAuthenticator: RefreshAuthenticator,
     dispatcherManager: DispatcherManager,
 ) : NetworkConfigManager {
@@ -31,9 +29,6 @@ class NetworkConfigManagerImpl(
         @Suppress("OPT_IN_USAGE")
         environmentRepository
             .environmentStateFlow
-            .onEach { environment ->
-                baseUrlInterceptors.environment = environment
-            }
             .debounce(timeoutMillis = ENVIRONMENT_DEBOUNCE_TIMEOUT_MS)
             .onEach { _ ->
                 // This updates the stored service configuration by performing a network request.

app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt

@@ -12,7 +12,6 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.PushDiskSource
 import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
 import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacyAppCenterMigrator
 import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.RefreshAuthenticator
-import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors
 import com.x8bit.bitwarden.data.platform.datasource.network.service.EventService
 import com.x8bit.bitwarden.data.platform.datasource.network.service.PushService
 import com.x8bit.bitwarden.data.platform.manager.AppStateManager
@@ -198,7 +197,6 @@ object PlatformManagerModule {
         authRepository: AuthRepository,
         environmentRepository: EnvironmentRepository,
         serverConfigRepository: ServerConfigRepository,
-        baseUrlInterceptors: BaseUrlInterceptors,
         refreshAuthenticator: RefreshAuthenticator,
         dispatcherManager: DispatcherManager,
     ): NetworkConfigManager =
@@ -206,7 +204,6 @@ object PlatformManagerModule {
             authRepository = authRepository,
             environmentRepository = environmentRepository,
             serverConfigRepository = serverConfigRepository,
-            baseUrlInterceptors = baseUrlInterceptors,
             refreshAuthenticator = refreshAuthenticator,
             dispatcherManager = dispatcherManager,
         )

app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/FakeEnvironmentDiskSource.kt

@@ -0,0 +1,34 @@
+package com.x8bit.bitwarden.data.platform.datasource.disk
+
+import com.x8bit.bitwarden.data.auth.datasource.disk.model.EnvironmentUrlDataJson
+import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.onSubscription
+
+class FakeEnvironmentDiskSource : EnvironmentDiskSource {
+    private val storedEmailVerificationUrls = mutableMapOf<String, EnvironmentUrlDataJson?>()
+
+    override var preAuthEnvironmentUrlData: EnvironmentUrlDataJson? = null
+        set(value) {
+            field = value
+            mutablePreAuthEnvironmentUrlDataFlow.tryEmit(value)
+        }
+
+    override val preAuthEnvironmentUrlDataFlow: Flow<EnvironmentUrlDataJson?>
+        get() = mutablePreAuthEnvironmentUrlDataFlow
+            .onSubscription { emit(preAuthEnvironmentUrlData) }
+
+    override fun getPreAuthEnvironmentUrlDataForEmail(
+        userEmail: String,
+    ): EnvironmentUrlDataJson? = storedEmailVerificationUrls[userEmail]
+
+    override fun storePreAuthEnvironmentUrlDataForEmail(
+        userEmail: String,
+        urls: EnvironmentUrlDataJson,
+    ) {
+        storedEmailVerificationUrls[userEmail] = urls
+    }
+
+    private val mutablePreAuthEnvironmentUrlDataFlow =
+        bufferedMutableSharedFlow<EnvironmentUrlDataJson?>(replay = 1)
+}

app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/BaseUrlInterceptorTest.kt

@@ -6,7 +6,8 @@ import org.junit.jupiter.api.Assertions.assertNotEquals
 import org.junit.jupiter.api.Test
 
 class BaseUrlInterceptorTest {
-    private val baseUrlInterceptor = BaseUrlInterceptor()
+    private var baseUrl: String? = null
+    private val baseUrlInterceptor = BaseUrlInterceptor { baseUrl }
 
     @Test
     fun `intercept with a null base URL should proceed with the original request`() {
@@ -22,7 +23,7 @@ class BaseUrlInterceptorTest {
 
     @Test
     fun `intercept with a non-null base URL should update the base URL used by the request`() {
-        baseUrlInterceptor.baseUrl = "https://api.bitwarden.com"
+        baseUrl = "https://api.bitwarden.com"
 
         val request = Request.Builder().url("http://www.fake.com/").build()
         val chain = FakeInterceptorChain(request)

app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/BaseUrlInterceptorsTest.kt

@@ -1,101 +0,0 @@
-package com.x8bit.bitwarden.data.platform.datasource.network.interceptor
-
-import com.x8bit.bitwarden.data.auth.datasource.disk.model.EnvironmentUrlDataJson
-import com.x8bit.bitwarden.data.platform.repository.model.Environment
-import org.junit.jupiter.api.Assertions.assertEquals
-import org.junit.jupiter.api.Test
-
-class BaseUrlInterceptorsTest {
-    private val baseUrlInterceptors = BaseUrlInterceptors()
-
-    @Test
-    fun `the default environment should be US and all interceptors should have the correct URLs`() {
-        assertEquals(
-            Environment.Us,
-            baseUrlInterceptors.environment,
-        )
-        assertEquals(
-            "https://vault.bitwarden.com/api",
-            baseUrlInterceptors.apiInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://vault.bitwarden.com/identity",
-            baseUrlInterceptors.identityInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://vault.bitwarden.com/events",
-            baseUrlInterceptors.eventsInterceptor.baseUrl,
-        )
-    }
-
-    @Suppress("MaxLineLength")
-    @Test
-    fun `setting the environment should update all the interceptors correctly for a non-blank base URL`() {
-        baseUrlInterceptors.environment = Environment.Eu
-
-        assertEquals(
-            "https://vault.bitwarden.eu/api",
-            baseUrlInterceptors.apiInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://vault.bitwarden.eu/identity",
-            baseUrlInterceptors.identityInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://vault.bitwarden.eu/events",
-            baseUrlInterceptors.eventsInterceptor.baseUrl,
-        )
-    }
-
-    @Suppress("MaxLineLength")
-    @Test
-    fun `setting the environment should update all the interceptors correctly for a blank base URL and all URLs filled`() {
-        baseUrlInterceptors.environment = Environment.SelfHosted(
-            environmentUrlData = EnvironmentUrlDataJson(
-                base = "   ",
-                api = "https://api.com",
-                identity = "https://identity.com",
-                events = "https://events.com",
-            ),
-        )
-
-        assertEquals(
-            "https://api.com",
-            baseUrlInterceptors.apiInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://identity.com",
-            baseUrlInterceptors.identityInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://events.com",
-            baseUrlInterceptors.eventsInterceptor.baseUrl,
-        )
-    }
-
-    @Suppress("MaxLineLength")
-    @Test
-    fun `setting the environment should update all the interceptors correctly for a blank base URL and some or all URLs absent`() {
-        baseUrlInterceptors.environment = Environment.SelfHosted(
-            environmentUrlData = EnvironmentUrlDataJson(
-                base = "   ",
-                api = "",
-                identity = "",
-                icon = "   ",
-            ),
-        )
-
-        assertEquals(
-            "https://api.bitwarden.com",
-            baseUrlInterceptors.apiInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://identity.bitwarden.com",
-            baseUrlInterceptors.identityInterceptor.baseUrl,
-        )
-        assertEquals(
-            "https://events.bitwarden.com",
-            baseUrlInterceptors.eventsInterceptor.baseUrl,
-        )
-    }
-}

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

@@ -4,7 +4,6 @@ import com.x8bit.bitwarden.data.auth.repository.AuthRepository
 import com.x8bit.bitwarden.data.auth.repository.model.AuthState
 import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager
 import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.RefreshAuthenticator
-import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors
 import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
 import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
 import com.x8bit.bitwarden.data.platform.repository.ServerConfigRepository
@@ -17,7 +16,6 @@ import io.mockk.mockk
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
-import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 
@@ -42,7 +40,6 @@ class NetworkConfigManagerTest {
     }
 
     private val refreshAuthenticator = RefreshAuthenticator()
-    private val baseUrlInterceptors = BaseUrlInterceptors()
 
     private lateinit var networkConfigManager: NetworkConfigManager
 
@@ -52,25 +49,15 @@ class NetworkConfigManagerTest {
             authRepository = authRepository,
             environmentRepository = environmentRepository,
             serverConfigRepository = serverConfigRepository,
-            baseUrlInterceptors = baseUrlInterceptors,
             refreshAuthenticator = refreshAuthenticator,
             dispatcherManager = dispatcherManager,
         )
     }
 
     @Test
-    fun `changes in the Environment should update the BaseUrlInterceptors`() {
+    fun `changes in the Environment should call getServerConfig after debounce period`() {
         mutableEnvironmentStateFlow.value = Environment.Us
-        assertEquals(
-            Environment.Us,
-            baseUrlInterceptors.environment,
-        )
-
         mutableEnvironmentStateFlow.value = Environment.Eu
-        assertEquals(
-            Environment.Eu,
-            baseUrlInterceptors.environment,
-        )
         testDispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 500L)
         coVerify(exactly = 1) {
             serverConfigRepository.getServerConfig(forceRefresh = true)

app/src/test/java/com/x8bit/bitwarden/data/platform/repository/EnvironmentRepositoryTest.kt

@@ -6,17 +6,14 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.EnvironmentUrlDataJso
 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.platform.base.FakeDispatcherManager
-import com.x8bit.bitwarden.data.platform.datasource.disk.EnvironmentDiskSource
+import com.x8bit.bitwarden.data.platform.datasource.disk.FakeEnvironmentDiskSource
 import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
 import com.x8bit.bitwarden.data.platform.repository.model.Environment
-import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
 import com.x8bit.bitwarden.data.platform.repository.util.toEnvironmentUrls
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkStatic
 import io.mockk.unmockkStatic
-import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.onSubscription
 import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.Assertions.assertEquals
@@ -218,30 +215,3 @@ class EnvironmentRepositoryTest {
 }
 
 private const val EMAIL = "email@example.com"
-
-private class FakeEnvironmentDiskSource : EnvironmentDiskSource {
-    private val storedEmailVerificationUrls = mutableMapOf<String, EnvironmentUrlDataJson?>()
-
-    override var preAuthEnvironmentUrlData: EnvironmentUrlDataJson? = null
-        set(value) {
-            field = value
-            mutablePreAuthEnvironmentUrlDataFlow.tryEmit(value)
-        }
-
-    override val preAuthEnvironmentUrlDataFlow: Flow<EnvironmentUrlDataJson?>
-        get() = mutablePreAuthEnvironmentUrlDataFlow
-            .onSubscription { emit(preAuthEnvironmentUrlData) }
-
-    override fun getPreAuthEnvironmentUrlDataForEmail(userEmail: String): EnvironmentUrlDataJson? =
-        storedEmailVerificationUrls[userEmail]
-
-    override fun storePreAuthEnvironmentUrlDataForEmail(
-        userEmail: String,
-        urls: EnvironmentUrlDataJson,
-    ) {
-        storedEmailVerificationUrls[userEmail] = urls
-    }
-
-    private val mutablePreAuthEnvironmentUrlDataFlow =
-        bufferedMutableSharedFlow<EnvironmentUrlDataJson?>(replay = 1)
-}