From 8871390167f4ab3063000a52ba14ee8ad55852bb Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 28 Apr 2020 12:25:50 +0200 Subject: [PATCH] Code review --- .../session/room/RoomSummaryUpdater.kt | 2 +- .../action/MessageActionsEpoxyController.kt | 2 +- .../helper/MessageInformationDataFactory.kt | 95 ++++++++++--------- .../timeline/item/AbsBaseMessageItem.kt | 3 +- .../room/detail/timeline/item/NoticeItem.kt | 7 +- vector/src/main/res/values/strings_riotX.xml | 2 +- 6 files changed, 57 insertions(+), 54 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomSummaryUpdater.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomSummaryUpdater.kt index 54c8e33bce..2b21301a0f 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomSummaryUpdater.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomSummaryUpdater.kt @@ -136,7 +136,7 @@ internal class RoomSummaryUpdater @Inject constructor( roomSummaryEntity.aliases.addAll(roomAliases) roomSummaryEntity.flatAliases = roomAliases.joinToString(separator = "|", prefix = "|") roomSummaryEntity.isEncrypted = encryptionEvent != null - roomSummaryEntity.encryptionEventTs = encryptionEvent?.originServerTs ?: System.currentTimeMillis() + roomSummaryEntity.encryptionEventTs = encryptionEvent?.originServerTs roomSummaryEntity.typingUserIds.clear() roomSummaryEntity.typingUserIds.addAll(ephemeralResult?.typingUserIds.orEmpty()) diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt index aa1119b283..e77d9ec73f 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt @@ -78,7 +78,7 @@ class MessageActionsEpoxyController @Inject constructor( bottomSheetSendStateItem { id("e2e_clear") showProgress(false) - text(stringProvider.getString(R.string.unencrytped)) + text(stringProvider.getString(R.string.unencrypted)) drawableStart(R.drawable.ic_shield_warning_small) } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/MessageInformationDataFactory.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/MessageInformationDataFactory.kt index 2521e30907..6b44b9f3d3 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/MessageInformationDataFactory.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/MessageInformationDataFactory.kt @@ -22,6 +22,7 @@ import im.vector.matrix.android.api.extensions.orFalse import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.api.session.events.model.EventType import im.vector.matrix.android.api.session.events.model.toModel +import im.vector.matrix.android.api.session.room.Room import im.vector.matrix.android.api.session.room.model.ReferencesAggregatedContent import im.vector.matrix.android.api.session.room.model.message.MessageVerificationRequestContent import im.vector.matrix.android.api.session.room.timeline.TimelineEvent @@ -76,51 +77,7 @@ class MessageInformationDataFactory @Inject constructor(private val session: Ses } val room = event.root.roomId?.let { session.getRoom(it) } - val e2eDecoration: E2EDecoration - val isUserVerified = session.cryptoService().crossSigningService().getUserCrossSigningKeys(event.root.senderId ?: "")?.isTrusted() == true - if (room?.isEncrypted() == true && isUserVerified) { - val ts = room.roomSummary()?.encryptionEventTs ?: 0 - val eventTs = event.root.originServerTs ?: 0 - if (event.isEncrypted()) { - // Do not decorate failed to decrypt, or redaction (we lost sender device info) - if (event.root.getClearType() == EventType.ENCRYPTED || event.root.isRedacted()) { - e2eDecoration = E2EDecoration.NONE - } else { - val sendingDevice = event.root.content - .toModel() - ?.deviceId - ?.let { deviceId -> - session.cryptoService().getDeviceInfo(event.root.senderId ?: "", deviceId) - } - e2eDecoration = when { - sendingDevice == null -> { - // For now do not decorate this with warning - // maybe it's a deleted session - E2EDecoration.NONE - } - sendingDevice.trustLevel == null -> { - E2EDecoration.WARN_SENT_BY_UNKNOWN - } - sendingDevice.trustLevel?.isVerified().orFalse() -> { - E2EDecoration.NONE - } - else -> { - E2EDecoration.WARN_SENT_BY_UNVERIFIED - } - } - } - } else { - if (EventType.isStateEvent(event.root.type)) { - // Do not warn for state event, they are always in clear - e2eDecoration = E2EDecoration.NONE - } else { - // If event is in clear after the room enabled encryption we should warn - e2eDecoration = if (eventTs > ts) E2EDecoration.WARN_IN_CLEAR else E2EDecoration.NONE - } - } - } else { - e2eDecoration = E2EDecoration.NONE - } + val e2eDecoration = getE2EDecoration(room, event) return MessageInformationData( eventId = eventId, senderId = event.root.senderId ?: "", @@ -165,6 +122,54 @@ class MessageInformationDataFactory @Inject constructor(private val session: Ses ) } + private fun getE2EDecoration(room: Room?, event: TimelineEvent): E2EDecoration { + return if (room?.isEncrypted() == true + // is user verified + && session.cryptoService().crossSigningService().getUserCrossSigningKeys(event.root.senderId ?: "")?.isTrusted() == true) { + val ts = room.roomSummary()?.encryptionEventTs ?: 0 + val eventTs = event.root.originServerTs ?: 0 + if (event.isEncrypted()) { + // Do not decorate failed to decrypt, or redaction (we lost sender device info) + if (event.root.getClearType() == EventType.ENCRYPTED || event.root.isRedacted()) { + E2EDecoration.NONE + } else { + val sendingDevice = event.root.content + .toModel() + ?.deviceId + ?.let { deviceId -> + session.cryptoService().getDeviceInfo(event.root.senderId ?: "", deviceId) + } + when { + sendingDevice == null -> { + // For now do not decorate this with warning + // maybe it's a deleted session + E2EDecoration.NONE + } + sendingDevice.trustLevel == null -> { + E2EDecoration.WARN_SENT_BY_UNKNOWN + } + sendingDevice.trustLevel?.isVerified().orFalse() -> { + E2EDecoration.NONE + } + else -> { + E2EDecoration.WARN_SENT_BY_UNVERIFIED + } + } + } + } else { + if (EventType.isStateEvent(event.root.type)) { + // Do not warn for state event, they are always in clear + E2EDecoration.NONE + } else { + // If event is in clear after the room enabled encryption we should warn + if (eventTs > ts) E2EDecoration.WARN_IN_CLEAR else E2EDecoration.NONE + } + } + } else { + E2EDecoration.NONE + } + } + /** * Tiles type message never show the sender information (like verification request), so we should repeat it for next message * even if same sender diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/AbsBaseMessageItem.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/AbsBaseMessageItem.kt index cccee05abd..e62de05518 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/AbsBaseMessageItem.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/AbsBaseMessageItem.kt @@ -21,7 +21,6 @@ import android.view.ViewGroup import android.widget.ImageView import android.widget.TextView import androidx.annotation.IdRes -import androidx.core.content.ContextCompat import androidx.core.view.isVisible import im.vector.matrix.android.api.session.room.send.SendState import im.vector.riotx.R @@ -100,7 +99,7 @@ abstract class AbsBaseMessageItem : BaseEventItem E2EDecoration.WARN_IN_CLEAR, E2EDecoration.WARN_SENT_BY_UNVERIFIED, E2EDecoration.WARN_SENT_BY_UNKNOWN -> { - holder.e2EDecorationView.setImageDrawable(ContextCompat.getDrawable(holder.view.context, R.drawable.ic_shield_warning)) + holder.e2EDecorationView.setImageResource(R.drawable.ic_shield_warning) holder.e2EDecorationView.isVisible = true } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/NoticeItem.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/NoticeItem.kt index 02cfc57bc7..a4d5d273e7 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/NoticeItem.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/item/NoticeItem.kt @@ -19,7 +19,6 @@ package im.vector.riotx.features.home.room.detail.timeline.item import android.view.View import android.widget.ImageView import android.widget.TextView -import androidx.core.content.ContextCompat import androidx.core.view.isVisible import com.airbnb.epoxy.EpoxyAttribute import com.airbnb.epoxy.EpoxyModelClass @@ -49,13 +48,13 @@ abstract class NoticeItem : BaseEventItem() { holder.avatarImageView.onClick(attributes.avatarClickListener) when (attributes.informationData.e2eDecoration) { - E2EDecoration.NONE -> { + E2EDecoration.NONE -> { holder.e2EDecorationView.isVisible = false } E2EDecoration.WARN_IN_CLEAR, E2EDecoration.WARN_SENT_BY_UNVERIFIED, - E2EDecoration.WARN_SENT_BY_UNKNOWN -> { - holder.e2EDecorationView.setImageDrawable(ContextCompat.getDrawable(holder.view.context, R.drawable.ic_shield_warning)) + E2EDecoration.WARN_SENT_BY_UNKNOWN -> { + holder.e2EDecorationView.setImageResource(R.drawable.ic_shield_warning) holder.e2EDecorationView.isVisible = true } } diff --git a/vector/src/main/res/values/strings_riotX.xml b/vector/src/main/res/values/strings_riotX.xml index e1b989b107..a8b7ede0cd 100644 --- a/vector/src/main/res/values/strings_riotX.xml +++ b/vector/src/main/res/values/strings_riotX.xml @@ -20,7 +20,7 @@ Backup could not be decrypted with this Recovery Key: please verify that you entered the correct Recovery Key. Failed to access secure storage - Unencrypted + Unencrytped Encrypted by an unverified device