Fix / accept button was not starting the verify sheet

Was launching start sheet, because request was not known by VerificationService. Due to message observer blocked trying to download keys..
This commit is contained in:
Valere 2020-01-03 19:06:00 +01:00
parent 08ed8d4fa7
commit c2cd149299
7 changed files with 99 additions and 29 deletions

View File

@ -66,7 +66,10 @@ interface SasVerificationService {
otherDeviceId: String, otherDeviceId: String,
callback: MatrixCallback<String>?): String? callback: MatrixCallback<String>?): String?
fun readyPendingVerificationInDMs(otherUserId: String, roomId: String, transactionId: String) /**
* Returns false if the request is unknwown
*/
fun readyPendingVerificationInDMs(otherUserId: String, roomId: String, transactionId: String): Boolean
// fun transactionUpdated(tx: SasVerificationTransaction) // fun transactionUpdated(tx: SasVerificationTransaction)

View File

@ -120,6 +120,7 @@ internal class DefaultRoomVerificationUpdateTask @Inject constructor(
} }
} }
Timber.v("## SAS Verification ignoring message sent by me: ${event.eventId} type: ${event.getClearType()}")
return@forEach return@forEach
} }

View File

@ -216,7 +216,7 @@ internal class DefaultSasVerificationService @Inject constructor(
} }
} }
suspend fun onRoomRequestReceived(event: Event) { fun onRoomRequestReceived(event: Event) {
Timber.v("## SAS Verification request from ${event.senderId} in room ${event.roomId}") Timber.v("## SAS Verification request from ${event.senderId} in room ${event.roomId}")
val requestInfo = event.getClearContent().toModel<MessageVerificationRequestContent>() val requestInfo = event.getClearContent().toModel<MessageVerificationRequestContent>()
?: return ?: return
@ -228,11 +228,11 @@ internal class DefaultSasVerificationService @Inject constructor(
return return
} }
if (checkKeysAreDownloaded(senderId, requestInfo.fromDevice) == null) { // We don't want to block here
// I should ignore this, it's not for me GlobalScope.launch {
Timber.e("## SAS Verification device ${requestInfo.fromDevice} is not knwon") if (checkKeysAreDownloaded(senderId, requestInfo.fromDevice) == null) {
// TODO cancel? Timber.e("## SAS Verification device ${requestInfo.fromDevice} is not knwon")
return }
} }
// Remember this request // Remember this request
@ -249,6 +249,7 @@ internal class DefaultSasVerificationService @Inject constructor(
requestInfo = requestInfo requestInfo = requestInfo
) )
requestsForUser.add(pendingVerificationRequest) requestsForUser.add(pendingVerificationRequest)
dispatchRequestAdded(pendingVerificationRequest)
/* /*
* After the m.key.verification.ready event is sent, either party can send an m.key.verification.start event * After the m.key.verification.ready event is sent, either party can send an m.key.verification.start event
@ -389,9 +390,14 @@ internal class DefaultSasVerificationService @Inject constructor(
private suspend fun checkKeysAreDownloaded(otherUserId: String, private suspend fun checkKeysAreDownloaded(otherUserId: String,
fromDevice: String): MXUsersDevicesMap<MXDeviceInfo>? { fromDevice: String): MXUsersDevicesMap<MXDeviceInfo>? {
return try { return try {
val keys = deviceListManager.downloadKeys(listOf(otherUserId), true) var keys = deviceListManager.downloadKeys(listOf(otherUserId), false)
val deviceIds = keys.getUserDeviceIds(otherUserId) ?: return null if (keys.getUserDeviceIds(otherUserId)?.contains(fromDevice) == true) {
keys.takeIf { deviceIds.contains(fromDevice) } return keys
} else {
// force download
keys = deviceListManager.downloadKeys(listOf(otherUserId), true)
return keys.takeIf { keys.getUserDeviceIds(otherUserId)?.contains(fromDevice) == true }
}
} catch (e: Exception) { } catch (e: Exception) {
null null
} }
@ -785,16 +791,19 @@ internal class DefaultSasVerificationService @Inject constructor(
} }
} }
override fun readyPendingVerificationInDMs(otherUserId: String, roomId: String, transactionId: String) { override fun readyPendingVerificationInDMs(otherUserId: String, roomId: String, transactionId: String): Boolean {
Timber.v("## SAS readyPendingVerificationInDMs $otherUserId room:$roomId tx:$transactionId")
// Let's find the related request // Let's find the related request
getExistingVerificationRequest(otherUserId)?.find { it.transactionId == transactionId }?.let { val existingRequest = getExistingVerificationRequest(otherUserId, transactionId)
if (existingRequest != null) {
// we need to send a ready event, with matching methods // we need to send a ready event, with matching methods
val transport = sasTransportRoomMessageFactory.createTransport(credentials.userId, credentials.deviceId val transport = sasTransportRoomMessageFactory.createTransport(credentials.userId, credentials.deviceId
?: "", roomId, null) ?: "", roomId, null)
val methods = it.requestInfo?.methods?.intersect(listOf(KeyVerificationStart.VERIF_METHOD_SAS))?.toList() val methods = existingRequest.requestInfo?.methods?.intersect(listOf(KeyVerificationStart.VERIF_METHOD_SAS))?.toList()
if (methods.isNullOrEmpty()) { if (methods.isNullOrEmpty()) {
Timber.i("Cannot ready this request, no common methods found txId:$transactionId") Timber.i("Cannot ready this request, no common methods found txId:$transactionId")
return@let // TODO buttons should not be shown in this case?
return false
} }
// TODO this is not yet related to a transaction, maybe we should use another method like for cancel? // TODO this is not yet related to a transaction, maybe we should use another method like for cancel?
val readyMsg = transport.createReady(transactionId, credentials.deviceId ?: "", methods) val readyMsg = transport.createReady(transactionId, credentials.deviceId ?: "", methods)
@ -803,7 +812,12 @@ internal class DefaultSasVerificationService @Inject constructor(
CancelCode.User, CancelCode.User,
null // TODO handle error? null // TODO handle error?
) )
updatePendingRequest(it.copy(readyInfo = readyMsg)) updatePendingRequest(existingRequest.copy(readyInfo = readyMsg))
return true
} else {
Timber.e("## SAS readyPendingVerificationInDMs Verification not found")
// :/ should not be possible... unless live observer very slow
return false
} }
} }

View File

@ -45,6 +45,7 @@ import im.vector.riotx.features.home.AvatarRenderer
import im.vector.riotx.features.themes.ThemeUtils import im.vector.riotx.features.themes.ThemeUtils
import kotlinx.android.parcel.Parcelize import kotlinx.android.parcel.Parcelize
import kotlinx.android.synthetic.main.bottom_sheet_verification.* import kotlinx.android.synthetic.main.bottom_sheet_verification.*
import timber.log.Timber
import javax.inject.Inject import javax.inject.Inject
import kotlin.reflect.KClass import kotlin.reflect.KClass
@ -154,25 +155,41 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment() {
return@withState return@withState
} }
// At this point there is no transaction for this request
// Transaction has not yet started // Transaction has not yet started
if (it.pendingRequest?.cancelConclusion != null) { if (it.pendingRequest?.cancelConclusion != null) {
// The request has been declined, we should dismiss // The request has been declined, we should dismiss
dismiss() dismiss()
} else if (it.pendingRequest == null || !it.pendingRequest.isReady) { }
// We are waiting for other party to reply with ready
showFragment(VerificationRequestFragment::class, Bundle().apply { // If it's an outgoing
putParcelable(MvRx.KEY_ARG, VerificationArgs( if (it.pendingRequest == null || !it.pendingRequest.isIncoming) {
it.otherUserMxItem?.id ?: "", Timber.v("## SAS show bottom sheet for outgoing request")
it.pendingRequest?.transactionId, if (it.pendingRequest?.isReady == true) {
it.roomId)) Timber.v("## SAS show bottom sheet for outgoing and ready request")
}) // Show choose method fragment with waiting
} else if (it.pendingRequest.isReady) { showFragment(VerificationChooseMethodFragment::class, Bundle().apply {
putParcelable(MvRx.KEY_ARG, VerificationArgs(it.otherUserMxItem?.id
?: "", it.pendingRequest.transactionId))
})
} else {
// Stay on the start fragment
showFragment(VerificationRequestFragment::class, Bundle().apply {
putParcelable(MvRx.KEY_ARG, VerificationArgs(
it.otherUserMxItem?.id ?: "",
it.pendingRequest?.transactionId,
it.roomId))
})
}
} else if (it.pendingRequest.isIncoming) {
Timber.v("## SAS show bottom sheet for Incoming request")
// For incoming we can switch to choose method because ready is being sent or already sent
showFragment(VerificationChooseMethodFragment::class, Bundle().apply { showFragment(VerificationChooseMethodFragment::class, Bundle().apply {
putParcelable(MvRx.KEY_ARG, VerificationArgs(it.otherUserMxItem?.id putParcelable(MvRx.KEY_ARG, VerificationArgs(it.otherUserMxItem?.id
?: "", it.pendingRequest.transactionId)) ?: "", it.pendingRequest.transactionId))
}) })
} }
super.invalidate() super.invalidate()
} }

View File

@ -22,7 +22,10 @@ import com.airbnb.mvrx.ViewModelContext
import com.squareup.inject.assisted.Assisted import com.squareup.inject.assisted.Assisted
import com.squareup.inject.assisted.AssistedInject import com.squareup.inject.assisted.AssistedInject
import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.api.session.Session
import im.vector.matrix.android.api.session.crypto.sas.SasVerificationService
import im.vector.matrix.android.api.session.crypto.sas.SasVerificationTransaction
import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationStart import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationStart
import im.vector.matrix.android.internal.crypto.verification.PendingVerificationRequest
import im.vector.riotx.core.di.HasScreenInjector import im.vector.riotx.core.di.HasScreenInjector
import im.vector.riotx.core.platform.EmptyAction import im.vector.riotx.core.platform.EmptyAction
import im.vector.riotx.core.platform.VectorViewModel import im.vector.riotx.core.platform.VectorViewModel
@ -37,13 +40,41 @@ data class VerificationChooseMethodViewState(
class VerificationChooseMethodViewModel @AssistedInject constructor( class VerificationChooseMethodViewModel @AssistedInject constructor(
@Assisted initialState: VerificationChooseMethodViewState, @Assisted initialState: VerificationChooseMethodViewState,
private val session: Session private val session: Session
) : VectorViewModel<VerificationChooseMethodViewState, EmptyAction>(initialState) { ) : VectorViewModel<VerificationChooseMethodViewState, EmptyAction>(initialState), SasVerificationService.SasVerificationListener {
override fun transactionCreated(tx: SasVerificationTransaction) {}
override fun transactionUpdated(tx: SasVerificationTransaction) {}
override fun verificationRequestUpdated(pr: PendingVerificationRequest) = withState { state ->
val pvr = session.getSasVerificationService().getExistingVerificationRequest(state.otherUserId, state.transactionId)
val qrAvailable = pvr?.readyInfo?.methods?.contains(KeyVerificationStart.VERIF_METHOD_SCAN)
?: false
val emojiAvailable = pvr?.readyInfo?.methods?.contains(KeyVerificationStart.VERIF_METHOD_SAS)
?: false
setState {
copy(
QRModeAvailable = qrAvailable,
SASMOdeAvailable = emojiAvailable
)
}
}
@AssistedInject.Factory @AssistedInject.Factory
interface Factory { interface Factory {
fun create(initialState: VerificationChooseMethodViewState): VerificationChooseMethodViewModel fun create(initialState: VerificationChooseMethodViewState): VerificationChooseMethodViewModel
} }
init {
session.getSasVerificationService().addListener(this)
}
override fun onCleared() {
super.onCleared()
session.getSasVerificationService().removeListener(this)
}
companion object : MvRxViewModelFactory<VerificationChooseMethodViewModel, VerificationChooseMethodViewState> { companion object : MvRxViewModelFactory<VerificationChooseMethodViewModel, VerificationChooseMethodViewState> {
override fun create(viewModelContext: ViewModelContext, state: VerificationChooseMethodViewState): VerificationChooseMethodViewModel? { override fun create(viewModelContext: ViewModelContext, state: VerificationChooseMethodViewState): VerificationChooseMethodViewModel? {
val fragment: VerificationChooseMethodFragment = (viewModelContext as FragmentViewModelContext).fragment() val fragment: VerificationChooseMethodFragment = (viewModelContext as FragmentViewModelContext).fragment()

View File

@ -963,12 +963,14 @@ class RoomDetailFragment @Inject constructor(
} }
} }
is RoomDetailAction.RequestVerification -> { is RoomDetailAction.RequestVerification -> {
Timber.v("## SAS RequestVerification action")
VerificationBottomSheet.withArgs( VerificationBottomSheet.withArgs(
roomDetailArgs.roomId, roomDetailArgs.roomId,
data.userId data.userId
).show(parentFragmentManager, "REQ") ).show(parentFragmentManager, "REQ")
} }
is RoomDetailAction.AcceptVerificationRequest -> { is RoomDetailAction.AcceptVerificationRequest -> {
Timber.v("## SAS AcceptVerificationRequest action")
VerificationBottomSheet.withArgs( VerificationBottomSheet.withArgs(
roomDetailArgs.roomId, roomDetailArgs.roomId,
data.otherUserId, data.otherUserId,

View File

@ -795,9 +795,11 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro
} }
private fun handleAcceptVerification(action: RoomDetailAction.AcceptVerificationRequest) { private fun handleAcceptVerification(action: RoomDetailAction.AcceptVerificationRequest) {
session.getSasVerificationService().readyPendingVerificationInDMs(action.otherUserId, room.roomId, Timber.v("## SAS handleAcceptVerification ${action.otherUserId}, roomId:${room.roomId}, txId:${action.transactionId}")
action.transactionId) if (session.getSasVerificationService().readyPendingVerificationInDMs(action.otherUserId, room.roomId,
_requestLiveData.postValue(LiveEvent(Success(action))) action.transactionId)) {
_requestLiveData.postValue(LiveEvent(Success(action)))
}
} }
private fun handleDeclineVerification(action: RoomDetailAction.DeclineVerificationRequest) { private fun handleDeclineVerification(action: RoomDetailAction.DeclineVerificationRequest) {