From 82af848c33fecef805fd55da1c4332ee3188b98e Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 12 Dec 2019 10:27:22 +0100 Subject: [PATCH] Fix / Verification Request Local Echo --- .../crypto/tasks/RequestVerificationDMTask.kt | 32 ++++++----- .../tasks/SendVerificationMessageTask.kt | 55 +++++++++++-------- .../DefaultSasVerificationService.kt | 3 +- .../verification/SasTransportRoomMessage.kt | 6 +- .../room/EventRelationsAggregationTask.kt | 5 +- .../factory/VerificationItemFactory.kt | 5 +- .../item/VerificationRequestConclusionItem.kt | 4 ++ .../timeline/item/VerificationRequestItem.kt | 4 ++ .../layout/item_timeline_event_base_state.xml | 11 ++++ 9 files changed, 78 insertions(+), 47 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/RequestVerificationDMTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/RequestVerificationDMTask.kt index ae8a40f296..65d552940d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/RequestVerificationDMTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/RequestVerificationDMTask.kt @@ -29,12 +29,11 @@ import javax.inject.Inject internal interface RequestVerificationDMTask : Task { data class Params( - val roomId: String, - val from: String, - val methods: List, - val to: String, + val event: Event, val cryptoService: CryptoService ) + + fun createParamsAndLocalEcho(roomId: String, from: String, methods: List, to: String, cryptoService: CryptoService) : Params } internal class DefaultRequestVerificationDMTask @Inject constructor( @@ -45,8 +44,16 @@ internal class DefaultRequestVerificationDMTask @Inject constructor( private val roomAPI: RoomAPI) : RequestVerificationDMTask { + override fun createParamsAndLocalEcho(roomId: String, from: String, methods: List, to: String, cryptoService: CryptoService): RequestVerificationDMTask.Params { + val event = localEchoEventFactory.createVerificationRequest(roomId, from, to, methods) + .also { localEchoEventFactory.saveLocalEcho(monarchy, it) } + return RequestVerificationDMTask.Params( + event, + cryptoService + ) + } override suspend fun execute(params: RequestVerificationDMTask.Params): SendResponse { - val event = createRequestEvent(params) + val event = handleEncryption(params) val localID = event.eventId!! try { @@ -54,7 +61,7 @@ internal class DefaultRequestVerificationDMTask @Inject constructor( val executeRequest = executeRequest { apiCall = roomAPI.send( localID, - roomId = params.roomId, + roomId = event.roomId ?: "", content = event.content, eventType = event.type // message or room.encrypted ) @@ -67,14 +74,13 @@ internal class DefaultRequestVerificationDMTask @Inject constructor( } } - private suspend fun createRequestEvent(params: RequestVerificationDMTask.Params): Event { - val event = localEchoEventFactory.createVerificationRequest(params.roomId, params.from, params.to, params.methods) - .also { localEchoEventFactory.saveLocalEcho(monarchy, it) } - if (params.cryptoService.isRoomEncrypted(params.roomId)) { + private suspend fun handleEncryption(params: RequestVerificationDMTask.Params): Event { + val roomId = params.event.roomId ?: "" + if (params.cryptoService.isRoomEncrypted(roomId)) { try { return encryptEventTask.execute(EncryptEventTask.Params( - params.roomId, - event, + roomId, + params.event, listOf("m.relates_to"), params.cryptoService )) @@ -82,6 +88,6 @@ internal class DefaultRequestVerificationDMTask @Inject constructor( // We said it's ok to send verification request in clear } } - return event + return params.event } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/SendVerificationMessageTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/SendVerificationMessageTask.kt index b850a1a1e6..cf1b2d233a 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/SendVerificationMessageTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/SendVerificationMessageTask.kt @@ -34,10 +34,14 @@ import javax.inject.Inject internal interface SendVerificationMessageTask : Task { data class Params( val type: String, - val roomId: String, - val content: Content, + val event: Event, val cryptoService: CryptoService? ) + + fun createParamsAndLocalEcho(type: String, + roomId: String, + content: Content, + cryptoService: CryptoService?) : Params } internal class DefaultSendVerificationMessageTask @Inject constructor( @@ -48,8 +52,28 @@ internal class DefaultSendVerificationMessageTask @Inject constructor( @UserId private val userId: String, private val roomAPI: RoomAPI) : SendVerificationMessageTask { + override fun createParamsAndLocalEcho(type: String, roomId: String, content: Content, cryptoService: CryptoService?): SendVerificationMessageTask.Params { + val localID = LocalEcho.createLocalEchoId() + val event = Event( + roomId = roomId, + originServerTs = System.currentTimeMillis(), + senderId = userId, + eventId = localID, + type = type, + content = content, + unsignedData = UnsignedData(age = null, transactionId = localID) + ).also { + localEchoEventFactory.saveLocalEcho(monarchy, it) + } + return SendVerificationMessageTask.Params( + type, + event, + cryptoService + ) + } + override suspend fun execute(params: SendVerificationMessageTask.Params): SendResponse { - val event = createRequestEvent(params) + val event = handleEncryption(params) val localID = event.eventId!! try { @@ -57,7 +81,7 @@ internal class DefaultSendVerificationMessageTask @Inject constructor( val executeRequest = executeRequest { apiCall = roomAPI.send( localID, - roomId = params.roomId, + roomId = event.roomId ?: "", content = event.content, eventType = event.type ) @@ -70,25 +94,12 @@ internal class DefaultSendVerificationMessageTask @Inject constructor( } } - private suspend fun createRequestEvent(params: SendVerificationMessageTask.Params): Event { - val localID = LocalEcho.createLocalEchoId() - val event = Event( - roomId = params.roomId, - originServerTs = System.currentTimeMillis(), - senderId = userId, - eventId = localID, - type = params.type, - content = params.content, - unsignedData = UnsignedData(age = null, transactionId = localID) - ).also { - localEchoEventFactory.saveLocalEcho(monarchy, it) - } - - if (params.cryptoService?.isRoomEncrypted(params.roomId) == true) { + private suspend fun handleEncryption(params: SendVerificationMessageTask.Params): Event { + if (params.cryptoService?.isRoomEncrypted(params.event.roomId ?: "") == true) { try { return encryptEventTask.execute(EncryptEventTask.Params( - params.roomId, - event, + params.event.roomId ?: "", + params.event, listOf("m.relates_to"), params.cryptoService )) @@ -96,6 +107,6 @@ internal class DefaultSendVerificationMessageTask @Inject constructor( // We said it's ok to send verification request in clear } } - return event + return params.event } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt index d54ae3a22a..9149f99c17 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultSasVerificationService.kt @@ -38,7 +38,6 @@ import im.vector.matrix.android.internal.crypto.model.MXUsersDevicesMap import im.vector.matrix.android.internal.crypto.model.rest.* import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore import im.vector.matrix.android.internal.crypto.tasks.DefaultRequestVerificationDMTask -import im.vector.matrix.android.internal.crypto.tasks.RequestVerificationDMTask import im.vector.matrix.android.internal.session.SessionScope import im.vector.matrix.android.internal.session.room.send.SendResponse import im.vector.matrix.android.internal.task.TaskConstraints @@ -538,7 +537,7 @@ internal class DefaultSasVerificationService @Inject constructor( override fun requestKeyVerificationInDMs(userId: String, roomId: String, callback: MatrixCallback?) { requestVerificationDMTask.configureWith( - RequestVerificationDMTask.Params( + requestVerificationDMTask.createParamsAndLocalEcho( roomId = roomId, from = credentials.deviceId ?: "", methods = listOf(KeyVerificationStart.VERIF_METHOD_SAS), diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SasTransportRoomMessage.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SasTransportRoomMessage.kt index e40e8be31f..91adbbd705 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SasTransportRoomMessage.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SasTransportRoomMessage.kt @@ -50,7 +50,7 @@ internal class SasTransportRoomMessage( Timber.d("## SAS sending msg type $type") Timber.v("## SAS sending msg info $verificationInfo") sendVerificationMessageTask.configureWith( - SendVerificationMessageTask.Params( + sendVerificationMessageTask.createParamsAndLocalEcho( type, roomId, verificationInfo.toEventContent()!!, @@ -82,7 +82,7 @@ internal class SasTransportRoomMessage( override fun cancelTransaction(transactionId: String, userId: String, userDevice: String, code: CancelCode) { Timber.d("## SAS canceling transaction $transactionId for reason $code") sendVerificationMessageTask.configureWith( - SendVerificationMessageTask.Params( + sendVerificationMessageTask.createParamsAndLocalEcho( EventType.KEY_VERIFICATION_CANCEL, roomId, MessageVerificationCancelContent.create(transactionId, code).toContent(), @@ -108,7 +108,7 @@ internal class SasTransportRoomMessage( override fun done(transactionId: String) { sendVerificationMessageTask.configureWith( - SendVerificationMessageTask.Params( + sendVerificationMessageTask.createParamsAndLocalEcho( EventType.KEY_VERIFICATION_DONE, roomId, MessageVerificationDoneContent( diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt index bdf4b400ef..4e298826b1 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/EventRelationsAggregationTask.kt @@ -18,7 +18,6 @@ package im.vector.matrix.android.internal.session.room import com.zhuinden.monarchy.Monarchy import im.vector.matrix.android.api.session.crypto.CryptoService import im.vector.matrix.android.api.session.crypto.MXCryptoError -import im.vector.matrix.android.api.session.crypto.sas.CancelCode import im.vector.matrix.android.api.session.events.model.* import im.vector.matrix.android.api.session.room.model.ReferencesAggregatedContent import im.vector.matrix.android.api.session.room.model.message.MessageContent @@ -481,14 +480,14 @@ internal class DefaultEventRelationsAggregationTask @Inject constructor( } } - private fun updateVerificationState(oldState: VerificationState?, newState: VerificationState) : VerificationState{ + private fun updateVerificationState(oldState: VerificationState?, newState: VerificationState) : VerificationState { // Cancel is always prioritary ? // Eg id i found that mac or keys mismatch and send a cancel and the other send a done, i have to // consider as canceled if (newState == VerificationState.CANCELED_BY_OTHER || newState == VerificationState.CANCELED_BY_ME) { return newState } - //never move out of cancel + // never move out of cancel if (oldState == VerificationState.CANCELED_BY_OTHER || oldState == VerificationState.CANCELED_BY_ME) { return oldState } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/VerificationItemFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/VerificationItemFactory.kt index beeb33b989..b29ed0486a 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/VerificationItemFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/VerificationItemFactory.kt @@ -26,7 +26,6 @@ import im.vector.matrix.android.api.session.room.model.message.MessageVerificati import im.vector.matrix.android.api.session.room.model.message.MessageVerificationRequestContent import im.vector.matrix.android.api.session.room.timeline.TimelineEvent import im.vector.matrix.android.internal.session.room.VerificationState -import im.vector.matrix.android.internal.session.room.isCanceled import im.vector.riotx.core.epoxy.VectorEpoxyModel import im.vector.riotx.core.resources.ColorProvider import im.vector.riotx.core.resources.UserPreferencesProvider @@ -91,7 +90,7 @@ class VerificationItemFactory @Inject constructor( CancelCode.MismatchedCommitment, CancelCode.MismatchedKeys, CancelCode.MismatchedSas -> { - //We should display these bad conclusions + // We should display these bad conclusions return VerificationRequestConclusionItem_() .attributes( VerificationRequestConclusionItem.Attributes( @@ -113,10 +112,8 @@ class VerificationItemFactory @Inject constructor( } else -> ignoredConclusion(event, highlight, callback) } - } EventType.KEY_VERIFICATION_DONE -> { - // Is the request referenced is actually really completed? if (referenceInformationData.referencesInfoData?.verificationStatus != VerificationState.DONE) return ignoredConclusion(event, highlight, callback) diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestConclusionItem.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestConclusionItem.kt index a5344e1055..036bf2b036 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestConclusionItem.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestConclusionItem.kt @@ -18,6 +18,7 @@ package im.vector.riotx.features.home.room.detail.timeline.item import android.annotation.SuppressLint import android.graphics.Typeface import android.view.View +import android.widget.ImageView import android.widget.RelativeLayout import androidx.appcompat.widget.AppCompatTextView import androidx.core.content.ContextCompat @@ -55,12 +56,15 @@ abstract class VerificationRequestConclusionItem : AbsBaseMessageItem(R.id.itemVerificationDoneTitleTextView) val descriptionView by bind(R.id.itemVerificationDoneDetailTextView) val endGuideline by bind(R.id.messageEndGuideline) + val failedToSendIndicator by bind(R.id.messageFailToSendIndicator) } companion object { diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestItem.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestItem.kt index 1a0e89fc32..7964707d3c 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestItem.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/VerificationRequestItem.kt @@ -20,6 +20,7 @@ import android.graphics.Typeface import android.view.View import android.view.ViewGroup import android.widget.Button +import android.widget.ImageView import android.widget.RelativeLayout import android.widget.TextView import androidx.appcompat.widget.AppCompatTextView @@ -110,6 +111,8 @@ abstract class VerificationRequestItem : AbsBaseMessageItem(R.id.messageEndGuideline) private val declineButton by bind