From 6ff974b3eab925024ae9897ee9955eb6e4316b47 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoitm@matrix.org>
Date: Wed, 12 Feb 2020 11:16:21 +0100
Subject: [PATCH] Fix issue with verification when other client declares it can
only show QR code (#988)
---
CHANGES.md | 1 +
.../verification/qrcode/VerificationTest.kt | 232 ++++++++++++++++++
.../PendingVerificationRequest.kt | 38 ++-
.../VerificationChooseMethodViewModel.kt | 14 +-
4 files changed, 272 insertions(+), 13 deletions(-)
create mode 100644 matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/verification/qrcode/VerificationTest.kt
diff --git a/CHANGES.md b/CHANGES.md
index 1ffa3c7ebe..0920127803 100644
--- a/CHANGES.md
+++ b/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 🗣:
-
diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/verification/qrcode/VerificationTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/verification/qrcode/VerificationTest.kt
new file mode 100644
index 0000000000..adde452619
--- /dev/null
+++ b/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()
+ }
+}
diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/PendingVerificationRequest.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/PendingVerificationRequest.kt
index 3379ddd2ed..fe5f9dadb9 100644
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/PendingVerificationRequest.kt
+++ b/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()
}
}
}
diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/choose/VerificationChooseMethodViewModel.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/choose/VerificationChooseMethodViewModel.kt
index 75c1b69058..17d8c1578d 100644
--- a/vector/src/main/java/im/vector/riotx/features/crypto/verification/choose/VerificationChooseMethodViewModel.kt
+++ b/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()
)
}
}