Code review fixes.

This commit is contained in:
Onuray Sahin 2021-02-08 17:04:50 +03:00 committed by Benoit Marty
parent a2de80091d
commit a623395585
12 changed files with 68 additions and 68 deletions

View File

@ -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(

View File

@ -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()
}

View File

@ -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.")
}
}
}

View File

@ -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<String>)
}

View File

@ -138,10 +138,11 @@ internal class MXMegolmEncryption(
return MXOutboundSessionInfo(sessionId, SharedWithHelper(roomId, sessionId, cryptoStore))
}
override suspend fun ensureOutboundSession(usersInRoom: List<String>) {
ensureOutboundSession(getDevicesInRoom(usersInRoom).allowedDevices)
}
/**
* Ensure the outbound session
*
* @param devicesInRoom the devices list
*/
private suspend fun ensureOutboundSession(devicesInRoom: MXUsersDevicesMap<CryptoDeviceInfo>): MXOutboundSessionInfo {
Timber.v("## CRYPTO | ensureOutboundSession start")
var session = outboundSession

View File

@ -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() -> {

View File

@ -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<String>): 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)

View File

@ -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)
}
}

View File

@ -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)

View File

@ -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"

View File

@ -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()

View File

@ -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) {