Fix issue with verification when other client declares it can only show QR code (#988)

CHANGES.md

@@ -12,6 +12,7 @@ Other changes:
 
 Bugfix 🐛:
  - Fix crash by removing all notifications after clearing cache (#878)
+ - Fix issue with verification when other client declares it can only show QR code (#988)
 
 Translations 🗣:
  -

matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/verification/qrcode/VerificationTest.kt

@@ -0,0 +1,232 @@
+/*
+ * Copyright (c) 2020 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.matrix.android.internal.crypto.verification.qrcode
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import im.vector.matrix.android.InstrumentedTest
+import im.vector.matrix.android.api.session.crypto.sas.VerificationMethod
+import im.vector.matrix.android.api.session.crypto.sas.VerificationService
+import im.vector.matrix.android.common.CommonTestHelper
+import im.vector.matrix.android.common.CryptoTestHelper
+import im.vector.matrix.android.common.TestConstants
+import im.vector.matrix.android.internal.crypto.model.rest.UserPasswordAuth
+import im.vector.matrix.android.internal.crypto.verification.PendingVerificationRequest
+import org.amshove.kluent.shouldBe
+import org.junit.FixMethodOrder
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.MethodSorters
+import java.util.concurrent.CountDownLatch
+
+@RunWith(AndroidJUnit4::class)
+@FixMethodOrder(MethodSorters.JVM)
+class VerificationTest : InstrumentedTest {
+    private val mTestHelper = CommonTestHelper(context())
+    private val mCryptoTestHelper = CryptoTestHelper(mTestHelper)
+
+    data class ExpectedResult(
+            val sasIsSupported: Boolean = false,
+            val otherCanScanQrCode: Boolean = false,
+            val otherCanShowQrCode: Boolean = false
+    )
+
+    private val sas = listOf(
+            VerificationMethod.SAS
+    )
+
+    private val sasShow = listOf(
+            VerificationMethod.SAS,
+            VerificationMethod.QR_CODE_SHOW
+    )
+
+    private val sasScan = listOf(
+            VerificationMethod.SAS,
+            VerificationMethod.QR_CODE_SCAN
+    )
+
+    private val sasShowScan = listOf(
+            VerificationMethod.SAS,
+            VerificationMethod.QR_CODE_SHOW,
+            VerificationMethod.QR_CODE_SCAN
+    )
+
+    @Test
+    fun test_aliceAndBob_sas_sas() = doTest(
+            sas,
+            sas,
+            ExpectedResult(sasIsSupported = true),
+            ExpectedResult(sasIsSupported = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_sas_show() = doTest(
+            sas,
+            sasShow,
+            ExpectedResult(sasIsSupported = true),
+            ExpectedResult(sasIsSupported = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_show_sas() = doTest(
+            sasShow,
+            sas,
+            ExpectedResult(sasIsSupported = true),
+            ExpectedResult(sasIsSupported = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_sas_scan() = doTest(
+            sas,
+            sasScan,
+            ExpectedResult(sasIsSupported = true),
+            ExpectedResult(sasIsSupported = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_scan_sas() = doTest(
+            sasScan,
+            sas,
+            ExpectedResult(sasIsSupported = true),
+            ExpectedResult(sasIsSupported = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_scan_scan() = doTest(
+            sasScan,
+            sasScan,
+            ExpectedResult(sasIsSupported = true),
+            ExpectedResult(sasIsSupported = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_show_show() = doTest(
+            sasShow,
+            sasShow,
+            ExpectedResult(sasIsSupported = true),
+            ExpectedResult(sasIsSupported = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_show_scan() = doTest(
+            sasShow,
+            sasScan,
+            ExpectedResult(sasIsSupported = true, otherCanScanQrCode = true),
+            ExpectedResult(sasIsSupported = true, otherCanShowQrCode = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_scan_show() = doTest(
+            sasScan,
+            sasShow,
+            ExpectedResult(sasIsSupported = true, otherCanShowQrCode = true),
+            ExpectedResult(sasIsSupported = true, otherCanScanQrCode = true)
+    )
+
+    @Test
+    fun test_aliceAndBob_all_all() = doTest(
+            sasShowScan,
+            sasShowScan,
+            ExpectedResult(sasIsSupported = true, otherCanShowQrCode = true, otherCanScanQrCode = true),
+            ExpectedResult(sasIsSupported = true, otherCanShowQrCode = true, otherCanScanQrCode = true)
+    )
+
+    // TODO Add tests without SAS
+
+    private fun doTest(aliceSupportedMethods: List<VerificationMethod>,
+                       bobSupportedMethods: List<VerificationMethod>,
+                       expectedResultForAlice: ExpectedResult,
+                       expectedResultForBob: ExpectedResult) {
+        val cryptoTestData = mCryptoTestHelper.doE2ETestWithAliceAndBobInARoom()
+
+        val aliceSession = cryptoTestData.firstSession
+        val bobSession = cryptoTestData.secondSession!!
+
+        mTestHelper.doSync<Unit> { callback ->
+            aliceSession.getCrossSigningService()
+                    .initializeCrossSigning(UserPasswordAuth(
+                            user = aliceSession.myUserId,
+                            password = TestConstants.PASSWORD
+                    ), callback)
+        }
+
+        mTestHelper.doSync<Unit> { callback ->
+            bobSession.getCrossSigningService()
+                    .initializeCrossSigning(UserPasswordAuth(
+                            user = bobSession.myUserId,
+                            password = TestConstants.PASSWORD
+                    ), callback)
+        }
+
+        val aliceVerificationService = aliceSession.getVerificationService()
+        val bobVerificationService = bobSession.getVerificationService()
+
+        var aliceReadyPendingVerificationRequest: PendingVerificationRequest? = null
+        var bobReadyPendingVerificationRequest: PendingVerificationRequest? = null
+
+        val latch = CountDownLatch(2)
+        val aliceListener = object : VerificationService.VerificationListener {
+            override fun verificationRequestUpdated(pr: PendingVerificationRequest) {
+                // Step 4: Alice receive the ready request
+                if (pr.isReady) {
+                    aliceReadyPendingVerificationRequest = pr
+                    latch.countDown()
+                }
+            }
+        }
+        aliceVerificationService.addListener(aliceListener)
+
+        val bobListener = object : VerificationService.VerificationListener {
+            override fun verificationRequestCreated(pr: PendingVerificationRequest) {
+                // Step 2: Bob accepts the verification request
+                bobVerificationService.readyPendingVerificationInDMs(
+                        bobSupportedMethods,
+                        aliceSession.myUserId,
+                        cryptoTestData.roomId,
+                        pr.transactionId!!
+                )
+            }
+
+            override fun verificationRequestUpdated(pr: PendingVerificationRequest) {
+                // Step 3: Bob is ready
+                if (pr.isReady) {
+                    bobReadyPendingVerificationRequest = pr
+                    latch.countDown()
+                }
+            }
+        }
+        bobVerificationService.addListener(bobListener)
+
+        val bobUserId = bobSession.myUserId
+        // Step 1: Alice starts a verification request
+        aliceVerificationService.requestKeyVerificationInDMs(aliceSupportedMethods, bobUserId, cryptoTestData.roomId)
+        mTestHelper.await(latch)
+
+        aliceReadyPendingVerificationRequest!!.let { pr ->
+            pr.isSasSupported() shouldBe expectedResultForAlice.sasIsSupported
+            pr.otherCanShowQrCode() shouldBe expectedResultForAlice.otherCanShowQrCode
+            pr.otherCanScanQrCode() shouldBe expectedResultForAlice.otherCanScanQrCode
+        }
+
+        bobReadyPendingVerificationRequest!!.let { pr ->
+            pr.isSasSupported() shouldBe expectedResultForBob.sasIsSupported
+            pr.otherCanShowQrCode() shouldBe expectedResultForBob.otherCanShowQrCode
+            pr.otherCanScanQrCode() shouldBe expectedResultForBob.otherCanScanQrCode
+        }
+
+        cryptoTestData.close()
+    }
+}

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/PendingVerificationRequest.kt

@@ -15,8 +15,8 @@
  */
 package im.vector.matrix.android.internal.crypto.verification
 
+import im.vector.matrix.android.api.extensions.orFalse
 import im.vector.matrix.android.api.session.crypto.sas.CancelCode
-import im.vector.matrix.android.api.session.crypto.sas.VerificationMethod
 import im.vector.matrix.android.internal.crypto.model.rest.VERIFICATION_METHOD_QR_CODE_SCAN
 import im.vector.matrix.android.internal.crypto.model.rest.VERIFICATION_METHOD_QR_CODE_SHOW
 import im.vector.matrix.android.internal.crypto.model.rest.VERIFICATION_METHOD_SAS
@@ -46,11 +46,37 @@ data class PendingVerificationRequest(
 
     val isFinished: Boolean = isSuccessful || cancelConclusion != null
 
-    fun hasMethod(method: VerificationMethod): Boolean? {
-        return when (method) {
-            VerificationMethod.SAS          -> readyInfo?.methods?.contains(VERIFICATION_METHOD_SAS)
-            VerificationMethod.QR_CODE_SHOW -> readyInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SHOW)
-            VerificationMethod.QR_CODE_SCAN -> readyInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SCAN)
+    /**
+     * SAS is supported if I support it and the other party support it
+     */
+    fun isSasSupported(): Boolean {
+        return requestInfo?.methods?.contains(VERIFICATION_METHOD_SAS).orFalse()
+                && readyInfo?.methods?.contains(VERIFICATION_METHOD_SAS).orFalse()
+    }
+
+    /**
+     * Other can show QR code if I can scan QR code and other can show QR code
+     */
+    fun otherCanShowQrCode(): Boolean {
+        return if (isIncoming) {
+            requestInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SHOW).orFalse()
+                    && readyInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SCAN).orFalse()
+        } else {
+            requestInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SCAN).orFalse()
+                    && readyInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SHOW).orFalse()
+        }
+    }
+
+    /**
+     * Other can scan QR code if I can show QR code and other can scan QR code
+     */
+    fun otherCanScanQrCode(): Boolean {
+        return if (isIncoming) {
+            requestInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SCAN).orFalse()
+                    && readyInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SHOW).orFalse()
+        } else {
+            requestInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SHOW).orFalse()
+                    && readyInfo?.methods?.contains(VERIFICATION_METHOD_QR_CODE_SCAN).orFalse()
         }
     }
 }

vector/src/main/java/im/vector/riotx/features/crypto/verification/choose/VerificationChooseMethodViewModel.kt

@@ -21,9 +21,9 @@ import com.airbnb.mvrx.MvRxViewModelFactory
 import com.airbnb.mvrx.ViewModelContext
 import com.squareup.inject.assisted.Assisted
 import com.squareup.inject.assisted.AssistedInject
+import im.vector.matrix.android.api.extensions.orFalse
 import im.vector.matrix.android.api.session.Session
 import im.vector.matrix.android.api.session.crypto.sas.QrCodeVerificationTransaction
-import im.vector.matrix.android.api.session.crypto.sas.VerificationMethod
 import im.vector.matrix.android.api.session.crypto.sas.VerificationService
 import im.vector.matrix.android.api.session.crypto.sas.VerificationTransaction
 import im.vector.matrix.android.internal.crypto.verification.PendingVerificationRequest
@@ -66,9 +66,9 @@ class VerificationChooseMethodViewModel @AssistedInject constructor(
 
         setState {
             copy(
-                    otherCanShowQrCode = pvr?.hasMethod(VerificationMethod.QR_CODE_SHOW) ?: false,
-                    otherCanScanQrCode = pvr?.hasMethod(VerificationMethod.QR_CODE_SCAN) ?: false,
-                    SASModeAvailable = pvr?.hasMethod(VerificationMethod.SAS) ?: false
+                    otherCanShowQrCode = pvr?.otherCanShowQrCode().orFalse(),
+                    otherCanScanQrCode = pvr?.otherCanScanQrCode().orFalse(),
+                    SASModeAvailable = pvr?.isSasSupported().orFalse()
             )
         }
     }
@@ -103,10 +103,10 @@ class VerificationChooseMethodViewModel @AssistedInject constructor(
 
             return VerificationChooseMethodViewState(otherUserId = args.otherUserId,
                     transactionId = args.verificationId ?: "",
-                    otherCanShowQrCode = pvr?.hasMethod(VerificationMethod.QR_CODE_SHOW) ?: false,
-                    otherCanScanQrCode = pvr?.hasMethod(VerificationMethod.QR_CODE_SCAN) ?: false,
+                    otherCanShowQrCode = pvr?.otherCanShowQrCode().orFalse(),
+                    otherCanScanQrCode = pvr?.otherCanScanQrCode().orFalse(),
                     qrCodeText = (qrCodeVerificationTransaction as? QrCodeVerificationTransaction)?.qrCodeText,
-                    SASModeAvailable = pvr?.hasMethod(VerificationMethod.SAS) ?: false
+                    SASModeAvailable = pvr?.isSasSupported().orFalse()
             )
         }
     }