Merge pull request #1308 from vector-im/feature/fix_dm_shield_logic

Fix / Move DM shield rules to task

CHANGES.md

@@ -47,6 +47,7 @@ Bugfix 🐛:
  - Add user to direct chat by user id (#1065)
  - Use correct URL for SSO connection (#1178)
  - Emoji completion :tada: does not completes to 🎉 like on web (#1285)
+ - Fix bad Shield Logic for DM (#963)
 
 Translations 🗣:
  -

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/ComputeTrustTask.kt

@@ -19,6 +19,7 @@ import im.vector.matrix.android.api.crypto.RoomEncryptionTrustLevel
 import im.vector.matrix.android.api.extensions.orFalse
 import im.vector.matrix.android.api.session.crypto.crosssigning.MXCrossSigningInfo
 import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
+import im.vector.matrix.android.internal.di.UserId
 import im.vector.matrix.android.internal.task.Task
 import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
 import kotlinx.coroutines.withContext
@@ -26,17 +27,28 @@ import javax.inject.Inject
 
 internal interface ComputeTrustTask : Task<ComputeTrustTask.Params, RoomEncryptionTrustLevel> {
     data class Params(
-            val userIds: List<String>
+            val activeMemberUserIds: List<String>,
+            val isDirectRoom: Boolean
     )
 }
 
 internal class DefaultComputeTrustTask @Inject constructor(
         private val cryptoStore: IMXCryptoStore,
+        @UserId private val userId: String,
         private val coroutineDispatchers: MatrixCoroutineDispatchers
 ) : ComputeTrustTask {
 
     override suspend fun execute(params: ComputeTrustTask.Params): RoomEncryptionTrustLevel = withContext(coroutineDispatchers.crypto) {
-        val allTrustedUserIds = params.userIds
+        // The set of “all users” depends on the type of room:
+        // For regular / topic rooms, all users including yourself, are considered when decorating a room
+        // For 1:1 and group DM rooms, all other users (i.e. excluding yourself) are considered when decorating a room
+        val listToCheck = if (params.isDirectRoom) {
+            params.activeMemberUserIds.filter { it != userId }
+        } else {
+            params.activeMemberUserIds
+        }
+
+        val allTrustedUserIds = listToCheck
                 .filter { userId -> getUserCrossSigningKeys(userId)?.isTrusted() == true }
 
         if (allTrustedUserIds.isEmpty()) {
@@ -60,7 +72,7 @@ internal class DefaultComputeTrustTask @Inject constructor(
                         if (hasWarning) {
                             RoomEncryptionTrustLevel.Warning
                         } else {
-                            if (params.userIds.size == allTrustedUserIds.size) {
+                            if (listToCheck.size == allTrustedUserIds.size) {
                                 // all users are trusted and all devices are verified
                                 RoomEncryptionTrustLevel.Trusted
                             } else {

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/SessionToCryptoRoomMembersUpdate.kt

@@ -17,6 +17,7 @@ package im.vector.matrix.android.internal.crypto.crosssigning
 
 data class SessionToCryptoRoomMembersUpdate(
         val roomId: String,
+        val isDirect: Boolean,
         val userIds: List<String>
 )
 

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/ShieldTrustUpdater.kt

@@ -79,7 +79,7 @@ internal class ShieldTrustUpdater @Inject constructor(
             return
         }
         taskExecutor.executorScope.launch(BACKGROUND_HANDLER_DISPATCHER) {
-            val updatedTrust = computeTrustTask.execute(ComputeTrustTask.Params(update.userIds))
+            val updatedTrust = computeTrustTask.execute(ComputeTrustTask.Params(update.userIds, update.isDirect))
             // We need to send that back to session base
             backgroundSessionRealm.get()?.executeTransaction { realm ->
                 roomSummaryUpdater.updateShieldTrust(realm, update.roomId, updatedTrust)
@@ -109,8 +109,9 @@ internal class ShieldTrustUpdater @Inject constructor(
                 if (roomSummary?.isEncrypted.orFalse()) {
                     val allActiveRoomMembers = RoomMemberHelper(realm, roomId).getActiveRoomMemberIds()
                     try {
-                        // Can throw if the crypto database has been closed in between, in this case log and ignore?
-                        val updatedTrust = computeTrustTask.execute(ComputeTrustTask.Params(allActiveRoomMembers))
+                        val updatedTrust = computeTrustTask.execute(
+                                ComputeTrustTask.Params(allActiveRoomMembers, roomSummary?.isDirect == true)
+                        )
                         realm.executeTransaction {
                             roomSummaryUpdater.updateShieldTrust(it, roomId, updatedTrust)
                         }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomSummaryUpdater.kt

@@ -161,15 +161,7 @@ internal class RoomSummaryUpdater @Inject constructor(
             roomSummaryEntity.otherMemberIds.clear()
             roomSummaryEntity.otherMemberIds.addAll(otherRoomMembers)
             if (roomSummaryEntity.isEncrypted) {
-                // The set of “all users” depends on the type of room:
-                // For regular / topic rooms, all users including yourself, are considered when decorating a room
-                // For 1:1 and group DM rooms, all other users (i.e. excluding yourself) are considered when decorating a room
-                val listToCheck = if (roomSummaryEntity.isDirect) {
-                    roomSummaryEntity.otherMemberIds.toList()
-                } else {
-                    roomSummaryEntity.otherMemberIds.toList() + userId
-                }
-                eventBus.post(SessionToCryptoRoomMembersUpdate(roomId, listToCheck))
+                eventBus.post(SessionToCryptoRoomMembersUpdate(roomId, roomSummaryEntity.isDirect, roomSummaryEntity.otherMemberIds.toList() + userId))
             }
         }
     }

vector/src/main/java/im/vector/riotx/features/settings/devices/DeviceVerificationInfoBottomSheetViewModel.kt

@@ -61,8 +61,8 @@ class DeviceVerificationInfoBottomSheetViewModel @AssistedInject constructor(@As
         session.rx().liveCrossSigningInfo(session.myUserId)
                 .execute {
                     copy(
-                            hasAccountCrossSigning = it.invoke()?.get() != null,
-                            accountCrossSigningIsTrusted = it.invoke()?.get()?.isTrusted() == true
+                            hasAccountCrossSigning = it.invoke()?.getOrNull() != null,
+                            accountCrossSigningIsTrusted = it.invoke()?.getOrNull()?.isTrusted() == true
                     )
                 }
 

vector/src/main/java/im/vector/riotx/features/settings/devices/DevicesViewModel.kt

@@ -122,8 +122,8 @@ class DevicesViewModel @AssistedInject constructor(
         session.rx().liveCrossSigningInfo(session.myUserId)
                 .execute {
                     copy(
-                            hasAccountCrossSigning = it.invoke()?.get() != null,
-                            accountCrossSigningIsTrusted = it.invoke()?.get()?.isTrusted() == true
+                            hasAccountCrossSigning = it.invoke()?.getOrNull() != null,
+                            accountCrossSigningIsTrusted = it.invoke()?.getOrNull()?.isTrusted() == true
                     )
                 }
         session.cryptoService().verificationService().addListener(this)