Skip to content

Commit

Permalink
Merge pull request #6328 from vector-im/feature/bca/verif_resist_no_age
Browse files Browse the repository at this point in the history
Feature/bca/verif resist no age
  • Loading branch information
BillCarsonFr authored Jun 28, 2022
2 parents 69920a6 + 82e6847 commit a2aa047
Show file tree
Hide file tree
Showing 17 changed files with 152 additions and 18 deletions.
1 change: 1 addition & 0 deletions changelog.d/6328.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix | Some user verification requests couldn't be accepted/declined
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal class VerificationMessageProcessor @Inject constructor(
// If the request is in the future by more than 5 minutes or more than 10 minutes in the past,
// the message should be ignored by the receiver.

if (event.ageLocalTs != null && !VerificationService.isValidRequest(event.ageLocalTs, clock.epochMillis())) return Unit.also {
if (!VerificationService.isValidRequest(event.ageLocalTs, clock.epochMillis())) return Unit.also {
Timber.d("## SAS Verification live observer: msgId: ${event.eventId} is outdated age:$event.ageLocalTs ms")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ private fun HashMap<String, RoomMemberContent?>.addSenderState(realm: Realm, roo
* Create an EventEntity for the root thread event or get an existing one.
*/
private fun createEventEntity(realm: Realm, roomId: String, event: Event, currentTimeMillis: Long): EventEntity {
val ageLocalTs = event.unsignedData?.age?.let { currentTimeMillis - it }
val ageLocalTs = currentTimeMillis - (event.unsignedData?.age ?: 0)
return event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, EventInsertType.PAGINATION)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ internal fun EventEntity.asDomain(castJsonNumbers: Boolean = false): Event {
internal fun Event.toEntity(
roomId: String,
sendState: SendState,
ageLocalTs: Long?,
ageLocalTs: Long,
contentToInject: String? = null
): EventEntity {
return EventMapper.map(this, roomId).apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ internal class DefaultLoadRoomMembersTask @Inject constructor(
if (roomMemberEvent.eventId == null || roomMemberEvent.stateKey == null || roomMemberEvent.type == null) {
continue
}
val ageLocalTs = roomMemberEvent.unsignedData?.age?.let { now - it }
val ageLocalTs = now - (roomMemberEvent.unsignedData?.age ?: 0)
val eventEntity = roomMemberEvent.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, EventInsertType.PAGINATION)
CurrentStateEventEntity.getOrCreate(
realm,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ internal class DefaultFetchThreadTimelineTask @Inject constructor(
* Create an EventEntity to be added in the TimelineEventEntity.
*/
private fun createEventEntity(roomId: String, event: Event, realm: Realm): EventEntity {
val ageLocalTs = event.unsignedData?.age?.let { clock.epochMillis() - it }
val now = clock.epochMillis()
val ageLocalTs = now - (event.unsignedData?.age ?: 0)
return event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, EventInsertType.PAGINATION)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ internal class DefaultGetEventTask @Inject constructor(
}
}

event.ageLocalTs = event.unsignedData?.age?.let { clock.epochMillis() - it }
event.ageLocalTs = clock.epochMillis() - (event.unsignedData?.age ?: 0)

return event
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ internal class TokenChunkEventPersistor @Inject constructor(
val now = clock.epochMillis()

stateEvents?.forEach { stateEvent ->
val ageLocalTs = stateEvent.unsignedData?.age?.let { now - it }
val ageLocalTs = now - (stateEvent.unsignedData?.age ?: 0)
val stateEventEntity = stateEvent.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, EventInsertType.PAGINATION)
currentChunk.addStateEvent(roomId, stateEventEntity, direction)
if (stateEvent.type == EventType.STATE_ROOM_MEMBER && stateEvent.stateKey != null) {
Expand All @@ -155,7 +155,7 @@ internal class TokenChunkEventPersistor @Inject constructor(
if (event.eventId == null || event.senderId == null) {
return@forEach
}
val ageLocalTs = event.unsignedData?.age?.let { now - it }
val ageLocalTs = now - (event.unsignedData?.age ?: 0)
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, EventInsertType.PAGINATION)
if (event.type == EventType.STATE_ROOM_MEMBER && event.stateKey != null) {
val contentToUse = if (direction == PaginationDirection.BACKWARDS) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ internal class RoomSyncHandler @Inject constructor(
if (event.eventId == null || event.stateKey == null || event.type == null) {
continue
}
val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it }
val ageLocalTs = syncLocalTimestampMillis - (event.unsignedData?.age ?: 0)
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, insertType)
Timber.v("## received state event ${event.type} and key ${event.stateKey}")
CurrentStateEventEntity.getOrCreate(realm, roomId, event.stateKey, event.type).apply {
Expand Down Expand Up @@ -306,7 +306,7 @@ internal class RoomSyncHandler @Inject constructor(
if (event.stateKey == null || event.type == null) {
return@forEach
}
val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it }
val ageLocalTs = syncLocalTimestampMillis - (event.unsignedData?.age ?: 0)
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, insertType)
CurrentStateEventEntity.getOrCreate(realm, roomId, event.stateKey, event.type).apply {
eventId = eventEntity.eventId
Expand Down Expand Up @@ -336,7 +336,7 @@ internal class RoomSyncHandler @Inject constructor(
if (event.eventId == null || event.stateKey == null || event.type == null) {
continue
}
val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it }
val ageLocalTs = syncLocalTimestampMillis - (event.unsignedData?.age ?: 0)
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, insertType)
CurrentStateEventEntity.getOrCreate(realm, roomId, event.stateKey, event.type).apply {
eventId = event.eventId
Expand All @@ -348,7 +348,7 @@ internal class RoomSyncHandler @Inject constructor(
if (event.eventId == null || event.senderId == null || event.type == null) {
continue
}
val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it }
val ageLocalTs = syncLocalTimestampMillis - (event.unsignedData?.age ?: 0)
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, insertType)
if (event.stateKey != null) {
CurrentStateEventEntity.getOrCreate(realm, roomId, event.stateKey, event.type).apply {
Expand Down Expand Up @@ -401,7 +401,10 @@ internal class RoomSyncHandler @Inject constructor(
for (rawEvent in eventList) {
// It's annoying roomId is not there, but lot of code rely on it.
// And had to do it now as copy would delete all decryption results..
val event = rawEvent.copy(roomId = roomId)
val ageLocalTs = syncLocalTimestampMillis - (rawEvent.unsignedData?.age ?: 0)
val event = rawEvent.copy(roomId = roomId).also {
it.ageLocalTs = ageLocalTs
}
if (event.eventId == null || event.senderId == null || event.type == null) {
continue
}
Expand All @@ -423,7 +426,6 @@ internal class RoomSyncHandler @Inject constructor(
contentToInject = threadsAwarenessHandler.makeEventThreadAware(realm, roomId, event)
}

val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it }
val eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs, contentToInject).copyToRealmOrIgnore(realm, insertType)
if (event.stateKey != null) {
CurrentStateEventEntity.getOrCreate(realm, roomId, event.stateKey, event.type).apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import org.matrix.android.sdk.internal.session.permalinks.PermalinkFactory
import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
import org.matrix.android.sdk.internal.session.room.timeline.GetEventTask
import org.matrix.android.sdk.internal.util.awaitTransaction
import org.matrix.android.sdk.internal.util.time.Clock
import javax.inject.Inject

/**
Expand All @@ -64,7 +65,8 @@ internal class ThreadsAwarenessHandler @Inject constructor(
private val permalinkFactory: PermalinkFactory,
@SessionDatabase private val monarchy: Monarchy,
private val lightweightSettingsStorage: LightweightSettingsStorage,
private val getEventTask: GetEventTask
private val getEventTask: GetEventTask,
private val clock: Clock,
) {

// This caching is responsible to improve the performance when we receive a root event
Expand Down Expand Up @@ -120,7 +122,7 @@ internal class ThreadsAwarenessHandler @Inject constructor(
private suspend fun fetchThreadsEvents(threadsToFetch: Map<String, String>) {
val eventEntityList = threadsToFetch.mapNotNull { (eventId, roomId) ->
fetchEvent(eventId, roomId)?.let {
it.toEntity(roomId, SendState.SYNCED, it.ageLocalTs)
it.toEntity(roomId, SendState.SYNCED, it.ageLocalTs ?: clock.epochMillis())
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* 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.app.core.ui.list

import com.airbnb.epoxy.EpoxyAttribute
import com.airbnb.epoxy.EpoxyModelClass
import com.google.android.material.button.MaterialButton
import im.vector.app.R
import im.vector.app.core.epoxy.ClickListener
import im.vector.app.core.epoxy.VectorEpoxyHolder
import im.vector.app.core.epoxy.VectorEpoxyModel
import im.vector.app.core.epoxy.onClick
import im.vector.lib.core.utils.epoxy.charsequence.EpoxyCharSequence

/**
* A generic button list item.
*/
@EpoxyModelClass(layout = R.layout.item_positive_destrutive_buttons)
abstract class ButtonPositiveDestructiveButtonBarItem : VectorEpoxyModel<ButtonPositiveDestructiveButtonBarItem.Holder>() {

@EpoxyAttribute
var positiveText: EpoxyCharSequence? = null

@EpoxyAttribute
var destructiveText: EpoxyCharSequence? = null

@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash)
var positiveButtonClickAction: ClickListener? = null

@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash)
var destructiveButtonClickAction: ClickListener? = null

override fun bind(holder: Holder) {
super.bind(holder)
positiveText?.charSequence?.let { holder.positiveButton.text = it }
destructiveText?.charSequence?.let { holder.destructiveButton.text = it }

holder.positiveButton.onClick(positiveButtonClickAction)
holder.destructiveButton.onClick(destructiveButtonClickAction)
}

class Holder : VectorEpoxyHolder() {
val destructiveButton by bind<MaterialButton>(R.id.destructive_button)
val positiveButton by bind<MaterialButton>(R.id.positive_button)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ sealed class VerificationAction : VectorViewModelAction {
data class GotItConclusion(val verified: Boolean) : VerificationAction()
object SkipVerification : VerificationAction()
object VerifyFromPassphrase : VerificationAction()
object ReadyPendingVerification : VerificationAction()
object CancelPendingVerification : VerificationAction()
data class GotResultFromSsss(val cypherData: String, val alias: String) : VerificationAction()
object CancelledFromSsss : VerificationAction()
object SecuredStorageHasBeenReset : VerificationAction()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,27 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
as? SasVerificationTransaction)
?.shortCodeDoesNotMatch()
}
is VerificationAction.ReadyPendingVerification -> {
state.pendingRequest.invoke()?.let { request ->
// will only be there for dm verif
if (state.roomId != null) {
session.cryptoService().verificationService()
.readyPendingVerificationInDMs(
supportedVerificationMethodsProvider.provide(),
state.otherUserId,
state.roomId,
request.transactionId ?: ""
)
}
}
}
is VerificationAction.CancelPendingVerification -> {
state.pendingRequest.invoke()?.let {
session.cryptoService().verificationService()
.cancelVerificationRequest(it)
}
_viewEvents.post(VerificationBottomSheetViewEvents.Dismiss)
}
is VerificationAction.GotItConclusion -> {
if (state.isVerificationRequired && !action.verified) {
// we should go back to first screen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import im.vector.app.R
import im.vector.app.core.epoxy.bottomSheetDividerItem
import im.vector.app.core.resources.ColorProvider
import im.vector.app.core.resources.StringProvider
import im.vector.app.core.ui.list.buttonPositiveDestructiveButtonBarItem
import im.vector.app.features.crypto.verification.epoxy.bottomSheetVerificationActionItem
import im.vector.app.features.crypto.verification.epoxy.bottomSheetVerificationNoticeItem
import im.vector.app.features.crypto.verification.epoxy.bottomSheetVerificationQrCodeItem
Expand Down Expand Up @@ -108,6 +109,15 @@ class VerificationChooseMethodController @Inject constructor(
iconColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary))
listener { host.listener?.doVerifyBySas() }
}
} else if (!state.isReadySent) {
// a bit of a special case, if you tapped on the timeline cell but not on a button
buttonPositiveDestructiveButtonBarItem {
id("accept_decline")
positiveText(host.stringProvider.getString(R.string.action_accept).toEpoxyCharSequence())
destructiveText(host.stringProvider.getString(R.string.action_decline).toEpoxyCharSequence())
positiveButtonClickAction { host.listener?.acceptRequest() }
destructiveButtonClickAction { host.listener?.declineRequest() }
}
}

if (state.isMe && state.canCrossSign) {
Expand All @@ -131,5 +141,7 @@ class VerificationChooseMethodController @Inject constructor(
fun openCamera()
fun doVerifyBySas()
fun onClickOnWasNotMe()
fun acceptRequest()
fun declineRequest()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ class VerificationChooseMethodFragment @Inject constructor(
sharedViewModel.itWasNotMe()
}

override fun acceptRequest() {
sharedViewModel.handle(VerificationAction.ReadyPendingVerification)
}

override fun declineRequest() {
sharedViewModel.handle(VerificationAction.CancelPendingVerification)
}

private fun doOpenQRCodeScanner() {
QrCodeScannerActivity.startForResult(requireActivity(), scanActivityResultLauncher)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ data class VerificationChooseMethodViewState(
val qrCodeText: String? = null,
val sasModeAvailable: Boolean = false,
val isMe: Boolean = false,
val canCrossSign: Boolean = false
val canCrossSign: Boolean = false,
val isReadySent: Boolean = false
) : MavericksState

class VerificationChooseMethodViewModel @AssistedInject constructor(
Expand Down Expand Up @@ -81,7 +82,8 @@ class VerificationChooseMethodViewModel @AssistedInject constructor(
copy(
otherCanShowQrCode = pvr?.otherCanShowQrCode().orFalse(),
otherCanScanQrCode = pvr?.otherCanScanQrCode().orFalse(),
sasModeAvailable = pvr?.isSasSupported().orFalse()
sasModeAvailable = pvr?.isSasSupported().orFalse(),
isReadySent = pvr?.isReady.orFalse(),
)
}
}
Expand Down
24 changes: 24 additions & 0 deletions vector/src/main/res/layout/item_positive_destrutive_buttons.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center_horizontal"
android:orientation="horizontal">

<Button
android:id="@+id/destructive_button"
style="@style/Widget.Vector.Button.Destructive"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="16dp"
tools:text="@string/action_decline" />

<Button
android:id="@+id/positive_button"
style="@style/Widget.Vector.Button.Positive"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
tools:text="@string/action_accept" />

</LinearLayout>

0 comments on commit a2aa047

Please sign in to comment.