refactor: create a new okhttp client for each user on jvm target (#2304)

* refactor: create a new okhttp client for each target on jvm target * detekt * fix tests * lazy init OkHttpSingleton
wireapp · Dec 12, 2023 · 998f48e · 998f48e
1 parent 4cf1578
commit 998f48e
Show file tree

Hide file tree

Showing 5 changed files with 151 additions and 22 deletions.
diff --git a/network/src/androidMain/kotlin/com/wire/kalium/network/OkhttpClientFactory.kt b/network/src/androidMain/kotlin/com/wire/kalium/network/OkhttpClientFactory.kt
@@ -0,0 +1,42 @@
+/*
+ * Wire
+ * Copyright (C) 2023 Wire Swiss GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ */
+package com.wire.kalium.network
+
+import okhttp3.OkHttpClient
+import java.util.concurrent.TimeUnit
+
+@Suppress("FunctionName")
+fun OkhttpClientFactory(block: OkHttpClient.Builder.() -> Unit): OkHttpClient = OkHttpSingleton.createNew(block)
+
+private object OkHttpSingleton {
+    private val sharedClient by lazy {
+        OkHttpClient.Builder().apply {
+            // OkHttp doesn't support configuring ping intervals dynamically,
+            // so they must be set when creating the Engine
+            // See https://youtrack.jetbrains.com/issue/KTOR-4752
+            pingInterval(WEBSOCKET_PING_INTERVAL_MILLIS, TimeUnit.MILLISECONDS)
+                .connectTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+                .readTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+                .writeTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+        }.connectionSpecs(supportedConnectionSpecs()).build()
+    }
+
+    fun createNew(block: OkHttpClient.Builder.() -> Unit): OkHttpClient {
+        return sharedClient.newBuilder().apply(block).build()
+    }
+}
diff --git a/...ork/src/androidUnitTest/kotlin/com/wire/kalium/api/tools/HttpClientConnectionSpecsTest.kt b/...ork/src/androidUnitTest/kotlin/com/wire/kalium/api/tools/HttpClientConnectionSpecsTest.kt
@@ -0,0 +1,70 @@
+/*
+ * Wire
+ * Copyright (C) 2023 Wire Swiss GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ */
+package com.wire.kalium.api.tools
+
+import com.wire.kalium.network.OkhttpClientFactory
+import okhttp3.CipherSuite
+import okhttp3.ConnectionSpec
+import okhttp3.TlsVersion
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+
+class HttpClientConnectionSpecsTest {
+
+    @Test
+    // This test conforms to the following testing standards:
+    // @SF.Channel @TSFI.RESTfulAPI @S0.2 @S0.3 @S3
+    fun givenTheHttpClientIsCreated_ThenEnsureOnlySupportedSpecsArePresent() {
+        // given
+        val validTlsVersions = listOf(TlsVersion.TLS_1_3, TlsVersion.TLS_1_2)
+        val notValidTlsVersions = listOf(TlsVersion.TLS_1_1, TlsVersion.TLS_1_0, TlsVersion.SSL_3_0)
+        val validCipherSuites = listOf(
+            CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+            CipherSuite.TLS_AES_128_GCM_SHA256,
+            CipherSuite.TLS_AES_256_GCM_SHA384
+        )
+        val notValidCipherSuites = listOf(
+            CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+            CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
+            CipherSuite.TLS_RSA_WITH_AES_128_GCM_SHA256,
+            CipherSuite.TLS_RSA_WITH_AES_256_GCM_SHA384,
+            CipherSuite.TLS_RSA_WITH_AES_128_CBC_SHA,
+            CipherSuite.TLS_RSA_WITH_AES_256_CBC_SHA,
+            CipherSuite.TLS_RSA_WITH_3DES_EDE_CBC_SHA
+        )
+
+        // when
+        val connectionSpecs = OkhttpClientFactory { }.connectionSpecs
+
+        // then
+        with(connectionSpecs[0]) {
+            tlsVersions?.let {
+                assertTrue { it.containsAll(validTlsVersions) }
+                assertFalse { it.containsAll(notValidTlsVersions) }
+            }
+
+            cipherSuites?.let {
+                assertTrue { it.containsAll(validCipherSuites) }
+                assertFalse { it.containsAll(notValidCipherSuites) }
+            }
+        }
+        assertEquals(connectionSpecs[1], ConnectionSpec.CLEARTEXT)
+    }
+}
diff --git a/network/src/commonJvmAndroid/kotlin/com/wire/kalium/network/HttpEngine.kt b/network/src/commonJvmAndroid/kotlin/com/wire/kalium/network/HttpEngine.kt
@@ -35,35 +35,17 @@ import java.net.PasswordAuthentication
 import java.net.Proxy
 import java.security.SecureRandom
 import java.security.cert.X509Certificate
-import java.util.concurrent.TimeUnit
 import javax.net.ssl.SSLContext
 import javax.net.ssl.TrustManager
 import javax.net.ssl.X509TrustManager
 
-internal object OkHttpSingleton {
-    private val sharedClient = OkHttpClient.Builder().apply {
-
-        // OkHttp doesn't support configuring ping intervals dynamically,
-        // so they must be set when creating the Engine
-        // See https://youtrack.jetbrains.com/issue/KTOR-4752
-        pingInterval(WEBSOCKET_PING_INTERVAL_MILLIS, TimeUnit.MILLISECONDS)
-            .connectTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
-            .readTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
-            .writeTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
-    }.connectionSpecs(supportedConnectionSpecs()).build()
-
-    fun createNew(block: OkHttpClient.Builder.() -> Unit): OkHttpClient {
-        return sharedClient.newBuilder().apply(block).build()
-    }
-}
-
 actual fun defaultHttpEngine(
     serverConfigDTOApiProxy: ServerConfigDTO.ApiProxy?,
     proxyCredentials: ProxyCredentialsDTO?,
     ignoreSSLCertificates: Boolean,
     certificatePinning: CertificatePinning
 ): HttpClientEngine = OkHttp.create {
-    OkHttpSingleton.createNew {
+    OkhttpClientFactory {
         if (certificatePinning.isNotEmpty()) {
             val certPinner = CertificatePinner.Builder().apply {
                 certificatePinning.forEach { (cert, hosts) ->
@@ -119,7 +101,7 @@ private fun OkHttpClient.Builder.ignoreAllSSLErrors() {
     hostnameVerifier { _, _ -> true }
 }
 
-private fun supportedConnectionSpecs(): List<ConnectionSpec> {
+fun supportedConnectionSpecs(): List<ConnectionSpec> {
     val wireSpec = ConnectionSpec.Builder(ConnectionSpec.RESTRICTED_TLS).build()
     return listOf(wireSpec, ConnectionSpec.CLEARTEXT)
 }
diff --git a/network/src/jvmMain/kotlin/com/wire/kalium/network/OkhttpClientFactory.kt b/network/src/jvmMain/kotlin/com/wire/kalium/network/OkhttpClientFactory.kt
@@ -0,0 +1,35 @@
+/*
+ * Wire
+ * Copyright (C) 2023 Wire Swiss GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ */
+package com.wire.kalium.network
+
+import okhttp3.OkHttpClient
+import java.util.concurrent.TimeUnit
+
+@Suppress("FunctionName")
+fun OkhttpClientFactory(block: OkHttpClient.Builder.() -> Unit): OkHttpClient = OkHttpClient.Builder()
+    .apply(block)
+    .apply {
+
+        // OkHttp doesn't support configuring ping intervals dynamically,
+        // so they must be set when creating the Engine
+        // See https://youtrack.jetbrains.com/issue/KTOR-4752
+        pingInterval(WEBSOCKET_PING_INTERVAL_MILLIS, TimeUnit.MILLISECONDS)
+            .connectTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+            .readTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+            .writeTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+    }.connectionSpecs(supportedConnectionSpecs()).build()
diff --git a/network/src/jvmTest/kotlin/com/wire/kalium/HttpClientConnectionSpecsTest.kt b/network/src/jvmTest/kotlin/com/wire/kalium/HttpClientConnectionSpecsTest.kt
@@ -17,7 +17,7 @@
  */
 package com.wire.kalium
 
-import com.wire.kalium.network.OkHttpSingleton
+import com.wire.kalium.network.OkhttpClientFactory
 import okhttp3.CipherSuite
 import okhttp3.ConnectionSpec
 import okhttp3.TlsVersion
@@ -51,7 +51,7 @@ class HttpClientConnectionSpecsTest {
         )
 
         // when
-        val connectionSpecs = OkHttpSingleton.createNew {}.connectionSpecs
+        val connectionSpecs = OkhttpClientFactory { }.connectionSpecs
 
         // then
         with(connectionSpecs[0]) {