Merge pull request #8507 from vector-im/feature/bca/clean_room_shield_update

Clean room shield update logic

changelog.d/8507.bugfix

@@ -0,0 +1 @@
+In some conditions the room shield is not refreshed correctly

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt

@@ -99,6 +99,23 @@ class CommonTestHelper internal constructor(context: Context, val cryptoConfig:
                 }
             }
         }
+
+        @OptIn(ExperimentalCoroutinesApi::class)
+        internal fun runLongCryptoTest(context: Context, cryptoConfig: MXCryptoConfig? = null, autoSignoutOnClose: Boolean = true, block: suspend CoroutineScope.(CryptoTestHelper, CommonTestHelper) -> Unit) {
+            val testHelper = CommonTestHelper(context, cryptoConfig)
+            val cryptoTestHelper = CryptoTestHelper(testHelper)
+            return runTest(dispatchTimeoutMs = TestConstants.timeOutMillis * 4) {
+                try {
+                    withContext(Dispatchers.Default) {
+                        block(cryptoTestHelper, testHelper)
+                    }
+                } finally {
+                    if (autoSignoutOnClose) {
+                        testHelper.cleanUpOpenedSessions()
+                    }
+                }
+            }
+        }
     }
 
     internal val matrix: TestMatrix

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/RoomShieldTest.kt

@@ -0,0 +1,133 @@
+/*
+ * Copyright 2023 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 android.util.Log
+import androidx.lifecycle.Observer
+import androidx.test.filters.LargeTest
+import kotlinx.coroutines.DelicateCoroutinesApi
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.GlobalScope
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
+import org.junit.FixMethodOrder
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.runners.MethodSorters
+import org.matrix.android.sdk.InstrumentedTest
+import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
+import org.matrix.android.sdk.api.session.getRoom
+import org.matrix.android.sdk.api.session.room.model.RoomSummary
+import org.matrix.android.sdk.api.util.Optional
+import org.matrix.android.sdk.common.CommonTestHelper
+import org.matrix.android.sdk.common.SessionTestParams
+import org.matrix.android.sdk.common.TestConstants
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+@RunWith(JUnit4::class)
+@FixMethodOrder(MethodSorters.JVM)
+@LargeTest
+class RoomShieldTest : InstrumentedTest {
+
+    @Test
+    fun testShieldNoVerification() = CommonTestHelper.runCryptoTest(context()) { cryptoTestHelper, _ ->
+        val testData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom()
+
+        val roomId = testData.roomId
+
+        cryptoTestHelper.initializeCrossSigning(testData.firstSession)
+        cryptoTestHelper.initializeCrossSigning(testData.secondSession!!)
+
+        // Test are flaky unless I use liveData observer on main thread
+        // Just calling getRoomSummary() with retryWithBackOff keeps an outdated version of the value
+        testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Default)
+        testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Default)
+    }
+
+    @Test
+    fun testShieldInOneOne() = CommonTestHelper.runLongCryptoTest(context()) { cryptoTestHelper, testHelper ->
+        val testData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom()
+
+        val roomId = testData.roomId
+
+        Log.v("#E2E TEST", "Initialize cross signing...")
+        cryptoTestHelper.initializeCrossSigning(testData.firstSession)
+        cryptoTestHelper.initializeCrossSigning(testData.secondSession!!)
+        Log.v("#E2E TEST", "... Initialized.")
+
+        // let alive and bob verify
+        Log.v("#E2E TEST", "Alice and Bob verify each others...")
+        cryptoTestHelper.verifySASCrossSign(testData.firstSession, testData.secondSession!!, testData.roomId)
+
+        // Add a new session for bob
+        // This session will be unverified for now
+
+        Log.v("#E2E TEST", "Log in a new bob device...")
+        val bobSecondSession = testHelper.logIntoAccount(testData.secondSession!!.myUserId, SessionTestParams(true))
+
+        Log.v("#E2E TEST", "Bob session logged in ${bobSecondSession.myUserId.take(6)}")
+
+        Log.v("#E2E TEST", "Assert room shields...")
+        testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Warning)
+        // in 1:1 we ignore our own status
+        testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Trusted)
+
+        // Adding another user should make bob consider his devices now and see same shield as alice
+        Log.v("#E2E TEST", "Create Sam account")
+        val samSession = testHelper.createAccount(TestConstants.USER_SAM, SessionTestParams(withInitialSync = true))
+
+        // Let alice invite sam
+        Log.v("#E2E TEST", "Let alice invite sam")
+        testData.firstSession.getRoom(roomId)!!.membershipService().invite(samSession.myUserId)
+        testHelper.waitForAndAcceptInviteInRoom(samSession, roomId)
+
+        Log.v("#E2E TEST", "Assert room shields...")
+        testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Warning)
+        testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Warning)
+
+        // Now let's bob verify his session
+
+        Log.v("#E2E TEST", "Bob verifies his new session")
+        cryptoTestHelper.verifyNewSession(testData.secondSession!!, bobSecondSession)
+
+        testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Trusted)
+        testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Trusted)
+    }
+
+    @OptIn(DelicateCoroutinesApi::class)
+    private suspend fun Session.assertRoomShieldIs(roomId: String, state: RoomEncryptionTrustLevel?) {
+        val lock = CountDownLatch(1)
+        val roomLiveData = withContext(Dispatchers.Main) {
+            roomService().getRoomSummaryLive(roomId)
+        }
+        val observer = object : Observer<Optional<RoomSummary>> {
+            override fun onChanged(value: Optional<RoomSummary>) {
+                Log.v("#E2E TEST ${this@assertRoomShieldIs.myUserId.take(6)}", "Shield Update ${value.getOrNull()?.roomEncryptionTrustLevel}")
+                if (value.getOrNull()?.roomEncryptionTrustLevel == state) {
+                    lock.countDown()
+                    roomLiveData.removeObserver(this)
+                }
+            }
+        }
+        GlobalScope.launch(Dispatchers.Main) { roomLiveData.observeForever(observer) }
+
+        lock.await(40_000, TimeUnit.MILLISECONDS)
+    }
+}

matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt

@@ -49,7 +49,6 @@ import org.matrix.android.sdk.internal.di.SessionId
 import org.matrix.android.sdk.internal.di.UserId
 import org.matrix.android.sdk.internal.di.WorkManagerProvider
 import org.matrix.android.sdk.internal.session.SessionScope
-import org.matrix.android.sdk.internal.task.TaskExecutor
 import org.matrix.android.sdk.internal.util.JsonCanonicalizer
 import org.matrix.android.sdk.internal.util.logLimit
 import org.matrix.android.sdk.internal.worker.WorkerParamsFactory
@@ -66,7 +65,6 @@ internal class DefaultCrossSigningService @Inject constructor(
         private val deviceListManager: DeviceListManager,
         private val initializeCrossSigningTask: InitializeCrossSigningTask,
         private val uploadSignaturesTask: UploadSignaturesTask,
-        private val taskExecutor: TaskExecutor,
         private val coroutineDispatchers: MatrixCoroutineDispatchers,
         private val cryptoCoroutineScope: CoroutineScope,
         private val workManagerProvider: WorkManagerProvider,
@@ -612,9 +610,7 @@ internal class DefaultCrossSigningService @Inject constructor(
         withContext(coroutineDispatchers.crypto) {
             // This device should be yours
             val device = cryptoStore.getUserDevice(myUserId, deviceId)
-            if (device == null) {
+                    ?: throw IllegalArgumentException("This device [$deviceId] is not known, or not yours")
-                throw IllegalArgumentException("This device [$deviceId] is not known, or not yours")
-            }
 
             val myKeys = getUserCrossSigningKeys(myUserId)
                     ?: throw Throwable("CrossSigning is not setup for this account")

matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt

@@ -31,7 +31,7 @@ import org.matrix.android.sdk.api.session.crypto.crosssigning.isCrossSignedVerif
 import org.matrix.android.sdk.api.session.crypto.crosssigning.isVerified
 import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
 import org.matrix.android.sdk.internal.SessionManager
-import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
+import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider
 import org.matrix.android.sdk.internal.crypto.store.db.mapper.CrossSigningKeysMapper
 import org.matrix.android.sdk.internal.crypto.store.db.model.CrossSigningInfoEntity
 import org.matrix.android.sdk.internal.crypto.store.db.model.CrossSigningInfoEntityFields
@@ -40,10 +40,7 @@ import org.matrix.android.sdk.internal.crypto.store.db.model.TrustLevelEntity
 import org.matrix.android.sdk.internal.crypto.store.db.model.UserEntity
 import org.matrix.android.sdk.internal.crypto.store.db.model.UserEntityFields
 import org.matrix.android.sdk.internal.database.awaitTransaction
-import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntity
-import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntityFields
 import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity
-import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
 import org.matrix.android.sdk.internal.database.query.where
 import org.matrix.android.sdk.internal.di.CryptoDatabase
 import org.matrix.android.sdk.internal.di.SessionDatabase
@@ -83,35 +80,54 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses
     @Inject lateinit var myUserId: String
     @Inject lateinit var crossSigningKeysMapper: CrossSigningKeysMapper
     @Inject lateinit var updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository
+    @Inject lateinit var cryptoSessionInfoProvider: CryptoSessionInfoProvider
 
     //    @Inject lateinit var roomSummaryUpdater: RoomSummaryUpdater
-    @Inject lateinit var cryptoStore: IMXCryptoStore
+//    @Inject lateinit var cryptoStore: IMXCryptoStore
 
     override fun injectWith(injector: SessionComponent) {
         injector.inject(this)
     }
 
     override suspend fun doSafeWork(params: Params): Result {
-        val userList = params.filename
+        val sId = myUserId.take(5)
+        Timber.v("## CrossSigning - UpdateTrustWorker started..")
+        val workerParams = params.filename
                 ?.let { updateTrustWorkerDataRepository.getParam(it) }
-                ?.userIds
+                ?: return Result.success().also {
-                ?: params.updatedUserIds.orEmpty()
+                    Timber.w("## CrossSigning - UpdateTrustWorker failed to get params")
+                    cleanup(params)
+                }
+
+        Timber.v("## CrossSigning [$sId]- UpdateTrustWorker userIds:${workerParams.userIds.logLimit()}, roomIds:${workerParams.roomIds.orEmpty().logLimit()}")
+        val userList = workerParams.userIds
 
         // List should not be empty, but let's avoid go further in case of empty list
         if (userList.isNotEmpty()) {
             // Unfortunately we don't have much info on what did exactly changed (is it the cross signing keys of that user,
             // or a new device?) So we check all again :/
-            Timber.v("## CrossSigning - Updating trust for users: ${userList.logLimit()}")
+            Timber.v("## CrossSigning [$sId]- Updating trust for users: ${userList.logLimit()}")
             updateTrust(userList)
         }
 
+        val roomsToCheck = workerParams.roomIds ?: cryptoSessionInfoProvider.getRoomsWhereUsersAreParticipating(userList)
+        Timber.v("## CrossSigning [$sId]- UpdateTrustWorker roomShield to check:${roomsToCheck.logLimit()}")
+        var myCrossSigningInfo: MXCrossSigningInfo?
+        Realm.getInstance(cryptoRealmConfiguration).use { realm ->
+            myCrossSigningInfo = getCrossSigningInfo(realm, myUserId)
+        }
+        // So Cross Signing keys trust is updated, device trust is updated
+        // We can now update room shields? in the session DB?
+        updateRoomShieldInSummaries(roomsToCheck, myCrossSigningInfo)
+
         cleanup(params)
         return Result.success()
     }
 
     private suspend fun updateTrust(userListParam: List<String>) {
+        val sId = myUserId.take(5)
         var userList = userListParam
-        var myCrossSigningInfo: MXCrossSigningInfo? = null
+        var myCrossSigningInfo: MXCrossSigningInfo?
 
         // First we check that the users MSK are trusted by mine
         // After that we check the trust chain for each devices of each users
@@ -123,7 +139,7 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses
             var myTrustResult: UserTrustResult? = null
 
             if (userList.contains(myUserId)) {
-                Timber.d("## CrossSigning - Clear all trust as a change on my user was detected")
+                Timber.d("## CrossSigning [$sId]- Clear all trust as a change on my user was detected")
                 // i am in the list.. but i don't know exactly the delta of change :/
                 // If it's my cross signing keys we should refresh all trust
                 // do it anyway ?
@@ -153,7 +169,7 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses
                     myUserId -> myTrustResult
                     else -> {
                         crossSigningService.checkOtherMSKTrusted(myCrossSigningInfo, entry.value).also {
-                            Timber.v("## CrossSigning - user:${entry.key} result:$it")
+                            Timber.v("## CrossSigning [$sId]- user:${entry.key} result:$it")
                         }
                     }
                 }
@@ -163,12 +179,12 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses
             // i have all the new trusts, update DB
             trusts.forEach {
                 val verified = it.value?.isVerified() == true
-                Timber.v("[$myUserId] ## CrossSigning - Updating user trust: ${it.key} to $verified")
+                Timber.v("[$myUserId] ## CrossSigning [$sId]- Updating user trust: ${it.key} to $verified")
                 updateCrossSigningKeysTrust(cryptoRealm, it.key, verified)
             }
 
             // Ok so now we have to check device trust for all these users..
-            Timber.v("## CrossSigning - Updating devices cross trust users: ${trusts.keys.logLimit()}")
+            Timber.v("## CrossSigning [$sId]- Updating devices cross trust users: ${trusts.keys.logLimit()}")
             trusts.keys.forEach { userId ->
                 val devicesEntities = cryptoRealm.where<UserEntity>()
                         .equalTo(UserEntityFields.USER_ID, userId)
@@ -184,9 +200,9 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses
                 // Update trust if needed
                 devicesEntities?.forEach { device ->
                     val crossSignedVerified = trustMap?.get(device)?.isCrossSignedVerified()
-                    Timber.v("## CrossSigning - Trust for ${device.userId}|${device.deviceId} : cross verified: ${trustMap?.get(device)}")
+                    Timber.v("## CrossSigning [$sId]- Trust for ${device.userId}|${device.deviceId} : cross verified: ${trustMap?.get(device)}")
                     if (device.trustLevelEntity?.crossSignedVerified != crossSignedVerified) {
-                        Timber.d("## CrossSigning - Trust change detected for ${device.userId}|${device.deviceId} : cross verified: $crossSignedVerified")
+                        Timber.d("## CrossSigning [$sId]- Trust change detected for ${device.userId}|${device.deviceId} : cross verified: $crossSignedVerified")
                         // need to save
                         val trustEntity = device.trustLevelEntity
                         if (trustEntity == null) {
@@ -197,50 +213,46 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses
                         } else {
                             trustEntity.crossSignedVerified = crossSignedVerified
                         }
+                    } else {
+                        Timber.v("## CrossSigning [$sId]- Trust unchanged for ${device.userId}|${device.deviceId} : cross verified: $crossSignedVerified")
                     }
                 }
             }
         }
-
-        // So Cross Signing keys trust is updated, device trust is updated
-        // We can now update room shields? in the session DB?
-        updateTrustStep2(userList, myCrossSigningInfo)
     }
 
-    private suspend fun updateTrustStep2(userList: List<String>, myCrossSigningInfo: MXCrossSigningInfo?) {
+    private suspend fun updateRoomShieldInSummaries(roomList: List<String>, myCrossSigningInfo: MXCrossSigningInfo?) {
-        Timber.d("## CrossSigning - Updating shields for impacted rooms...")
+        val sId = myUserId.take(5)
+        Timber.d("## CrossSigning [$sId]- Updating shields for impacted rooms... ${roomList.logLimit()}")
         awaitTransaction(sessionRealmConfiguration) { sessionRealm ->
             Timber.d("## CrossSigning - Updating shields for impacted rooms - in transaction")
             Realm.getInstance(cryptoRealmConfiguration).use { cryptoRealm ->
-                sessionRealm.where(RoomMemberSummaryEntity::class.java)
+                roomList.forEach { roomId ->
-                        .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray())
+                    Timber.v("## CrossSigning [$sId]- Checking room $roomId")
-                        .distinct(RoomMemberSummaryEntityFields.ROOM_ID)
+                    RoomSummaryEntity.where(sessionRealm, roomId)
-                        .findAll()
+//                            .equalTo(RoomSummaryEntityFields.IS_ENCRYPTED, true)
-                        .map { it.roomId }
+                            .findFirst()
-                        .also { Timber.d("## CrossSigning -  ... impacted rooms ${it.logLimit()}") }
+                            ?.let { roomSummary ->
-                        .forEach { roomId ->
+                                Timber.v("## CrossSigning [$sId]- Check shield state for room $roomId")
-                            RoomSummaryEntity.where(sessionRealm, roomId)
+                                val allActiveRoomMembers = RoomMemberHelper(sessionRealm, roomId).getActiveRoomMemberIds()
-                                    .equalTo(RoomSummaryEntityFields.IS_ENCRYPTED, true)
+                                try {
-                                    .findFirst()
+                                    val updatedTrust = computeRoomShield(
-                                    ?.let { roomSummary ->
+                                            myCrossSigningInfo,
-                                        Timber.v("## CrossSigning - Check shield state for room $roomId")
+                                            cryptoRealm,
-                                        val allActiveRoomMembers = RoomMemberHelper(sessionRealm, roomId).getActiveRoomMemberIds()
+                                            allActiveRoomMembers,
-                                        try {
+                                            roomSummary
-                                            val updatedTrust = computeRoomShield(
+                                    )
-                                                    myCrossSigningInfo,
+                                    if (roomSummary.roomEncryptionTrustLevel != updatedTrust) {
-                                                    cryptoRealm,
+                                        Timber.d("## CrossSigning [$sId]- Shield change detected for $roomId -> $updatedTrust")
-                                                    allActiveRoomMembers,
+                                        roomSummary.roomEncryptionTrustLevel = updatedTrust
-                                                    roomSummary
+                                    } else {
-                                            )
+                                        Timber.v("## CrossSigning [$sId]- Shield unchanged for $roomId -> $updatedTrust")
-                                            if (roomSummary.roomEncryptionTrustLevel != updatedTrust) {
-                                                Timber.d("## CrossSigning - Shield change detected for $roomId -> $updatedTrust")
-                                                roomSummary.roomEncryptionTrustLevel = updatedTrust
-                                            }
-                                        } catch (failure: Throwable) {
-                                            Timber.e(failure)
-                                        }
                                     }
-                        }
+                                } catch (failure: Throwable) {
+                                    Timber.e(failure)
+                                }
+                            }
+                }
             }
         }
         Timber.d("## CrossSigning - Updating shields for impacted rooms - END")

matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2023 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.session.sync.handler
+
+import androidx.work.BackoffPolicy
+import androidx.work.ExistingWorkPolicy
+import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorker
+import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorkerDataRepository
+import org.matrix.android.sdk.internal.di.SessionId
+import org.matrix.android.sdk.internal.di.WorkManagerProvider
+import org.matrix.android.sdk.internal.session.SessionScope
+import org.matrix.android.sdk.internal.util.logLimit
+import org.matrix.android.sdk.internal.worker.WorkerParamsFactory
+import timber.log.Timber
+import java.util.concurrent.TimeUnit
+import javax.inject.Inject
+
+@SessionScope
+internal class ShieldSummaryUpdater @Inject constructor(
+        @SessionId private val sessionId: String,
+        private val workManagerProvider: WorkManagerProvider,
+        private val updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository,
+) {
+
+    fun refreshShieldsForRoomIds(roomIds: Set<String>) {
+        Timber.d("## CrossSigning - checkAffectedRoomShields for roomIds: ${roomIds.logLimit()}")
+        val workerParams = UpdateTrustWorker.Params(
+                sessionId = sessionId,
+                filename = updateTrustWorkerDataRepository.createParam(emptyList(), roomIds = roomIds.toList())
+        )
+        val workerData = WorkerParamsFactory.toData(workerParams)
+
+        val workRequest = workManagerProvider.matrixOneTimeWorkRequestBuilder<UpdateTrustWorker>()
+                .setInputData(workerData)
+                .setBackoffCriteria(BackoffPolicy.LINEAR, WorkManagerProvider.BACKOFF_DELAY_MILLIS, TimeUnit.MILLISECONDS)
+                .build()
+
+        workManagerProvider.workManager
+                .beginUniqueWork("TRUST_UPDATE_QUEUE", ExistingWorkPolicy.APPEND_OR_REPLACE, workRequest)
+                .enqueue()
+    }
+}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CryptoSessionInfoProvider.kt

@@ -19,19 +19,19 @@ package org.matrix.android.sdk.internal.crypto
 import com.zhuinden.monarchy.Monarchy
 import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
 import org.matrix.android.sdk.api.session.events.model.EventType
+import org.matrix.android.sdk.api.session.room.model.Membership
 import org.matrix.android.sdk.internal.database.mapper.EventMapper
 import org.matrix.android.sdk.internal.database.model.EventEntity
 import org.matrix.android.sdk.internal.database.model.EventEntityFields
-import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntity
-import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntityFields
 import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity
+import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
 import org.matrix.android.sdk.internal.database.query.where
 import org.matrix.android.sdk.internal.database.query.whereType
 import org.matrix.android.sdk.internal.di.SessionDatabase
 import org.matrix.android.sdk.internal.di.UserId
+import org.matrix.android.sdk.internal.query.process
 import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper
 import org.matrix.android.sdk.internal.util.fetchCopied
-import org.matrix.android.sdk.internal.util.logLimit
 import timber.log.Timber
 import javax.inject.Inject
 
@@ -89,14 +89,30 @@ internal class CryptoSessionInfoProvider @Inject constructor(
     }
 
     fun getRoomsWhereUsersAreParticipating(userList: List<String>): List<String> {
+        if (userList.contains(myUserId)) {
+            // just take all
+            val roomIds: List<String>? = null
+            monarchy.doWithRealm { sessionRealm ->
+                RoomSummaryEntity.where(sessionRealm)
+                        .process(RoomSummaryEntityFields.MEMBERSHIP_STR, Membership.activeMemberships())
+                        .findAll()
+                        .map { it.roomId }
+            }
+            return roomIds.orEmpty()
+        }
         var roomIds: List<String>? = null
         monarchy.doWithRealm { sessionRealm ->
-            roomIds = sessionRealm.where(RoomMemberSummaryEntity::class.java)
+            roomIds = RoomSummaryEntity.where(sessionRealm)
-                    .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray())
+                    .process(RoomSummaryEntityFields.MEMBERSHIP_STR, Membership.activeMemberships())
-                    .distinct(RoomMemberSummaryEntityFields.ROOM_ID)
                     .findAll()
+                    .filter { it.otherMemberIds.any { it in userList } }
                     .map { it.roomId }
-                    .also { Timber.d("## CrossSigning -  ... impacted rooms ${it.logLimit()}") }
+//            roomIds = sessionRealm.where(RoomMemberSummaryEntity::class.java)
+//                    .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray())
+//                    .distinct(RoomMemberSummaryEntityFields.ROOM_ID)
+//                    .findAll()
+//                    .map { it.roomId }
+//                    .also { Timber.d("## CrossSigning -  ... impacted rooms ${it.logLimit()}") }
         }
         return roomIds.orEmpty()
     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorkerDataRepository.kt

@@ -28,7 +28,10 @@ import javax.inject.Inject
 @JsonClass(generateAdapter = true)
 internal data class UpdateTrustWorkerData(
         @Json(name = "userIds")
-        val userIds: List<String>
+        val userIds: List<String>,
+        // When we just need to refresh the room shield (no change on user keys, but a membership change)
+        @Json(name = "roomIds")
+        val roomIds: List<String>? = null
 )
 
 internal class UpdateTrustWorkerDataRepository @Inject constructor(
@@ -38,12 +41,12 @@ internal class UpdateTrustWorkerDataRepository @Inject constructor(
     private val jsonAdapter = MoshiProvider.providesMoshi().adapter(UpdateTrustWorkerData::class.java)
 
     // Return the path of the created file
-    fun createParam(userIds: List<String>): String {
+    fun createParam(userIds: List<String>, roomIds: List<String>? = null): String {
         val filename = "${UUID.randomUUID()}.json"
         workingDirectory.mkdirs()
         val file = File(workingDirectory, filename)
 
-        UpdateTrustWorkerData(userIds = userIds)
+        UpdateTrustWorkerData(userIds = userIds, roomIds = roomIds)
                 .let { jsonAdapter.toJson(it) }
                 .let { file.writeText(it) }
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt

@@ -168,6 +168,9 @@ internal class RoomSummaryUpdater @Inject constructor(
         val roomAliases = ContentMapper.map(lastAliasesEvent?.content).toModel<RoomAliasesContent>()?.aliases
                 .orEmpty()
         roomSummaryEntity.updateAliases(roomAliases)
+
+        val wasEncrypted = roomSummaryEntity.isEncrypted
+
         roomSummaryEntity.isEncrypted = encryptionEvent != null
 
         roomSummaryEntity.e2eAlgorithm = ContentMapper.map(encryptionEvent?.content)
@@ -197,17 +200,13 @@ internal class RoomSummaryUpdater @Inject constructor(
                 // better to use what we know
                 roomSummaryEntity.joinedMembersCount = otherRoomMembers.size + 1
             }
-            if (roomSummaryEntity.isEncrypted && otherRoomMembers.isNotEmpty()) {
+        }
-                if (aggregator == null) {
+
-                    // Do it now
+        if (roomSummaryEntity.isEncrypted) {
-                    // mmm maybe we could only refresh shield instead of checking trust also?
+            if (!wasEncrypted || updateMembers || roomSummaryEntity.roomEncryptionTrustLevel == null) {
-                    // XXX why doing this here? we don't show shield anymore and it will be refreshed
+                // trigger a shield update
-                    // by the sdk
+                // if users add devices/keys or signatures the device list manager will trigger a refresh
-                    // crossSigningService.checkTrustAndAffectedRoomShields(otherRoomMembers)
+                aggregator?.roomsWithMembershipChangesForShieldUpdate?.add(roomId)
-                } else {
-                    // Schedule it
-                    aggregator.userIdsForCheckingTrustAndAffectedRoomShields.addAll(otherRoomMembers)
-                }
             }
         }
     }
@@ -410,7 +409,7 @@ internal class RoomSummaryUpdater @Inject constructor(
                         val relatedSpaces = lookupMap.keys
                                 .filter { it.roomType == RoomType.SPACE }
                                 .filter {
-                                    dmRoom.otherMemberIds.toList().intersect(it.otherMemberIds.toList()).isNotEmpty()
+                                    dmRoom.otherMemberIds.toList().intersect(it.otherMemberIds.toSet()).isNotEmpty()
                                 }
                                 .map { it.roomId }
                                 .distinct()

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt

@@ -29,7 +29,8 @@ internal class SyncResponsePostTreatmentAggregator {
     val userIdsToFetch = mutableSetOf<String>()
 
     // Set of users to call `crossSigningService.checkTrustAndAffectedRoomShields` once per sync
-    val userIdsForCheckingTrustAndAffectedRoomShields = mutableSetOf<String>()
+
+    val roomsWithMembershipChangesForShieldUpdate = mutableSetOf<String>()
 
     // For the crypto store
     val cryptoStoreAggregator = CryptoStoreAggregator()

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt

@@ -19,7 +19,6 @@ package org.matrix.android.sdk.internal.session.sync.handler
 import androidx.work.BackoffPolicy
 import androidx.work.ExistingWorkPolicy
 import org.matrix.android.sdk.api.MatrixPatterns
-import org.matrix.android.sdk.api.session.crypto.crosssigning.CrossSigningService
 import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorker
 import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorkerDataRepository
 import org.matrix.android.sdk.internal.di.SessionId
@@ -39,16 +38,16 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor(
         private val directChatsHelper: DirectChatsHelper,
         private val ephemeralTemporaryStore: RoomSyncEphemeralTemporaryStore,
         private val updateUserAccountDataTask: UpdateUserAccountDataTask,
-        private val crossSigningService: CrossSigningService,
         private val updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository,
         private val workManagerProvider: WorkManagerProvider,
+        private val roomShieldSummaryUpdater: ShieldSummaryUpdater,
         @SessionId private val sessionId: String,
 ) {
     suspend fun handle(aggregator: SyncResponsePostTreatmentAggregator) {
         cleanupEphemeralFiles(aggregator.ephemeralFilesToDelete)
         updateDirectUserIds(aggregator.directChatsToCheck)
         fetchAndUpdateUsers(aggregator.userIdsToFetch)
-        handleUserIdsForCheckingTrustAndAffectedRoomShields(aggregator.userIdsForCheckingTrustAndAffectedRoomShields)
+        handleRefreshRoomShieldsForRooms(aggregator.roomsWithMembershipChangesForShieldUpdate)
     }
 
     private fun cleanupEphemeralFiles(ephemeralFilesToDelete: List<String>) {
@@ -105,8 +104,8 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor(
                 .enqueue()
     }
 
-    private suspend fun handleUserIdsForCheckingTrustAndAffectedRoomShields(userIdsWithDeviceUpdate: Collection<String>) {
+    private fun handleRefreshRoomShieldsForRooms(roomIds: Set<String>) {
-        if (userIdsWithDeviceUpdate.isEmpty()) return
+        if (roomIds.isEmpty()) return
-        crossSigningService.checkTrustAndAffectedRoomShields(userIdsWithDeviceUpdate.toList())
+        roomShieldSummaryUpdater.refreshShieldsForRoomIds(roomIds)
     }
 }

matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/network/OutgoingRequestsProcessor.kt

@@ -27,10 +27,10 @@ import org.matrix.android.sdk.api.MatrixConfiguration
 import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
 import org.matrix.android.sdk.api.logger.LoggerTag
 import org.matrix.android.sdk.api.session.events.model.Event
-import org.matrix.android.sdk.internal.crypto.ComputeShieldForGroupUseCase
 import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider
 import org.matrix.android.sdk.internal.crypto.OlmMachine
 import org.matrix.android.sdk.internal.session.SessionScope
+import org.matrix.android.sdk.internal.session.sync.handler.ShieldSummaryUpdater
 import org.matrix.rustcomponents.sdk.crypto.Request
 import org.matrix.rustcomponents.sdk.crypto.RequestType
 import timber.log.Timber
@@ -43,7 +43,7 @@ internal class OutgoingRequestsProcessor @Inject constructor(
         private val requestSender: RequestSender,
         private val coroutineScope: CoroutineScope,
         private val cryptoSessionInfoProvider: CryptoSessionInfoProvider,
-        private val computeShieldForGroup: ComputeShieldForGroupUseCase,
+        private val shieldSummaryUpdater: ShieldSummaryUpdater,
         private val matrixConfiguration: MatrixConfiguration,
         private val coroutineDispatchers: MatrixCoroutineDispatchers,
 ) {
@@ -137,7 +137,7 @@ internal class OutgoingRequestsProcessor @Inject constructor(
         return try {
             val response = requestSender.queryKeys(request)
             olmMachine.markRequestAsSent(request.requestId, RequestType.KEYS_QUERY, response)
-            coroutineScope.updateShields(olmMachine, request.users)
+            shieldSummaryUpdater.refreshShieldsForRoomsWithMembers(request.users)
             coroutineScope.markMessageVerificationStatesAsDirty(request.users)
             true
         } catch (throwable: Throwable) {
@@ -146,18 +146,6 @@ internal class OutgoingRequestsProcessor @Inject constructor(
         }
     }
 
-    private fun CoroutineScope.updateShields(olmMachine: OlmMachine, userIds: List<String>) = launch(coroutineDispatchers.computation) {
-        cryptoSessionInfoProvider.getRoomsWhereUsersAreParticipating(userIds).forEach { roomId ->
-            if (cryptoSessionInfoProvider.isRoomEncrypted(roomId)) {
-                val userGroup = cryptoSessionInfoProvider.getUserListForShieldComputation(roomId)
-                val shield = computeShieldForGroup(olmMachine, userGroup)
-                cryptoSessionInfoProvider.updateShieldForRoom(roomId, shield)
-            } else {
-                cryptoSessionInfoProvider.updateShieldForRoom(roomId, null)
-            }
-        }
-    }
-
     private fun CoroutineScope.markMessageVerificationStatesAsDirty(userIds: List<String>) = launch(coroutineDispatchers.computation) {
         cryptoSessionInfoProvider.markMessageVerificationStateAsDirty(userIds)
     }

matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2023 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.session.sync.handler
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.launch
+import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
+import org.matrix.android.sdk.internal.crypto.ComputeShieldForGroupUseCase
+import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider
+import org.matrix.android.sdk.internal.crypto.OlmMachine
+import org.matrix.android.sdk.internal.session.SessionScope
+import javax.inject.Inject
+
+@SessionScope
+internal class ShieldSummaryUpdater @Inject constructor(
+        private val olmMachine: dagger.Lazy<OlmMachine>,
+        private val coroutineScope: CoroutineScope,
+        private val coroutineDispatchers: MatrixCoroutineDispatchers,
+        private val cryptoSessionInfoProvider: CryptoSessionInfoProvider,
+        private val computeShieldForGroup: ComputeShieldForGroupUseCase,
+) {
+
+    fun refreshShieldsForRoomsWithMembers(userIds: List<String>) {
+        coroutineScope.launch(coroutineDispatchers.computation) {
+            cryptoSessionInfoProvider.getRoomsWhereUsersAreParticipating(userIds).forEach { roomId ->
+                if (cryptoSessionInfoProvider.isRoomEncrypted(roomId)) {
+                    val userGroup = cryptoSessionInfoProvider.getUserListForShieldComputation(roomId)
+                    val shield = computeShieldForGroup(olmMachine.get(), userGroup)
+                    cryptoSessionInfoProvider.updateShieldForRoom(roomId, shield)
+                } else {
+                    cryptoSessionInfoProvider.updateShieldForRoom(roomId, null)
+                }
+            }
+        }
+    }
+
+    fun refreshShieldsForRoomIds(roomIds: Set<String>) {
+        coroutineScope.launch(coroutineDispatchers.computation) {
+            roomIds.forEach { roomId ->
+                val userGroup = cryptoSessionInfoProvider.getUserListForShieldComputation(roomId)
+                val shield = computeShieldForGroup(olmMachine.get(), userGroup)
+                cryptoSessionInfoProvider.updateShieldForRoom(roomId, shield)
+            }
+        }
+    }
+}