From 9c6fccab1d33a7df356cd629d856b4bc4fbdd675 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 1 Apr 2022 17:49:44 +0200 Subject: [PATCH] Suspend API: continue moving verifications --- .../QrCodeVerificationTransaction.kt | 6 +- .../SasVerificationTransaction.kt | 6 +- .../verification/VerificationTransaction.kt | 4 +- .../sdk/internal/crypto/QrCodeVerification.kt | 19 +- .../sdk/internal/crypto/SasVerification.kt | 19 +- .../sdk/internal/crypto/UserIdentities.kt | 10 +- .../IncomingVerificationRequestHandler.kt | 26 +- .../VerificationBottomSheetViewModel.kt | 225 ++++++++++-------- .../features/navigation/DefaultNavigator.kt | 26 +- 9 files changed, 192 insertions(+), 149 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/QrCodeVerificationTransaction.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/QrCodeVerificationTransaction.kt index 37855099be..3e3dac633f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/QrCodeVerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/QrCodeVerificationTransaction.kt @@ -26,15 +26,15 @@ interface QrCodeVerificationTransaction : VerificationTransaction { /** * Call when you have scan the other user QR code */ - fun userHasScannedOtherQrCode(otherQrCodeText: String) + suspend fun userHasScannedOtherQrCode(otherQrCodeText: String) /** * Call when you confirm that other user has scanned your QR code */ - fun otherUserScannedMyQrCode() + suspend fun otherUserScannedMyQrCode() /** * Call when you do not confirm that other user has scanned your QR code */ - fun otherUserDidNotScannedMyQrCode() + suspend fun otherUserDidNotScannedMyQrCode() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/SasVerificationTransaction.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/SasVerificationTransaction.kt index 2922d98a62..0b84025351 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/SasVerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/SasVerificationTransaction.kt @@ -30,9 +30,9 @@ interface SasVerificationTransaction : VerificationTransaction { * To be called by the client when the user has verified that * both short codes do match */ - fun userHasVerifiedShortCode() + suspend fun userHasVerifiedShortCode() - fun acceptVerification() + suspend fun acceptVerification() - fun shortCodeDoesNotMatch() + suspend fun shortCodeDoesNotMatch() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationTransaction.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationTransaction.kt index 4d35bc44ac..389309d915 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationTransaction.kt @@ -30,9 +30,9 @@ interface VerificationTransaction { /** * User wants to cancel the transaction */ - fun cancel() + suspend fun cancel() - fun cancel(code: CancelCode) + suspend fun cancel(code: CancelCode) fun isToDeviceTransport(): Boolean } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt index e61c5c8f8f..8afa9d7ef9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt @@ -17,7 +17,6 @@ package org.matrix.android.sdk.internal.crypto import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import org.matrix.android.sdk.api.session.crypto.verification.CancelCode import org.matrix.android.sdk.api.session.crypto.verification.QrCodeVerificationTransaction @@ -67,20 +66,18 @@ internal class QrCodeVerification( } /** Pass the data from a scanned QR code into the QR code verification object */ - override fun userHasScannedOtherQrCode(otherQrCodeText: String) { - runBlocking { + override suspend fun userHasScannedOtherQrCode(otherQrCodeText: String) { request.scanQrCode(otherQrCodeText) - } dispatchTxUpdated() } /** Confirm that the other side has indeed scanned the QR code we presented */ - override fun otherUserScannedMyQrCode() { - runBlocking { confirm() } + override suspend fun otherUserScannedMyQrCode() { + confirm() } /** Cancel the QR code verification, denying that the other side has scanned the QR code */ - override fun otherUserDidNotScannedMyQrCode() { + override suspend fun otherUserDidNotScannedMyQrCode() { // TODO Is this code correct here? The old code seems to do this cancelHelper(CancelCode.MismatchedKeys) } @@ -140,7 +137,7 @@ internal class QrCodeVerification( * * The method turns into a noop, if the verification flow has already been cancelled. * */ - override fun cancel() { + override suspend fun cancel() { cancelHelper(CancelCode.User) } @@ -155,7 +152,7 @@ internal class QrCodeVerification( * * @param code The cancel code that should be given as the reason for the cancellation. * */ - override fun cancel(code: CancelCode) { + override suspend fun cancel(code: CancelCode) { cancelHelper(code) } @@ -190,11 +187,11 @@ internal class QrCodeVerification( } } - private fun cancelHelper(code: CancelCode) { + private suspend fun cancelHelper(code: CancelCode) { val request = this.machine.cancelVerification(this.request.otherUser(), this.request.flowId(), code.value) if (request != null) { - runBlocking { sender.sendVerificationRequest(request) } + sender.sendVerificationRequest(request) dispatchTxUpdated() } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt index 11a00c3b64..ac775af10e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt @@ -17,7 +17,6 @@ package org.matrix.android.sdk.internal.crypto import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import org.matrix.android.sdk.api.session.crypto.verification.CancelCode import org.matrix.android.sdk.api.session.crypto.verification.EmojiRepresentation @@ -95,7 +94,7 @@ internal class SasVerification( * * The method turns into a noop, if the verification flow has already been cancelled. * */ - override fun cancel() { + override suspend fun cancel() { this.cancelHelper(CancelCode.User) } @@ -110,7 +109,7 @@ internal class SasVerification( * * @param code The cancel code that should be given as the reason for the cancellation. * */ - override fun cancel(code: CancelCode) { + override suspend fun cancel(code: CancelCode) { this.cancelHelper(code) } @@ -123,7 +122,7 @@ internal class SasVerification( * * The method turns into a noop, if the verification flow has already been cancelled. */ - override fun shortCodeDoesNotMatch() { + override suspend fun shortCodeDoesNotMatch() { this.cancelHelper(CancelCode.MismatchedSas) } @@ -153,8 +152,8 @@ internal class SasVerification( * This method is a noop if we're not yet in a presentable state, i.e. we didn't receive * a m.key.verification.key event from the other side or we're cancelled. */ - override fun userHasVerifiedShortCode() { - runBlocking { confirm() } + override suspend fun userHasVerifiedShortCode() { + confirm() } /** Accept the verification flow, signaling the other side that we do want to verify @@ -165,8 +164,8 @@ internal class SasVerification( * This method is a noop if we send the start event out or if the verification has already * been accepted. */ - override fun acceptVerification() { - runBlocking { accept() } + override suspend fun acceptVerification() { + accept() } /** Get the decimal representation of the short auth string @@ -220,11 +219,11 @@ internal class SasVerification( } } - private fun cancelHelper(code: CancelCode) { + private suspend fun cancelHelper(code: CancelCode) { val request = this.machine.cancelVerification(this.inner.otherUserId, inner.flowId, code.value) if (request != null) { - runBlocking { sender.sendVerificationRequest(request) } + sender.sendVerificationRequest(request) dispatchTxUpdated() } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt index 733d4a5e1f..00c18abe0e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/UserIdentities.kt @@ -65,7 +65,7 @@ sealed class UserIdentities { /** * Convert the identity into a MxCrossSigningInfo class. */ - abstract fun toMxCrossSigningInfo(): MXCrossSigningInfo + abstract suspend fun toMxCrossSigningInfo(): MXCrossSigningInfo } /** @@ -147,11 +147,11 @@ internal class OwnUserIdentity( /** * Convert the identity into a MxCrossSigningInfo class. */ - override fun toMxCrossSigningInfo(): MXCrossSigningInfo { + override suspend fun toMxCrossSigningInfo(): MXCrossSigningInfo { val masterKey = this.masterKey val selfSigningKey = this.selfSigningKey val userSigningKey = this.userSigningKey - val trustLevel = DeviceTrustLevel(runBlocking { verified() }, false) + val trustLevel = DeviceTrustLevel(verified(), false) // TODO remove this, this is silly, we have way too many methods to check if a user is verified masterKey.trustLevel = trustLevel selfSigningKey.trustLevel = trustLevel @@ -249,9 +249,9 @@ internal class UserIdentity( /** * Convert the identity into a MxCrossSigningInfo class. */ - override fun toMxCrossSigningInfo(): MXCrossSigningInfo { + override suspend fun toMxCrossSigningInfo(): MXCrossSigningInfo { // val crossSigningKeys = listOf(this.masterKey, this.selfSigningKey) - val trustLevel = DeviceTrustLevel(runBlocking { verified() }, false) + val trustLevel = DeviceTrustLevel(verified(), false) // TODO remove this, this is silly, we have way too many methods to check if a user is verified masterKey.trustLevel = trustLevel selfSigningKey.trustLevel = trustLevel diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt index aa97d5e078..14d4b9c5d9 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt @@ -91,12 +91,14 @@ class IncomingVerificationRequestHandler @Inject constructor( it.navigator.performDeviceVerification(it, tx.otherUserId, tx.transactionId) } } - dismissedAction = Runnable { + dismissedAction = LaunchCoroutineRunnable(coroutineScope) { tx.cancel() } addButton( context.getString(R.string.action_ignore), - { tx.cancel() } + LaunchCoroutineRunnable(coroutineScope) { + tx.cancel() + } ) addButton( context.getString(R.string.action_open), @@ -163,13 +165,11 @@ class IncomingVerificationRequestHandler @Inject constructor( } } } - dismissedAction = Runnable { - coroutineScope.launch { - session?.cryptoService()?.verificationService()?.declineVerificationRequestInDMs(pr.otherUserId, - pr.transactionId ?: "", - pr.roomId ?: "" - ) - } + dismissedAction = LaunchCoroutineRunnable(coroutineScope) { + session?.cryptoService()?.verificationService()?.declineVerificationRequestInDMs(pr.otherUserId, + pr.transactionId ?: "", + pr.roomId ?: "" + ) } colorAttribute = R.attr.vctr_notice_secondary // 5mn expiration @@ -186,6 +186,14 @@ class IncomingVerificationRequestHandler @Inject constructor( } } + private class LaunchCoroutineRunnable(private val coroutineScope: CoroutineScope, private val block: suspend () -> Unit) : Runnable { + override fun run() { + coroutineScope.launch { + block() + } + } + } + private fun uniqueIdForVerificationRequest(pr: PendingVerificationRequest) = "verificationRequest_${pr.transactionId}" } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt index f38fd53950..03c87bbaff 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt @@ -239,61 +239,22 @@ class VerificationBottomSheetViewModel @AssistedInject constructor( handleRequestVerificationByDM(roomId, otherUserId) } is VerificationAction.StartSASVerification -> { - val request = session.cryptoService().verificationService().getExistingVerificationRequest(otherUserId, action.pendingRequestTransactionId) - ?: return@withState - val otherDevice = if (request.isIncoming) request.requestInfo?.fromDevice else request.readyInfo?.fromDevice - viewModelScope.launch { - if (roomId == null) { - session.cryptoService().verificationService().beginKeyVerification( - VerificationMethod.SAS, - otherUserId = request.otherUserId, - otherDeviceId = otherDevice ?: "", - transactionId = action.pendingRequestTransactionId - ) - } else { - session.cryptoService().verificationService().beginKeyVerificationInDMs( - VerificationMethod.SAS, - transactionId = action.pendingRequestTransactionId, - roomId = roomId, - otherUserId = request.otherUserId, - otherDeviceId = otherDevice ?: "" - ) - } - } - Unit + handleStartSASVerification(roomId, otherUserId, action) } is VerificationAction.RemoteQrCodeScanned -> { - val existingTransaction = session.cryptoService().verificationService() - .getExistingTransaction(action.otherUserId, action.transactionId) as? QrCodeVerificationTransaction - existingTransaction - ?.userHasScannedOtherQrCode(action.scannedData) + handleRemoteQrCodeScanned(action) } is VerificationAction.OtherUserScannedSuccessfully -> { - val transactionId = state.transactionId ?: return@withState - - val existingTransaction = session.cryptoService().verificationService() - .getExistingTransaction(otherUserId, transactionId) as? QrCodeVerificationTransaction - existingTransaction - ?.otherUserScannedMyQrCode() + handleOtherUserScannedSuccessfully(state.transactionId, otherUserId) } is VerificationAction.OtherUserDidNotScanned -> { - val transactionId = state.transactionId ?: return@withState - - val existingTransaction = session.cryptoService().verificationService() - .getExistingTransaction(otherUserId, transactionId) as? QrCodeVerificationTransaction - existingTransaction - ?.otherUserDidNotScannedMyQrCode() + handleOtherUserDidNotScanned(state.transactionId, otherUserId) } is VerificationAction.SASMatchAction -> { - (session.cryptoService().verificationService() - .getExistingTransaction(action.otherUserId, action.sasTransactionId) - as? SasVerificationTransaction)?.userHasVerifiedShortCode() + handleSASMatchAction(action) } is VerificationAction.SASDoNotMatchAction -> { - (session.cryptoService().verificationService() - .getExistingTransaction(action.otherUserId, action.sasTransactionId) - as? SasVerificationTransaction) - ?.shortCodeDoesNotMatch() + handleSASDoNotMatchAction(action) } is VerificationAction.GotItConclusion -> { _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) @@ -324,6 +285,76 @@ class VerificationBottomSheetViewModel @AssistedInject constructor( }.exhaustive } + private fun handleStartSASVerification(roomId: String?, otherUserId: String, action: VerificationAction.StartSASVerification) { + val request = session.cryptoService().verificationService().getExistingVerificationRequest(otherUserId, action.pendingRequestTransactionId) + ?: return + val otherDevice = if (request.isIncoming) request.requestInfo?.fromDevice else request.readyInfo?.fromDevice + viewModelScope.launch { + if (roomId == null) { + session.cryptoService().verificationService().beginKeyVerification( + VerificationMethod.SAS, + otherUserId = request.otherUserId, + otherDeviceId = otherDevice ?: "", + transactionId = action.pendingRequestTransactionId + ) + } else { + session.cryptoService().verificationService().beginKeyVerificationInDMs( + VerificationMethod.SAS, + transactionId = action.pendingRequestTransactionId, + roomId = roomId, + otherUserId = request.otherUserId, + otherDeviceId = otherDevice ?: "" + ) + } + } + } + + private fun handleSASDoNotMatchAction(action: VerificationAction.SASDoNotMatchAction) { + viewModelScope.launch { + (session.cryptoService().verificationService() + .getExistingTransaction(action.otherUserId, action.sasTransactionId) + as? SasVerificationTransaction) + ?.shortCodeDoesNotMatch() + } + } + + private fun handleSASMatchAction(action: VerificationAction.SASMatchAction) { + viewModelScope.launch { + (session.cryptoService().verificationService() + .getExistingTransaction(action.otherUserId, action.sasTransactionId) + as? SasVerificationTransaction)?.userHasVerifiedShortCode() + } + } + + private fun handleOtherUserDidNotScanned(transactionId: String?, otherUserId: String) { + transactionId ?: return + viewModelScope.launch { + val existingTransaction = session.cryptoService().verificationService() + .getExistingTransaction(otherUserId, transactionId) as? QrCodeVerificationTransaction + existingTransaction + ?.otherUserDidNotScannedMyQrCode() + } + } + + private fun handleOtherUserScannedSuccessfully(transactionId: String?, otherUserId: String) { + transactionId ?: return + viewModelScope.launch { + val existingTransaction = session.cryptoService().verificationService() + .getExistingTransaction(otherUserId, transactionId) as? QrCodeVerificationTransaction + existingTransaction + ?.otherUserScannedMyQrCode() + } + } + + private fun handleRemoteQrCodeScanned(action: VerificationAction.RemoteQrCodeScanned) { + viewModelScope.launch { + val existingTransaction = session.cryptoService().verificationService() + .getExistingTransaction(action.otherUserId, action.transactionId) as? QrCodeVerificationTransaction + existingTransaction + ?.userHasScannedOtherQrCode(action.scannedData) + } + } + private fun handleRequestVerificationByDM(roomId: String?, otherUserId: String) { viewModelScope.launch { if (roomId == null) { @@ -449,60 +480,66 @@ class VerificationBottomSheetViewModel @AssistedInject constructor( } } + private fun handleTransactionUpdate(state: VerificationBottomSheetViewState, tx: VerificationTransaction){ + viewModelScope.launch { + if (state.selfVerificationMode && state.transactionId == null) { + // is this an incoming with that user + if (tx.isIncoming && tx.otherUserId == state.otherUserMxItem?.id) { + // Also auto accept incoming if needed! + // TODO is state.transactionId ever null for self verifications, doesn't seem + // like this will ever trigger + if (tx is SasVerificationTransaction && tx.state == VerificationTxState.OnStarted) { + tx.acceptVerification() + } + /* + if (tx is IncomingSasVerificationTransaction) { + if (tx.uxState == IncomingSasVerificationTransaction.UxState.SHOW_ACCEPT) { + tx.performAccept() + } + } + */ + // Use this one! + setState { + copy( + transactionId = tx.transactionId, + sasTransactionState = tx.state.takeIf { tx is SasVerificationTransaction }, + qrTransactionState = tx.state.takeIf { tx is QrCodeVerificationTransaction } + ) + } + } + } + + when (tx) { + is SasVerificationTransaction -> { + if (tx.transactionId == (state.pendingRequest.invoke()?.transactionId ?: state.transactionId)) { + // A SAS tx has been started following this request + setState { + copy( + sasTransactionState = tx.state + ) + } + } + } + is QrCodeVerificationTransaction -> { + if (tx.transactionId == (state.pendingRequest.invoke()?.transactionId ?: state.transactionId)) { + // A QR tx has been started following this request + setState { + copy( + qrTransactionState = tx.state + ) + } + } + } + } + } + } + override fun transactionCreated(tx: VerificationTransaction) { transactionUpdated(tx) } override fun transactionUpdated(tx: VerificationTransaction) = withState { state -> - if (state.selfVerificationMode && state.transactionId == null) { - // is this an incoming with that user - if (tx.isIncoming && tx.otherUserId == state.otherUserMxItem?.id) { - // Also auto accept incoming if needed! - // TODO is state.transactionId ever null for self verifications, doesn't seem - // like this will ever trigger - if (tx is SasVerificationTransaction && tx.state == VerificationTxState.OnStarted) { - tx.acceptVerification() - } - /* - if (tx is IncomingSasVerificationTransaction) { - if (tx.uxState == IncomingSasVerificationTransaction.UxState.SHOW_ACCEPT) { - tx.performAccept() - } - } - */ - // Use this one! - setState { - copy( - transactionId = tx.transactionId, - sasTransactionState = tx.state.takeIf { tx is SasVerificationTransaction }, - qrTransactionState = tx.state.takeIf { tx is QrCodeVerificationTransaction } - ) - } - } - } - - when (tx) { - is SasVerificationTransaction -> { - if (tx.transactionId == (state.pendingRequest.invoke()?.transactionId ?: state.transactionId)) { - // A SAS tx has been started following this request - setState { - copy( - sasTransactionState = tx.state - ) - } - } - } - is QrCodeVerificationTransaction -> { - if (tx.transactionId == (state.pendingRequest.invoke()?.transactionId ?: state.transactionId)) { - // A QR tx has been started following this request - setState { - copy( - qrTransactionState = tx.state - ) - } - } - } - } + handleTransactionUpdate(state, tx) } override fun verificationRequestCreated(pr: PendingVerificationRequest) { diff --git a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt index 2c1e373075..ee101be732 100644 --- a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt +++ b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt @@ -196,19 +196,21 @@ class DefaultNavigator @Inject constructor( } override fun performDeviceVerification(context: Context, otherUserId: String, sasTransactionId: String) { - val session = sessionHolder.getSafeActiveSession() ?: return - val tx = session.cryptoService().verificationService().getExistingTransaction(otherUserId, sasTransactionId) - ?: return - if (tx is SasVerificationTransaction && tx.isIncoming) { - tx.acceptVerification() - } + coroutineScope.launch { + val session = sessionHolder.getSafeActiveSession() ?: return@launch + val tx = session.cryptoService().verificationService().getExistingTransaction(otherUserId, sasTransactionId) + ?: return@launch + if (tx is SasVerificationTransaction && tx.isIncoming) { + tx.acceptVerification() + } - if (context is AppCompatActivity) { - VerificationBottomSheet.withArgs( - roomId = null, - otherUserId = otherUserId, - transactionId = sasTransactionId - ).show(context.supportFragmentManager, "REQPOP") + if (context is AppCompatActivity) { + VerificationBottomSheet.withArgs( + roomId = null, + otherUserId = otherUserId, + transactionId = sasTransactionId + ).show(context.supportFragmentManager, "REQPOP") + } } }