Fixing missing device without encryption support in the unverified session list

2022-12-27 17:24:45 +01:00 · 2022-12-27 17:24:45 +01:00 · fa7766f8a6
commit fa7766f8a6
parent 6fdb1216ba
6 changed files with 64 additions and 44 deletions
--- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt
@ -18,7 +18,6 @@ package im.vector.app.features.settings.devices.v2

 import android.content.SharedPreferences
 import com.airbnb.mvrx.MavericksViewModelFactory
-import com.airbnb.mvrx.Success
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
@ -32,10 +31,10 @@ import im.vector.app.features.settings.devices.v2.signout.SignoutSessionsReAuthN
 import im.vector.app.features.settings.devices.v2.signout.SignoutSessionsUseCase
 import im.vector.app.features.settings.devices.v2.verification.CheckIfCurrentSessionCanBeVerifiedUseCase
 import im.vector.app.features.settings.devices.v2.verification.GetCurrentSessionCrossSigningInfoUseCase
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
-import org.matrix.android.sdk.api.extensions.orFalse
 import org.matrix.android.sdk.api.session.uia.DefaultBaseAuth
 import timber.log.Timber

@ -103,27 +102,27 @@ class DevicesViewModel @AssistedInject constructor(
    }

    private fun observeDevices() {
-        getDeviceFullInfoListUseCase.execute(
+        val allSessionsFlow = getDeviceFullInfoListUseCase.execute(
                filterType = DeviceManagerFilterType.ALL_SESSIONS,
-                excludeCurrentDevice = false
+                excludeCurrentDevice = false,
+        )
+        val unverifiedSessionsFlow = getDeviceFullInfoListUseCase.execute(
+                filterType = DeviceManagerFilterType.UNVERIFIED,
+                excludeCurrentDevice = true,
+        )
+        val inactiveSessionsFlow = getDeviceFullInfoListUseCase.execute(
+                filterType = DeviceManagerFilterType.INACTIVE,
+                excludeCurrentDevice = true,
        )
-                .execute { async ->
-                    if (async is Success) {
-                        val deviceFullInfoList = async.invoke()
-                        val unverifiedSessionsCount = deviceFullInfoList.count { !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse() }
-                        val inactiveSessionsCount = deviceFullInfoList.count { it.isInactive }

-                        copy(
-                                devices = async,
-                                unverifiedSessionsCount = unverifiedSessionsCount,
-                                inactiveSessionsCount = inactiveSessionsCount,
-                        )
-                    } else {
-                        copy(
-                                devices = async
-                        )
-                    }
-                }
+        combine(allSessionsFlow, unverifiedSessionsFlow, inactiveSessionsFlow) { allSessions, unverifiedSessions, inactiveSessions ->
+            DeviceFullInfoList(
+                    allSessions = allSessions,
+                    unverifiedSessionsCount = unverifiedSessions.size,
+                    inactiveSessionsCount = inactiveSessions.size,
+            )
+        }
+                .execute { async -> copy(devices = async) }
    }

    private fun refreshDevicesOnCryptoDevicesChange() {
@ -185,6 +184,7 @@ class DevicesViewModel @AssistedInject constructor(
    private fun getDeviceIdsOfOtherSessions(state: DevicesViewState): List<String> {
        val currentDeviceId = state.currentSessionCrossSigningInfo.deviceId
        return state.devices()
+                ?.allSessions
                ?.mapNotNull { fullInfo -> fullInfo.deviceInfo.deviceId.takeUnless { it == currentDeviceId } }
                .orEmpty()
    }
--- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewState.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewState.kt
@ -23,9 +23,13 @@ import im.vector.app.features.settings.devices.v2.verification.CurrentSessionCro

 data class DevicesViewState(
        val currentSessionCrossSigningInfo: CurrentSessionCrossSigningInfo = CurrentSessionCrossSigningInfo(),
-        val devices: Async<List<DeviceFullInfo>> = Uninitialized,
-        val unverifiedSessionsCount: Int = 0,
-        val inactiveSessionsCount: Int = 0,
+        val devices: Async<DeviceFullInfoList> = Uninitialized,
        val isLoading: Boolean = false,
        val isShowingIpAddress: Boolean = false,
 ) : MavericksState
+
+data class DeviceFullInfoList(
+        val allSessions: List<DeviceFullInfo>,
+        val unverifiedSessionsCount: Int,
+        val inactiveSessionsCount: Int,
+)
--- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/VectorSettingsDevicesFragment.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/VectorSettingsDevicesFragment.kt
@ -55,7 +55,6 @@ import im.vector.app.features.settings.devices.v2.signout.BuildConfirmSignoutDia
 import im.vector.app.features.workers.signout.SignOutUiWorker
 import org.matrix.android.sdk.api.auth.data.LoginFlowTypes
 import org.matrix.android.sdk.api.extensions.orFalse
-import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
 import javax.inject.Inject

 /**
@ -282,13 +281,15 @@ class VectorSettingsDevicesFragment :

    override fun invalidate() = withState(viewModel) { state ->
        if (state.devices is Success) {
-            val devices = state.devices()
+            val deviceFullInfoList = state.devices()
+            val devices = deviceFullInfoList?.allSessions
            val currentDeviceId = state.currentSessionCrossSigningInfo.deviceId
            val currentDeviceInfo = devices?.firstOrNull { it.deviceInfo.deviceId == currentDeviceId }
-            val isCurrentSessionVerified = currentDeviceInfo?.roomEncryptionTrustLevel == RoomEncryptionTrustLevel.Trusted
            val otherDevices = devices?.filter { it.deviceInfo.deviceId != currentDeviceId }
+            val inactiveSessionsCount = deviceFullInfoList?.inactiveSessionsCount ?: 0
+            val unverifiedSessionsCount = deviceFullInfoList?.unverifiedSessionsCount ?: 0

-            renderSecurityRecommendations(state.inactiveSessionsCount, state.unverifiedSessionsCount, isCurrentSessionVerified)
+            renderSecurityRecommendations(inactiveSessionsCount, unverifiedSessionsCount)
            renderCurrentSessionView(currentDeviceInfo, hasOtherDevices = otherDevices?.isNotEmpty().orFalse())
            renderOtherSessionsView(otherDevices, state.isShowingIpAddress)
        } else {
@ -303,9 +304,8 @@ class VectorSettingsDevicesFragment :
    private fun renderSecurityRecommendations(
            inactiveSessionsCount: Int,
            unverifiedSessionsCount: Int,
-            isCurrentSessionVerified: Boolean,
    ) {
-        val isUnverifiedSectionVisible = unverifiedSessionsCount > 0 && isCurrentSessionVerified
+        val isUnverifiedSectionVisible = unverifiedSessionsCount > 0
        val isInactiveSectionVisible = inactiveSessionsCount > 0
        if (isUnverifiedSectionVisible.not() && isInactiveSectionVisible.not()) {
            hideSecurityRecommendations()
--- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/filter/FilterDevicesUseCase.kt
+++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/filter/FilterDevicesUseCase.kt
@ -37,7 +37,9 @@ class FilterDevicesUseCase @Inject constructor() {
                        // when current session is not verified, other session status cannot be trusted
                        DeviceManagerFilterType.VERIFIED -> isCurrentSessionVerified && it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse()
                        // when current session is not verified, other session status cannot be trusted
-                        DeviceManagerFilterType.UNVERIFIED -> isCurrentSessionVerified && !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse()
+                        DeviceManagerFilterType.UNVERIFIED ->
+                            (isCurrentSessionVerified && !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse()) ||
+                                    it.cryptoDeviceInfo == null
                        DeviceManagerFilterType.INACTIVE -> it.isInactive
                    }
                }
--- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/DevicesViewModelTest.kt
+++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/DevicesViewModelTest.kt
@ -21,6 +21,7 @@ import com.airbnb.mvrx.Success
 import com.airbnb.mvrx.test.MavericksTestRule
 import im.vector.app.core.session.clientinfo.MatrixClientInfoContent
 import im.vector.app.features.settings.devices.v2.details.extended.DeviceExtendedInfo
+import im.vector.app.features.settings.devices.v2.filter.DeviceManagerFilterType
 import im.vector.app.features.settings.devices.v2.list.DeviceType
 import im.vector.app.features.settings.devices.v2.verification.CheckIfCurrentSessionCanBeVerifiedUseCase
 import im.vector.app.features.settings.devices.v2.verification.CurrentSessionCrossSigningInfo
@ -176,10 +177,7 @@ class DevicesViewModelTest {
        val viewModelTest = createViewModel().test()

        // Then
-        viewModelTest.assertLatestState {
-            it.devices is Success && it.devices.invoke() == deviceFullInfoList &&
-                    it.inactiveSessionsCount == 1 && it.unverifiedSessionsCount == 1
-        }
+        viewModelTest.assertLatestState { it.devices is Success && it.devices.invoke() == deviceFullInfoList }
        viewModelTest.finish()
    }

@ -403,7 +401,7 @@ class DevicesViewModelTest {
    /**
     * Generate mocked deviceFullInfo list with 1 unverified and inactive + 1 verified and active.
     */
-    private fun givenDeviceFullInfoList(deviceId1: String, deviceId2: String): List<DeviceFullInfo> {
+    private fun givenDeviceFullInfoList(deviceId1: String, deviceId2: String): DeviceFullInfoList {
        val verifiedCryptoDeviceInfo = mockk<CryptoDeviceInfo>()
        every { verifiedCryptoDeviceInfo.trustLevel } returns DeviceTrustLevel(crossSigningVerified = true, locallyVerified = true)
        val unverifiedCryptoDeviceInfo = mockk<CryptoDeviceInfo>()
@ -432,10 +430,15 @@ class DevicesViewModelTest {
                deviceExtendedInfo = DeviceExtendedInfo(DeviceType.MOBILE),
                matrixClientInfo = MatrixClientInfoContent(),
        )
-        val deviceFullInfoList = listOf(deviceFullInfo1, deviceFullInfo2)
-        val deviceFullInfoListFlow = flowOf(deviceFullInfoList)
-        every { getDeviceFullInfoListUseCase.execute(any(), any()) } returns deviceFullInfoListFlow
-        return deviceFullInfoList
+        val devices = listOf(deviceFullInfo1, deviceFullInfo2)
+        every { getDeviceFullInfoListUseCase.execute(DeviceManagerFilterType.ALL_SESSIONS, any()) } returns flowOf(devices)
+        every { getDeviceFullInfoListUseCase.execute(DeviceManagerFilterType.UNVERIFIED, any()) } returns flowOf(listOf(deviceFullInfo2))
+        every { getDeviceFullInfoListUseCase.execute(DeviceManagerFilterType.INACTIVE, any()) } returns flowOf(listOf(deviceFullInfo1))
+        return DeviceFullInfoList(
+                allSessions = devices,
+                unverifiedSessionsCount = 1,
+                inactiveSessionsCount = 1,
+        )
    }

    private fun givenInitialViewState(deviceId1: String, deviceId2: String): DevicesViewState {
@ -444,8 +447,6 @@ class DevicesViewModelTest {
        return DevicesViewState(
                currentSessionCrossSigningInfo = currentSessionCrossSigningInfo,
                devices = Success(deviceFullInfoList),
-                unverifiedSessionsCount = 1,
-                inactiveSessionsCount = 1,
                isLoading = false,
        )
    }
--- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/filter/FilterDevicesUseCaseTest.kt
+++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/filter/FilterDevicesUseCaseTest.kt
@ -22,6 +22,7 @@ import im.vector.app.features.settings.devices.v2.details.extended.DeviceExtende
 import im.vector.app.features.settings.devices.v2.list.DeviceType
 import im.vector.app.features.settings.devices.v2.verification.CurrentSessionCrossSigningInfo
 import org.amshove.kluent.shouldBeEqualTo
+import org.amshove.kluent.shouldContain
 import org.amshove.kluent.shouldContainAll
 import org.junit.Test
 import org.matrix.android.sdk.api.session.crypto.crosssigning.DeviceTrustLevel
@ -82,11 +83,22 @@ private val inactiveUnverifiedDevice = DeviceFullInfo(
        matrixClientInfo = MatrixClientInfoContent(),
 )

+private val deviceWithoutEncryptionSupport = DeviceFullInfo(
+        deviceInfo = DeviceInfo(deviceId = "DEVICE_WITHOUT_ENCRYPTION_SUPPORT"),
+        cryptoDeviceInfo = null,
+        roomEncryptionTrustLevel = null,
+        isInactive = false,
+        isCurrentDevice = false,
+        deviceExtendedInfo = DeviceExtendedInfo(DeviceType.UNKNOWN),
+        matrixClientInfo = MatrixClientInfoContent(),
+)
+
 private val devices = listOf(
        activeVerifiedDevice,
        inactiveVerifiedDevice,
        activeUnverifiedDevice,
        inactiveUnverifiedDevice,
+        deviceWithoutEncryptionSupport,
 )

 class FilterDevicesUseCaseTest {
@ -123,8 +135,8 @@ class FilterDevicesUseCaseTest {
        val currentSessionCrossSigningInfo = givenCurrentSessionVerified(true)
        val filteredDeviceList = filterDevicesUseCase.execute(currentSessionCrossSigningInfo, devices, DeviceManagerFilterType.UNVERIFIED, emptyList())

-        filteredDeviceList.size shouldBeEqualTo 2
-        filteredDeviceList shouldContainAll listOf(activeUnverifiedDevice, inactiveUnverifiedDevice)
+        filteredDeviceList.size shouldBeEqualTo 3
+        filteredDeviceList shouldContainAll listOf(activeUnverifiedDevice, inactiveUnverifiedDevice, deviceWithoutEncryptionSupport)
    }

    @Test
@ -132,7 +144,8 @@ class FilterDevicesUseCaseTest {
        val currentSessionCrossSigningInfo = givenCurrentSessionVerified(false)
        val filteredDeviceList = filterDevicesUseCase.execute(currentSessionCrossSigningInfo, devices, DeviceManagerFilterType.UNVERIFIED, emptyList())

-        filteredDeviceList.size shouldBeEqualTo 0
+        filteredDeviceList.size shouldBeEqualTo 1
+        filteredDeviceList shouldContain deviceWithoutEncryptionSupport
    }

    @Test