From a623395585744c0826421ffe54f506741af1bda0 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 8 Feb 2021 17:04:50 +0300 Subject: [PATCH] Code review fixes. --- .../android/sdk/api/MatrixConfiguration.kt | 1 - .../session/room/crypto/RoomCryptoService.kt | 7 ++++ .../internal/crypto/DefaultCryptoService.kt | 42 ++++++++++++------- .../crypto/algorithms/IMXGroupEncryption.kt | 7 ---- .../algorithms/megolm/MXMegolmEncryption.kt | 9 ++-- .../sdk/internal/session/room/DefaultRoom.kt | 4 ++ .../session/room/DefaultRoomService.kt | 23 +--------- .../session/room/timeline/DefaultTimeline.kt | 13 +++++- .../room/typing/DefaultTypingService.kt | 15 +------ vector/build.gradle | 2 +- .../java/im/vector/app/VectorApplication.kt | 2 +- .../home/room/detail/RoomDetailViewModel.kt | 11 +++++ 12 files changed, 68 insertions(+), 68 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt index c01aeab6dd..9f651a21fa 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt @@ -17,7 +17,6 @@ package org.matrix.android.sdk.api import org.matrix.android.sdk.api.crypto.MXCryptoConfig -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy import java.net.Proxy data class MatrixConfiguration( diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt index 1251fd9857..bb157ec7ee 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt @@ -30,4 +30,11 @@ interface RoomCryptoService { * Enable encryption of the room */ suspend fun enableEncryption(algorithm: String = MXCRYPTO_ALGORITHM_MEGOLM) + + /** + * Ensures all members of the room are loaded and outbound session keys are shared. + * Call this method according to [OutboundSessionKeySharingStrategy]. + * If this method is not called, CryptoService will ensure it before sending events. + */ + fun ensureOutboundSession() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 72cf8b261d..1a90377664 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -50,6 +50,7 @@ import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibility import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibilityContent import org.matrix.android.sdk.api.session.room.model.RoomMemberContent +import org.matrix.android.sdk.api.util.emptyJsonDict import org.matrix.android.sdk.internal.crypto.actions.MegolmSessionDataImporter import org.matrix.android.sdk.internal.crypto.actions.SetDeviceVerificationAction import org.matrix.android.sdk.internal.crypto.algorithms.IMXEncrypting @@ -709,7 +710,7 @@ internal class DefaultCryptoService @Inject constructor( */ @Throws(MXCryptoError::class) private fun internalDecryptEvent(event: Event, timeline: String): MXEventDecryptionResult { - return eventDecryptor.decryptEvent(event, timeline) + return eventDecryptor.decryptEvent(event, timeline) } /** @@ -1297,22 +1298,31 @@ internal class DefaultCryptoService @Inject constructor( } override fun ensureOutboundSession(roomId: String) { + // Ensure to load all room members + loadRoomMembersTask + .configureWith(LoadRoomMembersTask.Params(roomId)) { + this.callback = NoOpMatrixCallback() + } + .executeBy(taskExecutor) + cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - roomEncryptorsStore - .get(roomId) - ?.let { - getEncryptionAlgorithm(roomId)?.let { safeAlgorithm -> - val userIds = getRoomUserIds(roomId) - if (setEncryptionInRoom(roomId, safeAlgorithm, false, userIds)) { - val roomEncryptor = roomEncryptorsStore.get(roomId) - if (roomEncryptor is IMXGroupEncryption) { - roomEncryptor.ensureOutboundSession(getRoomUserIds(roomId)) - } else { - Timber.e("## CRYPTO | ensureOutboundSession() for:$roomId: Unable to handle IMXGroupEncryption for $safeAlgorithm") - } - } - } - } + val userIds = getRoomUserIds(roomId) + val alg = roomEncryptorsStore.get(roomId) + ?: getEncryptionAlgorithm(roomId) + ?.let { setEncryptionInRoom(roomId, it, false, userIds) } + ?.let { roomEncryptorsStore.get(roomId) } + + if (alg == null) { + val reason = String.format(MXCryptoError.UNABLE_TO_ENCRYPT_REASON, MXCryptoError.NO_MORE_ALGORITHM_REASON) + Timber.e("## CRYPTO | encryptEventContent() : $reason") + return@launch + } + + runCatching { + alg.encryptEventContent(emptyJsonDict, EventType.DUMMY, userIds) + }.onFailure { + Timber.e("## CRYPTO | encryptEventContent() failed.") + } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt index 97b005659d..2d0cf4bc01 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt @@ -47,11 +47,4 @@ internal interface IMXGroupEncryption { userId: String, deviceId: String, senderKey: String): Boolean - - /** - * Ensure the outbound session - * - * @param usersInRoom the users in the room - */ - suspend fun ensureOutboundSession(usersInRoom: List) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt index be85fcec8e..db4f7279c6 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt @@ -138,10 +138,11 @@ internal class MXMegolmEncryption( return MXOutboundSessionInfo(sessionId, SharedWithHelper(roomId, sessionId, cryptoStore)) } - override suspend fun ensureOutboundSession(usersInRoom: List) { - ensureOutboundSession(getDevicesInRoom(usersInRoom).allowedDevices) - } - + /** + * Ensure the outbound session + * + * @param devicesInRoom the devices list + */ private suspend fun ensureOutboundSession(devicesInRoom: MXUsersDevicesMap): MXOutboundSessionInfo { Timber.v("## CRYPTO | ensureOutboundSession start") var session = outboundSession diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt index 7a819250cf..a2b7a37b9b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt @@ -104,6 +104,10 @@ internal class DefaultRoom @Inject constructor(override val roomId: String, return cryptoService.shouldEncryptForInvitedMembers(roomId) } + override fun ensureOutboundSession() { + cryptoService.ensureOutboundSession(roomId) + } + override suspend fun enableEncryption(algorithm: String) { when { isEncrypted() -> { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt index c60150004f..383dd876d3 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt @@ -20,11 +20,6 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.Transformations import com.zhuinden.monarchy.Monarchy import org.matrix.android.sdk.api.MatrixCallback -import org.matrix.android.sdk.api.MatrixConfiguration -import org.matrix.android.sdk.api.NoOpMatrixCallback -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy -import org.matrix.android.sdk.api.extensions.orFalse -import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.room.Room import org.matrix.android.sdk.api.session.room.RoomService @@ -44,7 +39,6 @@ import org.matrix.android.sdk.internal.session.room.alias.DeleteRoomAliasTask import org.matrix.android.sdk.internal.session.room.alias.GetRoomIdByAliasTask import org.matrix.android.sdk.internal.session.room.alias.RoomAliasDescription import org.matrix.android.sdk.internal.session.room.create.CreateRoomTask -import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask import org.matrix.android.sdk.internal.session.room.membership.RoomChangeMembershipStateDataSource import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper import org.matrix.android.sdk.internal.session.room.membership.joining.JoinRoomTask @@ -71,10 +65,7 @@ internal class DefaultRoomService @Inject constructor( private val roomGetter: RoomGetter, private val roomSummaryDataSource: RoomSummaryDataSource, private val roomChangeMembershipStateDataSource: RoomChangeMembershipStateDataSource, - private val taskExecutor: TaskExecutor, - private val loadRoomMembersTask: LoadRoomMembersTask, - private val matrixConfiguration: MatrixConfiguration, - private val cryptoService: CryptoService + private val taskExecutor: TaskExecutor ) : RoomService { override fun createRoom(createRoomParams: CreateRoomParams, callback: MatrixCallback): Cancelable { @@ -114,18 +105,6 @@ internal class DefaultRoomService @Inject constructor( } override fun onRoomDisplayed(roomId: String): Cancelable { - // Ensure to load all room members - loadRoomMembersTask - .configureWith(LoadRoomMembersTask.Params(roomId)) { - this.callback = NoOpMatrixCallback() - } - .executeBy(taskExecutor) - - // Ensure to share the outbound session keys with all members - if (roomGetter.getRoom(roomId)?.isEncrypted().orFalse() - && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { - cryptoService.ensureOutboundSession(roomId) - } return updateBreadcrumbsTask .configureWith(UpdateBreadcrumbsTask.Params(roomId)) .executeBy(taskExecutor) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt index 1be5d55cad..ca272a3520 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt @@ -24,6 +24,7 @@ import io.realm.RealmQuery import io.realm.RealmResults import io.realm.Sort import org.matrix.android.sdk.api.MatrixCallback +import org.matrix.android.sdk.api.NoOpMatrixCallback import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.events.model.EventType @@ -49,6 +50,7 @@ import org.matrix.android.sdk.internal.database.query.filterEvents import org.matrix.android.sdk.internal.database.query.findAllInRoomWithSendStates import org.matrix.android.sdk.internal.database.query.where import org.matrix.android.sdk.internal.database.query.whereRoomId +import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith import org.matrix.android.sdk.internal.util.Debouncer @@ -77,8 +79,9 @@ internal class DefaultTimeline( private val hiddenReadReceipts: TimelineHiddenReadReceipts, private val timelineInput: TimelineInput, private val eventDecryptor: TimelineEventDecryptor, - private val realmSessionProvider: RealmSessionProvider - ) : Timeline, + private val realmSessionProvider: RealmSessionProvider, + private val loadRoomMembersTask: LoadRoomMembersTask +) : Timeline, TimelineHiddenReadReceipts.Delegate, TimelineInput.Listener { @@ -179,6 +182,12 @@ internal class DefaultTimeline( hiddenReadReceipts.start(realm, filteredEvents, nonFilteredEvents, this) } + loadRoomMembersTask + .configureWith(LoadRoomMembersTask.Params(roomId)) { + this.callback = NoOpMatrixCallback() + } + .executeBy(taskExecutor) + isReady.set(true) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt index 279ab3f334..39b7967bc1 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt @@ -21,13 +21,8 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import dagger.assisted.AssistedFactory import org.matrix.android.sdk.api.MatrixCallback -import org.matrix.android.sdk.api.MatrixConfiguration -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy -import org.matrix.android.sdk.api.extensions.orFalse -import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.room.typing.TypingService import org.matrix.android.sdk.api.util.Cancelable -import org.matrix.android.sdk.internal.session.room.RoomGetter import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith import timber.log.Timber @@ -41,10 +36,7 @@ import timber.log.Timber internal class DefaultTypingService @AssistedInject constructor( @Assisted private val roomId: String, private val taskExecutor: TaskExecutor, - private val sendTypingTask: SendTypingTask, - private val matrixConfiguration: MatrixConfiguration, - private val cryptoService: CryptoService, - private val roomGetter: RoomGetter + private val sendTypingTask: SendTypingTask ) : TypingService { @AssistedFactory @@ -77,11 +69,6 @@ internal class DefaultTypingService @AssistedInject constructor( currentTask?.cancel() - if (roomGetter.getRoom(roomId)?.isEncrypted().orFalse() - && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { - cryptoService.ensureOutboundSession(roomId) - } - val params = SendTypingTask.Params(roomId, true) currentTask = sendTypingTask .configureWith(params) diff --git a/vector/build.gradle b/vector/build.gradle index a257f6aaf6..c539ebb648 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -136,7 +136,7 @@ android { buildConfigField "String", "BUILD_NUMBER", "\"${buildNumber}\"" resValue "string", "build_number", "\"${buildNumber}\"" - buildConfigField "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy", "OutboundSessionKeySharingStrategy", "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy.WhenTyping" + buildConfigField "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy", "outboundSessionKeySharingStrategy", "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy.WhenTyping" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" diff --git a/vector/src/main/java/im/vector/app/VectorApplication.kt b/vector/src/main/java/im/vector/app/VectorApplication.kt index 1926a35c9c..dd94be77ba 100644 --- a/vector/src/main/java/im/vector/app/VectorApplication.kt +++ b/vector/src/main/java/im/vector/app/VectorApplication.kt @@ -205,7 +205,7 @@ class VectorApplication : } } - override fun providesMatrixConfiguration() = MatrixConfiguration(applicationFlavor = BuildConfig.FLAVOR_DESCRIPTION, outboundSessionKeySharingStrategy = BuildConfig.OutboundSessionKeySharingStrategy) + override fun providesMatrixConfiguration() = MatrixConfiguration(applicationFlavor = BuildConfig.FLAVOR_DESCRIPTION) override fun getWorkManagerConfiguration(): WorkConfiguration { return WorkConfiguration.Builder() diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt index 601891b15a..099f2eefff 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt @@ -28,6 +28,7 @@ import com.jakewharton.rxrelay2.PublishRelay import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject +import im.vector.app.BuildConfig import im.vector.app.R import im.vector.app.core.extensions.exhaustive import im.vector.app.core.platform.VectorViewModel @@ -60,6 +61,7 @@ import org.commonmark.renderer.html.HtmlRenderer import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.MatrixPatterns import org.matrix.android.sdk.api.NoOpMatrixCallback +import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.raw.RawService @@ -179,6 +181,11 @@ class RoomDetailViewModel @AssistedInject constructor( callManager.addPstnSupportListener(this) callManager.checkForPSTNSupportIfNeeded() chatEffectManager.delegate = this + + // Ensure to share the outbound session keys with all members + if (room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { + room.ensureOutboundSession() + } } private fun observePowerLevel() { @@ -591,6 +598,10 @@ class RoomDetailViewModel @AssistedInject constructor( room.userStopsTyping() } } + // Ensure outbound session keys + if (room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { + room.ensureOutboundSession() + } } private fun handleTombstoneEvent(action: RoomDetailAction.HandleTombstoneEvent) {