Code review

This commit is contained in:
Valere 2022-04-27 09:45:26 +02:00
parent 728faaee19
commit 8920ed3de8
14 changed files with 1797 additions and 1865 deletions

View file

@ -432,6 +432,7 @@ class KeyShareTests : InstrumentedTest {
.markedLocallyAsManuallyVerified(aliceNewSession.myUserId, aliceNewSession.sessionParams.deviceId!!)
// /!\ Stop initial alice session syncing so that it can't reply
aliceSession.cryptoService().enableKeyGossiping(false)
aliceSession.stopSync()
// Let's now try to request
@ -440,6 +441,7 @@ class KeyShareTests : InstrumentedTest {
// Should get a reply from bob and not from alice
commonTestHelper.waitWithLatch { latch ->
commonTestHelper.retryPeriodicallyWithLatch(latch) {
// Log.d("#TEST", "outgoing key requests :${aliceNewSession.cryptoService().getOutgoingRoomKeyRequests().joinToString { it.sessionId ?: "?" }}")
val outgoing = aliceNewSession.cryptoService().getOutgoingRoomKeyRequests().firstOrNull { it.sessionId == sentEventMegolmSession }
val bobReply = outgoing?.results?.firstOrNull { it.userId == bobSession.myUserId }
val result = bobReply?.result
@ -453,6 +455,7 @@ class KeyShareTests : InstrumentedTest {
assertEquals("The request should not be canceled", OutgoingRoomKeyRequestState.SENT, outgoingReq.state)
// let's wake up alice
aliceSession.cryptoService().enableKeyGossiping(true)
aliceSession.startSync(true)
// We should now get a reply from first session

View file

@ -37,5 +37,5 @@ data class MXCryptoConfig constructor(
* Currently megolm keys are requested to the sender device and to all of our devices.
* You can limit request only to your sessions by turning this setting to `true`
*/
val limitRoomKeyRequestsToMyDevices: Boolean = false
val limitRoomKeyRequestsToMyDevices: Boolean = false,
)

View file

@ -1192,7 +1192,7 @@ internal class DefaultCryptoService @Inject constructor(
*/
override fun removeRoomKeysRequestListener(listener: GossipingRequestListener) {
incomingKeyRequestManager.removeRoomKeysRequestListener(listener)
secretShareManager.addListener(listener)
secretShareManager.removeListener(listener)
}
/**

View file

@ -577,11 +577,11 @@ internal class MXOlmDevice @Inject constructor(
// Inbound group session
sealed class AddSessionResult {
data class Imported(val ratchetIndex: Int) : AddSessionResult()
abstract class Failure : AddSessionResult()
sealed interface AddSessionResult {
data class Imported(val ratchetIndex: Int) : AddSessionResult
abstract class Failure : AddSessionResult
object NotImported : Failure()
data class NotImportedHigherIndex(val newIndex: Int) : AddSessionResult()
data class NotImportedHigherIndex(val newIndex: Int) : Failure()
}
/**

View file

@ -1,27 +0,0 @@
/*
* Copyright 2020 The Matrix.org Foundation C.I.C.
*
* 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 org.matrix.android.sdk.internal.crypto
import org.matrix.android.sdk.api.session.crypto.OutgoingRoomKeyRequestState
interface OutgoingGossipingRequest {
var recipients: Map<String, List<String>>
var requestId: String
var state: OutgoingRoomKeyRequestState
// transaction id for the cancellation, if any
// var cancellationTxnId: String?
}

View file

@ -41,7 +41,6 @@ import org.matrix.android.sdk.api.session.events.model.content.RoomKeyWithHeldCo
import org.matrix.android.sdk.api.session.events.model.toModel
import org.matrix.android.sdk.api.util.fromBase64
import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
import org.matrix.android.sdk.internal.crypto.tasks.DefaultSendToDeviceTask
import org.matrix.android.sdk.internal.crypto.tasks.SendToDeviceTask
import org.matrix.android.sdk.internal.di.SessionId
import org.matrix.android.sdk.internal.di.UserId
@ -70,7 +69,7 @@ internal class OutgoingKeyRequestManager @Inject constructor(
private val coroutineDispatchers: MatrixCoroutineDispatchers,
private val cryptoConfig: MXCryptoConfig,
private val inboundGroupSessionStore: InboundGroupSessionStore,
private val sendToDeviceTask: DefaultSendToDeviceTask,
private val sendToDeviceTask: SendToDeviceTask,
private val deviceListManager: DeviceListManager,
private val perSessionBackupQueryRateLimiter: PerSessionBackupQueryRateLimiter) {

View file

@ -1,39 +0,0 @@
/*
* Copyright 2020 The Matrix.org Foundation C.I.C.
*
* 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 org.matrix.android.sdk.internal.crypto
import com.squareup.moshi.JsonClass
import org.matrix.android.sdk.api.session.crypto.OutgoingRoomKeyRequestState
/**
* Represents an outgoing room key request
*/
@JsonClass(generateAdapter = true)
internal class OutgoingSecretRequest(
// Secret Name
val secretName: String?,
// list of recipients for the request
override var recipients: Map<String, List<String>>,
// Unique id for this request. Used for both
// an id within the request for later pairing with a cancellation, and for
// the transaction id when sending the to_device messages to our local
override var requestId: String,
// current state of this request
override var state: OutgoingRoomKeyRequestState) : OutgoingGossipingRequest {
// transaction id for the cancellation, if any
}

View file

@ -66,7 +66,7 @@ internal class PerSessionBackupQueryRateLimiter @Inject constructor(
private var backupVersion: KeysVersionResult? = null
private var savedKeyBackupKeyInfo: SavedKeyBackupKeyInfo? = null
var backupWasCheckedFromServer: Boolean = false
var now = System.currentTimeMillis()
val now = System.currentTimeMillis()
fun refreshBackupInfoIfNeeded(force: Boolean = false) {
if (backupWasCheckedFromServer && !force) return

View file

@ -28,7 +28,6 @@ import javax.inject.Inject
@SessionScope
internal class RoomEncryptorsStore @Inject constructor(
private val cryptoStore: IMXCryptoStore,
// Repository
private val megolmEncryptionFactory: MXMegolmEncryptionFactory,
private val olmEncryptionFactory: MXOlmEncryptionFactory,
) {

View file

@ -85,7 +85,7 @@ internal class SecretShareManager @Inject constructor(
}
}
fun removeRoomKeysRequestListener(listener: GossipingRequestListener) {
fun removeListener(listener: GossipingRequestListener) {
synchronized(gossipingRequestListeners) {
gossipingRequestListeners.remove(listener)
}
@ -120,7 +120,7 @@ internal class SecretShareManager @Inject constructor(
val userId = toDevice.senderId ?: return Unit.also {
Timber.tag(loggerTag.value)
.v("handleSecretRequest() : Missing secret name")
.v("handleSecretRequest() : Missing senderId")
}
if (userId != credentials.userId) {

View file

@ -229,7 +229,7 @@ internal class MXMegolmDecryption(
}
Timber.tag(loggerTag.value).i("onRoomKeyEvent addInboundGroupSession ${roomKeyContent.sessionId}")
val added = olmDevice.addInboundGroupSession(roomKeyContent.sessionId,
val addSessionResult = olmDevice.addInboundGroupSession(roomKeyContent.sessionId,
roomKeyContent.sessionKey,
roomKeyContent.roomId,
senderKey,
@ -237,9 +237,9 @@ internal class MXMegolmDecryption(
keysClaimed,
exportFormat)
when (added) {
is MXOlmDevice.AddSessionResult.Imported -> added.ratchetIndex
is MXOlmDevice.AddSessionResult.NotImportedHigherIndex -> added.newIndex
when (addSessionResult) {
is MXOlmDevice.AddSessionResult.Imported -> addSessionResult.ratchetIndex
is MXOlmDevice.AddSessionResult.NotImportedHigherIndex -> addSessionResult.newIndex
else -> null
}?.let { index ->
if (event.getClearType() == EventType.FORWARDED_ROOM_KEY) {
@ -273,7 +273,7 @@ internal class MXMegolmDecryption(
}
}
if (added is MXOlmDevice.AddSessionResult.Imported) {
if (addSessionResult is MXOlmDevice.AddSessionResult.Imported) {
Timber.tag(loggerTag.value)
.d("onRoomKeyEvent(${event.getClearType()}) : Added megolm session ${roomKeyContent.sessionId} in ${roomKeyContent.roomId}")
defaultKeysBackupService.maybeBackupKeys()

View file

@ -892,7 +892,8 @@ internal class RealmCryptoStore @Inject constructor(
val sessionIdentifier = olmInboundGroupSessionWrapper.olmInboundGroupSession?.sessionIdentifier()
val key = OlmInboundGroupSessionEntity.createPrimaryKey(
sessionIdentifier,
olmInboundGroupSessionWrapper.senderKey)
olmInboundGroupSessionWrapper.senderKey
)
val existing = realm.where<OlmInboundGroupSessionEntity>()
.equalTo(OlmInboundGroupSessionEntityFields.PRIMARY_KEY, key)
@ -1049,7 +1050,7 @@ internal class RealmCryptoStore @Inject constructor(
.equalTo(OutgoingKeyRequestEntityFields.ROOM_ID, requestBody.roomId)
.equalTo(OutgoingKeyRequestEntityFields.MEGOLM_SESSION_ID, requestBody.sessionId)
}.map {
it.toOutgoingGossipingRequest()
it.toOutgoingKeyRequest()
}.firstOrNull {
it.requestBody?.algorithm == requestBody.algorithm &&
it.requestBody?.roomId == requestBody.roomId &&
@ -1063,7 +1064,7 @@ internal class RealmCryptoStore @Inject constructor(
realm.where<OutgoingKeyRequestEntity>()
.equalTo(OutgoingKeyRequestEntityFields.REQUEST_ID, requestId)
}.map {
it.toOutgoingGossipingRequest()
it.toOutgoingKeyRequest()
}.firstOrNull()
}
@ -1074,7 +1075,7 @@ internal class RealmCryptoStore @Inject constructor(
.equalTo(OutgoingKeyRequestEntityFields.ROOM_ID, roomId)
.equalTo(OutgoingKeyRequestEntityFields.MEGOLM_SESSION_ID, sessionId)
}.map {
it.toOutgoingGossipingRequest()
it.toOutgoingKeyRequest()
}.filter {
it.requestBody?.algorithm == algorithm &&
it.requestBody?.senderKey == senderKey
@ -1088,30 +1089,35 @@ internal class RealmCryptoStore @Inject constructor(
val dataSourceFactory = realmDataSourceFactory.map {
AuditTrailMapper.map(it)
// mm we can't map not null...
?: AuditTrail(
System.currentTimeMillis(),
TrailType.Unknown,
IncomingKeyRequestInfo(
"",
"",
"",
"",
"",
"",
"",
)
)
?: createUnknownTrail()
}
return monarchy.findAllPagedWithChanges(realmDataSourceFactory,
LivePagedListBuilder(dataSourceFactory,
return monarchy.findAllPagedWithChanges(
realmDataSourceFactory,
LivePagedListBuilder(
dataSourceFactory,
PagedList.Config.Builder()
.setPageSize(20)
.setEnablePlaceholders(false)
.setPrefetchDistance(1)
.build())
.build()
)
)
}
private fun createUnknownTrail() = AuditTrail(
System.currentTimeMillis(),
TrailType.Unknown,
IncomingKeyRequestInfo(
"",
"",
"",
"",
"",
"",
"",
)
)
override fun <T> getGossipingEventsTrail(type: TrailType, mapper: ((AuditTrail) -> T)): LiveData<PagedList<T>> {
val realmDataSourceFactory = monarchy.createDataSourceFactory { realm ->
realm.where<AuditTrailEntity>()
@ -1121,28 +1127,19 @@ internal class RealmCryptoStore @Inject constructor(
val dataSourceFactory = realmDataSourceFactory.map { entity ->
(AuditTrailMapper.map(entity)
// mm we can't map not null...
?: AuditTrail(
System.currentTimeMillis(),
type,
IncomingKeyRequestInfo(
"",
"",
"",
"",
"",
"",
"",
)
)
?: createUnknownTrail()
).let { mapper.invoke(it) }
}
return monarchy.findAllPagedWithChanges(realmDataSourceFactory,
LivePagedListBuilder(dataSourceFactory,
return monarchy.findAllPagedWithChanges(
realmDataSourceFactory,
LivePagedListBuilder(
dataSourceFactory,
PagedList.Config.Builder()
.setPageSize(20)
.setEnablePlaceholders(false)
.setPrefetchDistance(1)
.build())
.build()
)
)
}
@ -1166,7 +1163,7 @@ internal class RealmCryptoStore @Inject constructor(
.equalTo(OutgoingKeyRequestEntityFields.ROOM_ID, requestBody.roomId)
.findAll()
.map {
it.toOutgoingGossipingRequest()
it.toOutgoingKeyRequest()
}.also {
if (it.size > 1) {
// there should be one or zero but not more, worth warning
@ -1188,7 +1185,7 @@ internal class RealmCryptoStore @Inject constructor(
this.requestState = OutgoingRoomKeyRequestState.UNSENT
this.setRequestBody(requestBody)
this.creationTimeStamp = System.currentTimeMillis()
}.toOutgoingGossipingRequest()
}.toOutgoingKeyRequest()
} else {
request = existing
}
@ -1231,7 +1228,7 @@ internal class RealmCryptoStore @Inject constructor(
.equalTo(OutgoingKeyRequestEntityFields.ROOM_ID, roomId)
.equalTo(OutgoingKeyRequestEntityFields.MEGOLM_SESSION_ID, sessionId)
.findAll().firstOrNull { entity ->
entity.toOutgoingGossipingRequest().let {
entity.toOutgoingKeyRequest().let {
it.requestBody?.senderKey == senderKey &&
it.requestBody?.algorithm == algorithm
}
@ -1473,7 +1470,7 @@ internal class RealmCryptoStore @Inject constructor(
realm
.where(OutgoingKeyRequestEntity::class.java)
}, { entity ->
entity.toOutgoingGossipingRequest()
entity.toOutgoingKeyRequest()
})
.filterNotNull()
}
@ -1484,7 +1481,7 @@ internal class RealmCryptoStore @Inject constructor(
.where(OutgoingKeyRequestEntity::class.java)
.`in`(OutgoingKeyRequestEntityFields.REQUEST_STATE_STR, inStates.map { it.name }.toTypedArray())
}, { entity ->
entity.toOutgoingGossipingRequest()
entity.toOutgoingKeyRequest()
})
.filterNotNull()
}
@ -1495,15 +1492,18 @@ internal class RealmCryptoStore @Inject constructor(
.where(OutgoingKeyRequestEntity::class.java)
}
val dataSourceFactory = realmDataSourceFactory.map {
it.toOutgoingGossipingRequest()
it.toOutgoingKeyRequest()
}
val trail = monarchy.findAllPagedWithChanges(realmDataSourceFactory,
LivePagedListBuilder(dataSourceFactory,
val trail = monarchy.findAllPagedWithChanges(
realmDataSourceFactory,
LivePagedListBuilder(
dataSourceFactory,
PagedList.Config.Builder()
.setPageSize(20)
.setEnablePlaceholders(false)
.setPrefetchDistance(1)
.build())
.build()
)
)
return trail
}

View file

@ -58,7 +58,7 @@ internal open class OutgoingKeyRequestEntity(
)
}
fun getRequestedKeyInfo(): RoomKeyRequestBody? = RoomKeyRequestBody.fromJson(requestedInfoStr)
private fun getRequestedKeyInfo(): RoomKeyRequestBody? = RoomKeyRequestBody.fromJson(requestedInfoStr)
fun setRequestBody(body: RoomKeyRequestBody) {
requestedInfoStr = body.toJson()
@ -92,7 +92,7 @@ internal open class OutgoingKeyRequestEntity(
replies.add(newReply)
}
fun toOutgoingGossipingRequest(): OutgoingKeyRequest {
fun toOutgoingKeyRequest(): OutgoingKeyRequest {
return OutgoingKeyRequest(
requestBody = getRequestedKeyInfo(),
recipients = getRecipients().orEmpty(),

View file

@ -18,7 +18,6 @@ package org.matrix.android.sdk.internal.session.room.membership.joining
import io.realm.RealmConfiguration
import kotlinx.coroutines.TimeoutCancellationException
import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
import org.matrix.android.sdk.api.session.events.model.toContent
import org.matrix.android.sdk.api.session.identity.model.SignInvitationResult
@ -71,13 +70,11 @@ internal class DefaultJoinRoomTask @Inject constructor(
}
val joinRoomResponse = try {
executeRequest(globalErrorReceiver) {
withContext(coroutineDispatcher.io) {
roomAPI.join(
roomIdOrAlias = params.roomIdOrAlias,
viaServers = params.viaServers.take(3),
params = extraParams
)
}
roomAPI.join(
roomIdOrAlias = params.roomIdOrAlias,
viaServers = params.viaServers.take(3),
params = extraParams
)
}
} catch (failure: Throwable) {
roomChangeMembershipStateDataSource.updateState(params.roomIdOrAlias, ChangeMembershipState.FailedJoining(failure))