From 2111daea5237534dbff2f1c1818f8d9d6e208518 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 30 Jan 2020 10:08:10 +0100 Subject: [PATCH] Add a step to confirm that other user has scanned the SR code --- .../sas/QrCodeVerificationTransaction.kt | 10 +++ .../session/crypto/sas/VerificationTxState.kt | 5 +- .../DefaultQrCodeVerificationTransaction.kt | 13 +++- .../im/vector/riotx/core/di/FragmentModule.kt | 7 +- .../crypto/verification/VerificationAction.kt | 3 + .../verification/VerificationBottomSheet.kt | 12 ++- .../VerificationBottomSheetViewModel.kt | 33 ++++++-- .../VerificationQrScannedByOtherController.kt | 76 +++++++++++++++++++ .../VerificationQrScannedByOtherFragment.kt | 62 +++++++++++++++ vector/src/main/res/values/strings_riotX.xml | 4 + 10 files changed, 212 insertions(+), 13 deletions(-) create mode 100644 vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherController.kt create mode 100644 vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherFragment.kt diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/QrCodeVerificationTransaction.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/QrCodeVerificationTransaction.kt index b2b6cfdd39..ef6462f854 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/QrCodeVerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/QrCodeVerificationTransaction.kt @@ -27,4 +27,14 @@ interface QrCodeVerificationTransaction : VerificationTransaction { * Call when you have scan the other user QR code */ fun userHasScannedOtherQrCode(otherQrCodeText: String) + + /** + * Call when you confirm that other user has scanned your QR code + */ + fun otherUserScannedMyQrCode() + + /** + * Call when you do not confirm that other user has scanned your QR code + */ + fun otherUserDidNotScannedMyQrCode() } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/VerificationTxState.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/VerificationTxState.kt index 2ef659ba6a..b30dde2d4d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/VerificationTxState.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/sas/VerificationTxState.kt @@ -39,7 +39,10 @@ sealed class VerificationTxState { object Verifying : VerificationSasTxState() // Specific for QR code - // TODO Add code for the confirmation step for the user who has been scanned + abstract class VerificationQrTxState : VerificationTxState() + + // Will be used to ask the user if the other user has correctly scanned + object QrScannedByOther : VerificationQrTxState() // Terminal states abstract class TerminalTxState : VerificationTxState() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt index 6241142c37..b999059d86 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt @@ -198,13 +198,24 @@ internal class DefaultQrCodeVerificationTransaction( if (startReq.sharedSecret == qrCodeData.sharedSecret) { // Ok, we can trust the other user // We can only trust the master key in this case - trust(true, emptyList()) + // But first, ask the user for a confirmation + state = VerificationTxState.QrScannedByOther } else { // Display a warning cancel(CancelCode.MismatchedKeys) } } + override fun otherUserScannedMyQrCode() { + trust(true, emptyList()) + } + + override fun otherUserDidNotScannedMyQrCode() { + // What can I do then? + // At least remove the transaction... + state = VerificationTxState.Cancelled(CancelCode.MismatchedKeys, true) + } + private fun trust(canTrustOtherUserMasterKey: Boolean, toVerifyDeviceIds: List) { // If not me sign his MSK and upload the signature if (otherUserId != userId && canTrustOtherUserMasterKey) { diff --git a/vector/src/main/java/im/vector/riotx/core/di/FragmentModule.kt b/vector/src/main/java/im/vector/riotx/core/di/FragmentModule.kt index 5cc0e071a8..6031a9f6cf 100644 --- a/vector/src/main/java/im/vector/riotx/core/di/FragmentModule.kt +++ b/vector/src/main/java/im/vector/riotx/core/di/FragmentModule.kt @@ -28,6 +28,7 @@ import im.vector.riotx.features.crypto.keysbackup.settings.KeysBackupSettingsFra import im.vector.riotx.features.crypto.verification.choose.VerificationChooseMethodFragment import im.vector.riotx.features.crypto.verification.conclusion.VerificationConclusionFragment import im.vector.riotx.features.crypto.verification.emoji.VerificationEmojiCodeFragment +import im.vector.riotx.features.crypto.verification.qrconfirmation.VerificationQrScannedByOtherFragment import im.vector.riotx.features.crypto.verification.request.VerificationRequestFragment import im.vector.riotx.features.grouplist.GroupListFragment import im.vector.riotx.features.home.HomeDetailFragment @@ -77,7 +78,6 @@ import im.vector.riotx.features.signout.soft.SoftLogoutFragment @Module interface FragmentModule { - /** * Fragments with @IntoMap will be injected by this factory */ @@ -319,6 +319,11 @@ interface FragmentModule { @FragmentKey(VerificationEmojiCodeFragment::class) fun bindVerificationEmojiCodeFragment(fragment: VerificationEmojiCodeFragment): Fragment + @Binds + @IntoMap + @FragmentKey(VerificationQrScannedByOtherFragment::class) + fun bindVerificationQrScannedByOtherFragment(fragment: VerificationQrScannedByOtherFragment): Fragment + @Binds @IntoMap @FragmentKey(VerificationConclusionFragment::class) diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationAction.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationAction.kt index 83fdc46270..74c85a75c6 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationAction.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationAction.kt @@ -18,10 +18,13 @@ package im.vector.riotx.features.crypto.verification import im.vector.riotx.core.platform.VectorViewModelAction +// TODO Remove otherUserId and transactionId when it's not necessary. Should be known by the ViewModel, no? sealed class VerificationAction : VectorViewModelAction { data class RequestVerificationByDM(val otherUserId: String, val roomId: String?) : VerificationAction() data class StartSASVerification(val otherUserId: String, val pendingRequestTransactionId: String) : VerificationAction() data class RemoteQrCodeScanned(val otherUserId: String, val transactionId: String, val scannedData: String) : VerificationAction() + object OtherUserScannedSuccessfully : VerificationAction() + object OtherUserDidNotScanned : VerificationAction() data class SASMatchAction(val otherUserId: String, val sasTransactionId: String) : VerificationAction() data class SASDoNotMatchAction(val otherUserId: String, val sasTransactionId: String) : VerificationAction() object GotItConclusion : VerificationAction() diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheet.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheet.kt index 693b7039ff..03942fd307 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheet.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheet.kt @@ -35,10 +35,12 @@ import im.vector.matrix.android.api.session.crypto.sas.VerificationTxState import im.vector.riotx.R import im.vector.riotx.core.di.ScreenComponent import im.vector.riotx.core.extensions.commitTransactionNow +import im.vector.riotx.core.extensions.exhaustive import im.vector.riotx.core.platform.VectorBaseBottomSheetDialogFragment import im.vector.riotx.features.crypto.verification.choose.VerificationChooseMethodFragment import im.vector.riotx.features.crypto.verification.conclusion.VerificationConclusionFragment import im.vector.riotx.features.crypto.verification.emoji.VerificationEmojiCodeFragment +import im.vector.riotx.features.crypto.verification.qrconfirmation.VerificationQrScannedByOtherFragment import im.vector.riotx.features.crypto.verification.request.VerificationRequestFragment import im.vector.riotx.features.home.AvatarRenderer import kotlinx.android.parcel.Parcelize @@ -149,19 +151,23 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment() { } when (it.qrTransactionState) { - is VerificationTxState.Verified -> { + is VerificationTxState.QrScannedByOther -> { + showFragment(VerificationQrScannedByOtherFragment::class, Bundle()) + return@withState + } + is VerificationTxState.Verified -> { showFragment(VerificationConclusionFragment::class, Bundle().apply { putParcelable(MvRx.KEY_ARG, VerificationConclusionFragment.Args(true, null)) }) return@withState } - is VerificationTxState.Cancelled -> { + is VerificationTxState.Cancelled -> { showFragment(VerificationConclusionFragment::class, Bundle().apply { putParcelable(MvRx.KEY_ARG, VerificationConclusionFragment.Args(false, it.qrTransactionState.cancelCode.value)) }) return@withState } - else -> Unit + else -> Unit } // At this point there is no SAS transaction for this request diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt index 6d63f7d467..61fd2e74fa 100644 --- a/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/crypto/verification/VerificationBottomSheetViewModel.kt @@ -42,6 +42,7 @@ import im.vector.matrix.android.api.util.MatrixItem import im.vector.matrix.android.api.util.toMatrixItem import im.vector.matrix.android.internal.crypto.verification.PendingVerificationRequest import im.vector.riotx.core.di.HasScreenInjector +import im.vector.riotx.core.extensions.exhaustive import im.vector.riotx.core.platform.EmptyViewEvents import im.vector.riotx.core.platform.VectorViewModel import im.vector.riotx.core.utils.LiveEvent @@ -118,7 +119,7 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(@Assisted ini ?: session.getExistingDirectRoomWithUser(otherUserId)?.roomId when (action) { - is VerificationAction.RequestVerificationByDM -> { + is VerificationAction.RequestVerificationByDM -> { if (roomId == null) { val localID = LocalEcho.createLocalEchoId() setState { @@ -163,8 +164,9 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(@Assisted ini ) } } + Unit } - is VerificationAction.StartSASVerification -> { + is VerificationAction.StartSASVerification -> { val request = session.getVerificationService().getExistingVerificationRequest(otherUserId, action.pendingRequestTransactionId) ?: return@withState if (roomId == null) return@withState @@ -177,8 +179,9 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(@Assisted ini otherDeviceId = otherDevice ?: "", callback = null ) + Unit } - is VerificationAction.RemoteQrCodeScanned -> { + is VerificationAction.RemoteQrCodeScanned -> { val existingTransaction = session.getVerificationService() .getExistingTransaction(action.otherUserId, action.transactionId) as? QrCodeVerificationTransaction existingTransaction @@ -189,21 +192,37 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(@Assisted ini // TODO } } - is VerificationAction.SASMatchAction -> { + is VerificationAction.OtherUserScannedSuccessfully -> { + val transactionId = state.transactionId ?: return@withState + + val existingTransaction = session.getVerificationService() + .getExistingTransaction(otherUserId, transactionId) as? QrCodeVerificationTransaction + existingTransaction + ?.otherUserScannedMyQrCode() + } + is VerificationAction.OtherUserDidNotScanned -> { + val transactionId = state.transactionId ?: return@withState + + val existingTransaction = session.getVerificationService() + .getExistingTransaction(otherUserId, transactionId) as? QrCodeVerificationTransaction + existingTransaction + ?.otherUserDidNotScannedMyQrCode() + } + is VerificationAction.SASMatchAction -> { (session.getVerificationService() .getExistingTransaction(action.otherUserId, action.sasTransactionId) as? SasVerificationTransaction)?.userHasVerifiedShortCode() } - is VerificationAction.SASDoNotMatchAction -> { + is VerificationAction.SASDoNotMatchAction -> { (session.getVerificationService() .getExistingTransaction(action.otherUserId, action.sasTransactionId) as? SasVerificationTransaction) ?.shortCodeDoesNotMatch() } - is VerificationAction.GotItConclusion -> { + is VerificationAction.GotItConclusion -> { _requestLiveData.postValue(LiveEvent(Success(action))) } - } + }.exhaustive } override fun transactionCreated(tx: VerificationTransaction) { diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherController.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherController.kt new file mode 100644 index 0000000000..f775ac7941 --- /dev/null +++ b/vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherController.kt @@ -0,0 +1,76 @@ +/* + * Copyright 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.riotx.features.crypto.verification.qrconfirmation + +import com.airbnb.epoxy.EpoxyController +import im.vector.riotx.R +import im.vector.riotx.core.epoxy.dividerItem +import im.vector.riotx.core.resources.ColorProvider +import im.vector.riotx.core.resources.StringProvider +import im.vector.riotx.features.crypto.verification.epoxy.bottomSheetVerificationActionItem +import im.vector.riotx.features.crypto.verification.epoxy.bottomSheetVerificationNoticeItem +import javax.inject.Inject + +class VerificationQrScannedByOtherController @Inject constructor( + private val stringProvider: StringProvider, + private val colorProvider: ColorProvider +) : EpoxyController() { + + var listener: Listener? = null + + init { + requestModelBuild() + } + + override fun buildModels() { + bottomSheetVerificationNoticeItem { + id("notice") + notice(stringProvider.getString(R.string.qr_code_scanned_by_other_notice)) + } + + dividerItem { + id("sep0") + } + + bottomSheetVerificationActionItem { + id("confirm") + title(stringProvider.getString(R.string.qr_code_scanned_by_other_yes)) + titleColor(colorProvider.getColor(R.color.riotx_accent)) + iconRes(R.drawable.ic_check_on) + iconColor(colorProvider.getColor(R.color.riotx_accent)) + listener { listener?.onUserConfirmsQrCodeScanned() } + } + + dividerItem { + id("sep1") + } + + bottomSheetVerificationActionItem { + id("deny") + title(stringProvider.getString(R.string.qr_code_scanned_by_other_no)) + titleColor(colorProvider.getColor(R.color.vector_error_color)) + iconRes(R.drawable.ic_check_off) + iconColor(colorProvider.getColor(R.color.vector_error_color)) + listener { listener?.onUserDeniesQrCodeScanned() } + } + } + + interface Listener { + fun onUserConfirmsQrCodeScanned() + fun onUserDeniesQrCodeScanned() + } +} diff --git a/vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherFragment.kt b/vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherFragment.kt new file mode 100644 index 0000000000..14d294a27a --- /dev/null +++ b/vector/src/main/java/im/vector/riotx/features/crypto/verification/qrconfirmation/VerificationQrScannedByOtherFragment.kt @@ -0,0 +1,62 @@ +/* + * Copyright 2019 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.riotx.features.crypto.verification.qrconfirmation + +import android.os.Bundle +import android.view.View +import com.airbnb.mvrx.parentFragmentViewModel +import im.vector.riotx.R +import im.vector.riotx.core.extensions.cleanup +import im.vector.riotx.core.extensions.configureWith +import im.vector.riotx.core.platform.VectorBaseFragment +import im.vector.riotx.features.crypto.verification.VerificationAction +import im.vector.riotx.features.crypto.verification.VerificationBottomSheetViewModel +import kotlinx.android.synthetic.main.bottom_sheet_verification_child_fragment.* +import javax.inject.Inject + +class VerificationQrScannedByOtherFragment @Inject constructor( + val controller: VerificationQrScannedByOtherController +) : VectorBaseFragment(), VerificationQrScannedByOtherController.Listener { + + private val sharedViewModel by parentFragmentViewModel(VerificationBottomSheetViewModel::class) + + override fun getLayoutResId() = R.layout.bottom_sheet_verification_child_fragment + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + + setupRecyclerView() + } + + override fun onDestroyView() { + bottomSheetVerificationRecyclerView.cleanup() + controller.listener = null + super.onDestroyView() + } + + private fun setupRecyclerView() { + bottomSheetVerificationRecyclerView.configureWith(controller, hasFixedSize = false, disableItemAnimation = true) + controller.listener = this + } + + override fun onUserConfirmsQrCodeScanned() { + sharedViewModel.handle(VerificationAction.OtherUserScannedSuccessfully) + } + + override fun onUserDeniesQrCodeScanned() { + sharedViewModel.handle(VerificationAction.OtherUserDidNotScanned) + } +} diff --git a/vector/src/main/res/values/strings_riotX.xml b/vector/src/main/res/values/strings_riotX.xml index 628493397f..9aeb2b2a64 100644 --- a/vector/src/main/res/values/strings_riotX.xml +++ b/vector/src/main/res/values/strings_riotX.xml @@ -147,4 +147,8 @@ Reset Keys QR code + + Did the other user successfully scan the QR code? + Yes + No