Provide callback instead of doing UI tasks of calling app #49

README.md

@@ -80,11 +80,6 @@ Example of initialzing an okhttp client:
     }
 
 
-You can overwrite resources when you want, just have a look at the `res/strings`
-directory. Especially `certificate_notification_connection_security` and
-`trust_certificate_unknown_certificate_found` should contain your app name.
-
-
 # License 
 
 Copyright (C) Ricki Hirner and [contributors](https://github.com/bitfireAT/cert4android/graphs/contributors).

lib/src/androidTest/java/at/bitfire/cert4android/CustomCertManagerTest.kt

@@ -5,6 +5,10 @@
 package at.bitfire.cert4android
 
 import androidx.test.platform.app.InstrumentationRegistry
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.cancel
+import org.junit.After
 import org.junit.Assume.assumeNotNull
 import org.junit.Before
 import org.junit.Test
@@ -19,6 +23,7 @@ class CustomCertManagerTest {
 
     private lateinit var certManager: CustomCertManager
     private lateinit var paranoidCertManager: CustomCertManager
+    private val scope: CoroutineScope = CoroutineScope(Dispatchers.Default)
 
     private var siteCerts: List<X509Certificate>? =
         try {
@@ -32,10 +37,14 @@ class CustomCertManagerTest {
 
     @Before
     fun createCertManager() {
-        certManager = CustomCertManager(context, true, null)
-        paranoidCertManager = CustomCertManager(context, false, null)
+        certManager = CustomCertManager(context, true, scope, getUserDecision = { true })
+        paranoidCertManager = CustomCertManager(context, false, scope, getUserDecision = { false })
     }
 
+    @After
+    fun cleanUp() {
+        scope.cancel()
+    }
 
     @Test(expected = CertificateException::class)
     fun testCheckClientCertificate() {

lib/src/androidTest/java/at/bitfire/cert4android/UserDecisionRegistryTest.kt

@@ -1,20 +1,22 @@
 package at.bitfire.cert4android
 
 import androidx.test.platform.app.InstrumentationRegistry
-import io.mockk.every
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.mockk
 import io.mockk.mockkObject
 import io.mockk.unmockkAll
-import io.mockk.verify
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import org.junit.After
 import org.junit.Assert.*
 import org.junit.Before
 import org.junit.Test
+import java.security.cert.X509Certificate
 import java.util.Collections
 import java.util.concurrent.Semaphore
-import kotlin.concurrent.thread
 
 class UserDecisionRegistryTest {
 
@@ -27,9 +29,7 @@ class UserDecisionRegistryTest {
 
     @Before
     fun setUp() {
-        mockkObject(NotificationUtils)
         mockkObject(registry)
-        every { registry.requestDecision(any(), any(), any()) } returns Unit
     }
 
     @After
@@ -41,86 +41,73 @@ class UserDecisionRegistryTest {
 
     @Test
     fun testCheck_FirstDecision_Negative() {
-        every { registry.requestDecision(testCert, any(), any()) } answers {
-            registry.onUserDecision(testCert, false)
-        }
         assertFalse(runBlocking {
-            registry.check(testCert, true)
+            registry.check(testCert, this) { false }
         })
     }
 
     @Test
     fun testCheck_FirstDecision_Positive() {
-        every { registry.requestDecision(testCert, any(), any()) } answers {
-            registry.onUserDecision(testCert, true)
-        }
         assertTrue(runBlocking {
-            registry.check(testCert, true)
+            registry.check(testCert, this) { true }
         })
     }
 
     @Test
     fun testCheck_MultipleDecisionsForSameCert_Negative() {
         val canSendFeedback = Semaphore(0)
-        every { registry.requestDecision(testCert, any(), any()) } answers {
-            thread {
-                canSendFeedback.acquire()
-                registry.onUserDecision(testCert, false)
+        val getUserDecision: suspend (X509Certificate) -> Boolean = mockk {
+            coEvery { this@mockk(testCert) } coAnswers {
+                canSendFeedback.acquire() // block call until released
+                false
             }
         }
         val results = Collections.synchronizedList(mutableListOf<Boolean>())
-        runBlocking {
+        runBlocking(Dispatchers.Default) {
+            // launch 5 getUserDecision calls (each will be blocked by the semaphore)
             repeat(5) {
-                launch(Dispatchers.Default) {
-                    results += registry.check(testCert, true)
+                launch {
+                    results += registry.check(testCert, this, getUserDecision)
                 }
             }
-            canSendFeedback.release()
+            delay(1000) // wait a bit for all getUserDecision calls to be launched and blocked
+            canSendFeedback.release() // now unblock all calls at the same time
         }
+
+        // pendingDecisions should be empty
         synchronized(registry.pendingDecisions) {
             assertFalse(registry.pendingDecisions.containsKey(testCert))
         }
-        assertEquals(5, results.size)
-        assertTrue(results.all { !it })
-        verify(exactly = 1) { registry.requestDecision(any(), any(), any()) }
+        assertEquals(5, results.size) // should be 5 results
+        assertTrue(results.all { result -> !result }) // all results should be false
+        coVerify(exactly = 1) { getUserDecision(testCert) } // getUserDecision should be called only once
     }
 
     @Test
     fun testCheck_MultipleDecisionsForSameCert_Positive() {
         val canSendFeedback = Semaphore(0)
-        every { registry.requestDecision(testCert, any(), any()) } answers {
-            thread {
+        val getUserDecision: suspend (X509Certificate) -> Boolean = mockk {
+            coEvery { this@mockk(testCert) } coAnswers {
                 canSendFeedback.acquire()
-                registry.onUserDecision(testCert, true)
+                true
             }
         }
         val results = Collections.synchronizedList(mutableListOf<Boolean>())
-        runBlocking {
+        runBlocking(Dispatchers.Default) {
             repeat(5) {
-                launch(Dispatchers.Default) {
-                    results += registry.check(testCert, true)
+                launch {
+                    results += registry.check(testCert, this, getUserDecision)
                 }
             }
+            delay(1000)
             canSendFeedback.release()
         }
         synchronized(registry.pendingDecisions) {
             assertFalse(registry.pendingDecisions.containsKey(testCert))
         }
         assertEquals(5, results.size)
         assertTrue(results.all { it })
-        verify(exactly = 1) { registry.requestDecision(any(), any(), any()) }
-    }
-
-    @Test
-    fun testCheck_UserDecisionImpossible() {
-        every { NotificationUtils.notificationsPermitted(any()) } returns false
-        assertFalse(runBlocking {
-            // should return instantly
-            registry.check(testCert, false)
-        })
-        verify(inverse = true) {
-            registry.requestDecision(any(), any(), any())
-        }
+        coVerify(exactly = 1) { getUserDecision(testCert) }
     }
 
 }

lib/src/main/AndroidManifest.xml

@@ -3,14 +3,4 @@
 
     <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
 
-    <application>
-
-        <activity
-            android:name=".TrustCertificateActivity"
-            android:label="@string/certificate_notification_connection_security"
-            android:launchMode="singleInstance"
-            android:excludeFromRecents="true"
-            android:exported="true"/>
-
-    </application>
 </manifest>

lib/src/main/java/at/bitfire/cert4android/CustomCertManager.kt

@@ -6,7 +6,7 @@ package at.bitfire.cert4android
 
 import android.annotation.SuppressLint
 import android.content.Context
-import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.CoroutineScope
 import java.security.cert.CertificateException
 import java.security.cert.X509Certificate
 import javax.net.ssl.SSLSession
@@ -18,15 +18,15 @@ import javax.net.ssl.X509TrustManager
  * Initializes Conscrypt when it is first loaded.
  *
  * @param trustSystemCerts whether system certificates will be trusted
- * @param appInForeground  - `true`: if needed, directly launches [TrustCertificateActivity] and shows notification (if possible)
- *                         - `false`: if needed, shows notification (if possible)
- *                         - `null`: non-interactive mode: does not show notification or launch activity
+ * @param getUserDecision  anonymous function to retrieve user decision on whether to trust a
+ *                         certificate; should return *true* if the user trusts the certificate
  */
 @SuppressLint("CustomX509TrustManager")
 class CustomCertManager @JvmOverloads constructor(
     context: Context,
     val trustSystemCerts: Boolean = true,
-    var appInForeground: StateFlow<Boolean>?
+    private val scope: CoroutineScope,
+    private val getUserDecision: suspend (X509Certificate) -> Boolean
 ): X509TrustManager {
 
     val certStore = CustomCertStore.getInstance(context)
@@ -47,7 +47,7 @@ class CustomCertManager @JvmOverloads constructor(
      */
     @Throws(CertificateException::class)
     override fun checkServerTrusted(chain: Array<X509Certificate>, authType: String) {
-        if (!certStore.isTrusted(chain, authType, trustSystemCerts, appInForeground))
+        if (!certStore.isTrusted(chain, authType, trustSystemCerts, scope, getUserDecision))
             throw CertificateException("Certificate chain not trusted")
     }
 
@@ -71,7 +71,7 @@ class CustomCertManager @JvmOverloads constructor(
             // Allow users to explicitly accept certificates that have a bad hostname here
             (session.peerCertificates.firstOrNull() as? X509Certificate)?.let { cert ->
                 // Check without trusting system certificates so that the user will be asked even for system-trusted certificates
-                if (certStore.isTrusted(arrayOf(cert), "RSA", false, appInForeground))
+                if (certStore.isTrusted(arrayOf(cert), "RSA", false, scope, getUserDecision))
                     return true
             }
 

lib/src/main/java/at/bitfire/cert4android/CustomCertStore.kt

@@ -7,8 +7,8 @@ package at.bitfire.cert4android
 import android.annotation.SuppressLint
 import android.content.Context
 import androidx.annotation.VisibleForTesting
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.TimeoutCancellationException
-import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
 import org.conscrypt.Conscrypt
@@ -77,7 +77,13 @@ class CustomCertStore internal constructor(
     /**
      * Determines whether a certificate chain is trusted.
      */
-    fun isTrusted(chain: Array<X509Certificate>, authType: String, trustSystemCerts: Boolean, appInForeground: StateFlow<Boolean>?): Boolean {
+    fun isTrusted(
+        chain: Array<X509Certificate>,
+        authType: String,
+        trustSystemCerts: Boolean,
+        scope: CoroutineScope,
+        getUserDecision: suspend (X509Certificate) -> Boolean
+    ): Boolean {
         if (chain.isEmpty())
             throw IllegalArgumentException("Certificate chain must not be empty")
         val cert = chain[0]
@@ -103,17 +109,12 @@ class CustomCertStore internal constructor(
                 }
         }
 
-        if (appInForeground == null) {
-            Cert4Android.log.log(Level.INFO, "Certificate not known and running in non-interactive mode, rejecting")
-            return false
-        }
-
         return runBlocking {
             val ui = UserDecisionRegistry.getInstance(context)
 
             try {
                 withTimeout(userTimeout) {
-                    ui.check(cert, appInForeground.value)
+                    ui.check(cert, scope, getUserDecision)
                 }
             } catch (_: TimeoutCancellationException) {
                 Cert4Android.log.log(Level.WARNING, "User timeout while waiting for certificate decision, rejecting")