From 01429b352af7616f868b9dc9588d3847360ac3c4 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 27 Dec 2022 16:30:05 +0100 Subject: [PATCH 1/5] Fix inactivity status when last seen timestamp is null --- .../settings/devices/DevicesViewModel.kt | 2 +- .../v2/GetDeviceFullInfoListUseCase.kt | 2 +- .../list/CheckIfSessionIsInactiveUseCase.kt | 14 +++--- .../v2/overview/GetDeviceFullInfoUseCase.kt | 2 +- .../CheckIfSessionIsInactiveUseCaseTest.kt | 50 ++++++++++++++----- 5 files changed, 49 insertions(+), 21 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/DevicesViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/devices/DevicesViewModel.kt index e779948b41..cbc02ba0f0 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/DevicesViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/DevicesViewModel.kt @@ -142,7 +142,7 @@ class DevicesViewModel @AssistedInject constructor( .map { deviceInfo -> val cryptoDeviceInfo = cryptoList.firstOrNull { it.deviceId == deviceInfo.deviceId } val trustLevelForShield = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoDeviceInfo) - val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs ?: 0) + val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs) DeviceFullInfo(deviceInfo, cryptoDeviceInfo, trustLevelForShield, isInactive) } } diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/GetDeviceFullInfoListUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/GetDeviceFullInfoListUseCase.kt index 6adb33d5ab..1ae7d8836f 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/GetDeviceFullInfoListUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/GetDeviceFullInfoListUseCase.kt @@ -75,7 +75,7 @@ class GetDeviceFullInfoListUseCase @Inject constructor( .map { deviceInfo -> val cryptoDeviceInfo = cryptoList.firstOrNull { it.deviceId == deviceInfo.deviceId } val roomEncryptionTrustLevel = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoDeviceInfo) - val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs ?: 0) + val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs) val isCurrentDevice = currentSessionCrossSigningInfo.deviceId == cryptoDeviceInfo?.deviceId val deviceExtendedInfo = parseDeviceUserAgentUseCase.execute(deviceInfo.getBestLastSeenUserAgent()) val matrixClientInfo = deviceInfo.deviceId diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCase.kt index 8991ad1e3d..f3670793bd 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCase.kt @@ -24,11 +24,13 @@ class CheckIfSessionIsInactiveUseCase @Inject constructor( private val clock: Clock, ) { - fun execute(lastSeenTs: Long): Boolean { - // In case of the server doesn't send the last seen date. - if (lastSeenTs == 0L) return true - - val diffMilliseconds = clock.epochMillis() - lastSeenTs - return diffMilliseconds >= TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) + fun execute(lastSeenTsMillis: Long?): Boolean { + return if (lastSeenTsMillis == null || lastSeenTsMillis <= 0) { + // in these situations we cannot say anything about the inactivity of the session + false + } else { + val diffMilliseconds = clock.epochMillis() - lastSeenTsMillis + diffMilliseconds >= TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) + } } } diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt index 140b55a30c..3b67eee780 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt @@ -51,7 +51,7 @@ class GetDeviceFullInfoUseCase @Inject constructor( val cryptoInfo = cryptoDeviceInfo.getOrNull() val fullInfo = if (info != null && cryptoInfo != null) { val roomEncryptionTrustLevel = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoInfo) - val isInactive = checkIfSessionIsInactiveUseCase.execute(info.lastSeenTs ?: 0) + val isInactive = checkIfSessionIsInactiveUseCase.execute(info.lastSeenTs) val isCurrentDevice = currentSessionCrossSigningInfo.deviceId == cryptoInfo.deviceId val deviceUserAgent = parseDeviceUserAgentUseCase.execute(info.getBestLastSeenUserAgent()) val matrixClientInfo = info.deviceId diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCaseTest.kt index b7d56a88bf..20291e5ee2 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCaseTest.kt @@ -17,43 +17,69 @@ package im.vector.app.features.settings.devices.v2.list import im.vector.app.test.fakes.FakeClock -import org.amshove.kluent.shouldBeEqualTo +import org.amshove.kluent.shouldBeFalse +import org.amshove.kluent.shouldBeTrue import org.junit.Test import java.util.concurrent.TimeUnit -private const val A_TIMESTAMP = 1654689143L +private const val A_TIMESTAMP_MILLIS = 1654689143000L class CheckIfSessionIsInactiveUseCaseTest { - private val clock = FakeClock().apply { givenEpoch(A_TIMESTAMP) } + private val clock = FakeClock().apply { givenEpoch(A_TIMESTAMP_MILLIS) } private val checkIfSessionIsInactiveUseCase = CheckIfSessionIsInactiveUseCase(clock) @Test fun `given an old last seen date then session is inactive`() { - val lastSeenDate = A_TIMESTAMP - TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) - 1 + val lastSeenDate = A_TIMESTAMP_MILLIS - TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) - 1 - checkIfSessionIsInactiveUseCase.execute(lastSeenDate) shouldBeEqualTo true + val result = checkIfSessionIsInactiveUseCase.execute(lastSeenDate) + + result.shouldBeTrue() } @Test fun `given a last seen date equal to the threshold then session is inactive`() { - val lastSeenDate = A_TIMESTAMP - TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) + val lastSeenDate = A_TIMESTAMP_MILLIS - TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) - checkIfSessionIsInactiveUseCase.execute(lastSeenDate) shouldBeEqualTo true + val result = checkIfSessionIsInactiveUseCase.execute(lastSeenDate) + + result.shouldBeTrue() } @Test fun `given a recent last seen date then session is active`() { - val lastSeenDate = A_TIMESTAMP - TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) + 1 + val lastSeenDate = A_TIMESTAMP_MILLIS - TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong()) + 1 - checkIfSessionIsInactiveUseCase.execute(lastSeenDate) shouldBeEqualTo false + val result = checkIfSessionIsInactiveUseCase.execute(lastSeenDate) + + result.shouldBeFalse() } @Test - fun `given a last seen date as zero then session is inactive`() { - // In case of the server doesn't send the last seen date. + fun `given a last seen date as zero then session is not inactive`() { val lastSeenDate = 0L - checkIfSessionIsInactiveUseCase.execute(lastSeenDate) shouldBeEqualTo true + val result = checkIfSessionIsInactiveUseCase.execute(lastSeenDate) + + result.shouldBeFalse() + } + + @Test + fun `given a last seen date as null then session is not inactive`() { + val lastSeenDate = null + + val result = checkIfSessionIsInactiveUseCase.execute(lastSeenDate) + + result.shouldBeFalse() + } + + @Test + fun `given a last seen date as negative then session is not inactive`() { + val lastSeenDate = -3L + + val result = checkIfSessionIsInactiveUseCase.execute(lastSeenDate) + + result.shouldBeFalse() } } From 1af712910f431712a77374702a6064b5c839177e Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 27 Dec 2022 16:41:25 +0100 Subject: [PATCH 2/5] Use deviceId as session name when there is no display name --- .../settings/devices/v2/list/OtherSessionsController.kt | 3 ++- .../features/settings/devices/v2/list/SessionInfoView.kt | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/OtherSessionsController.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/OtherSessionsController.kt index 5e2549f42a..e85e394681 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/OtherSessionsController.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/OtherSessionsController.kt @@ -63,12 +63,13 @@ class OtherSessionsController @Inject constructor( } val drawableColor = host.colorProvider.getColorFromAttribute(R.attr.vctr_content_secondary) val descriptionDrawable = if (device.isInactive) host.drawableProvider.getDrawable(R.drawable.ic_inactive_sessions, drawableColor) else null + val sessionName = device.deviceInfo.displayName ?: device.deviceInfo.deviceId otherSessionItem { id(device.deviceInfo.deviceId) deviceType(device.deviceExtendedInfo.deviceType) roomEncryptionTrustLevel(device.roomEncryptionTrustLevel) - sessionName(device.deviceInfo.displayName) + sessionName(sessionName) sessionDescription(description) sessionDescriptionDrawable(descriptionDrawable) sessionDescriptionColor(descriptionColor) diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/SessionInfoView.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/SessionInfoView.kt index 5d2daf2941..81a8aae666 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/SessionInfoView.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/list/SessionInfoView.kt @@ -62,9 +62,10 @@ class SessionInfoView @JvmOverloads constructor( stringProvider: StringProvider, ) { renderDeviceInfo( - sessionInfoViewState.deviceFullInfo.deviceInfo.displayName.orEmpty(), - sessionInfoViewState.deviceFullInfo.deviceExtendedInfo.deviceType, - stringProvider, + sessionName = sessionInfoViewState.deviceFullInfo.deviceInfo.displayName + ?: sessionInfoViewState.deviceFullInfo.deviceInfo.deviceId.orEmpty(), + deviceType = sessionInfoViewState.deviceFullInfo.deviceExtendedInfo.deviceType, + stringProvider = stringProvider, ) renderVerificationStatus( sessionInfoViewState.deviceFullInfo.roomEncryptionTrustLevel, From 6fdb1216ba5781a61f85baca1188b2e1b53020c3 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 27 Dec 2022 17:24:32 +0100 Subject: [PATCH 3/5] Fixing missing session info when there is no crypto info --- .../v2/overview/GetDeviceFullInfoUseCase.kt | 4 +- .../overview/GetDeviceFullInfoUseCaseTest.kt | 47 +++++++++++++++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt index 3b67eee780..445d2309a4 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt @@ -49,10 +49,10 @@ class GetDeviceFullInfoUseCase @Inject constructor( ) { currentSessionCrossSigningInfo, deviceInfo, cryptoDeviceInfo -> val info = deviceInfo.getOrNull() val cryptoInfo = cryptoDeviceInfo.getOrNull() - val fullInfo = if (info != null && cryptoInfo != null) { + val fullInfo = if (info != null) { val roomEncryptionTrustLevel = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoInfo) val isInactive = checkIfSessionIsInactiveUseCase.execute(info.lastSeenTs) - val isCurrentDevice = currentSessionCrossSigningInfo.deviceId == cryptoInfo.deviceId + val isCurrentDevice = currentSessionCrossSigningInfo.deviceId == info.deviceId val deviceUserAgent = parseDeviceUserAgentUseCase.execute(info.getBestLastSeenUserAgent()) val matrixClientInfo = info.deviceId ?.takeIf { it.isNotEmpty() } diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCaseTest.kt index 2185c295d0..48c5ce74b4 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCaseTest.kt @@ -117,6 +117,45 @@ class GetDeviceFullInfoUseCaseTest { } } + @Test + fun `given current session and no crypto info for device when getting device info then the result is correct`() = runTest { + // Given + val currentSessionCrossSigningInfo = givenCurrentSessionCrossSigningInfo() + val deviceInfo = givenADeviceInfo() + val cryptoDeviceInfo = null + val trustLevel = givenTrustLevel(currentSessionCrossSigningInfo, cryptoDeviceInfo) + val isInactive = false + val isCurrentDevice = true + every { checkIfSessionIsInactiveUseCase.execute(any()) } returns isInactive + every { parseDeviceUserAgentUseCase.execute(any()) } returns DeviceExtendedInfo(DeviceType.MOBILE) + val matrixClientInfo = givenAMatrixClientInfo() + fakeActiveSessionHolder.fakeSession.fakeCryptoService.cryptoDeviceInfoWithIdLiveData = MutableLiveData(Optional(null)) + fakeActiveSessionHolder.fakeSession.fakeCryptoService.cryptoDeviceInfoWithIdLiveData.givenAsFlow() + + // When + val deviceFullInfo = getDeviceFullInfoUseCase.execute(A_DEVICE_ID).firstOrNull() + + // Then + deviceFullInfo shouldBeEqualTo DeviceFullInfo( + deviceInfo = deviceInfo, + cryptoDeviceInfo = cryptoDeviceInfo, + roomEncryptionTrustLevel = trustLevel, + isInactive = isInactive, + isCurrentDevice = isCurrentDevice, + deviceExtendedInfo = DeviceExtendedInfo(DeviceType.MOBILE), + matrixClientInfo = matrixClientInfo, + ) + verify { + fakeActiveSessionHolder.instance.getSafeActiveSession() + getCurrentSessionCrossSigningInfoUseCase.execute() + getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoDeviceInfo) + fakeActiveSessionHolder.fakeSession.fakeCryptoService.getMyDevicesInfoLive(A_DEVICE_ID).asFlow() + fakeActiveSessionHolder.fakeSession.fakeCryptoService.getLiveCryptoDeviceInfoWithId(A_DEVICE_ID).asFlow() + checkIfSessionIsInactiveUseCase.execute(A_TIMESTAMP) + getMatrixClientInfoUseCase.execute(fakeActiveSessionHolder.fakeSession, A_DEVICE_ID) + } + } + @Test fun `given current session and no info for device when getting device info then the result is empty`() = runTest { // Given @@ -131,9 +170,11 @@ class GetDeviceFullInfoUseCaseTest { // Then deviceFullInfo.shouldBeNull() - verify { fakeActiveSessionHolder.instance.getSafeActiveSession() } - verify { fakeActiveSessionHolder.fakeSession.fakeCryptoService.getMyDevicesInfoLive(A_DEVICE_ID).asFlow() } - verify { fakeActiveSessionHolder.fakeSession.fakeCryptoService.getLiveCryptoDeviceInfoWithId(A_DEVICE_ID).asFlow() } + verify { + fakeActiveSessionHolder.instance.getSafeActiveSession() + fakeActiveSessionHolder.fakeSession.fakeCryptoService.getMyDevicesInfoLive(A_DEVICE_ID).asFlow() + fakeActiveSessionHolder.fakeSession.fakeCryptoService.getLiveCryptoDeviceInfoWithId(A_DEVICE_ID).asFlow() + } } @Test From fa7766f8a68755b3196c239c76daf17ae035f271 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 27 Dec 2022 17:24:45 +0100 Subject: [PATCH 4/5] Fixing missing device without encryption support in the unverified session list --- .../settings/devices/v2/DevicesViewModel.kt | 40 +++++++++---------- .../settings/devices/v2/DevicesViewState.kt | 10 +++-- .../v2/VectorSettingsDevicesFragment.kt | 12 +++--- .../devices/v2/filter/FilterDevicesUseCase.kt | 4 +- .../devices/v2/DevicesViewModelTest.kt | 23 ++++++----- .../v2/filter/FilterDevicesUseCaseTest.kt | 19 +++++++-- 6 files changed, 64 insertions(+), 44 deletions(-) diff --git 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 index 232fcd50f7..f2e6b25f32 100644 --- 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 { val currentDeviceId = state.currentSessionCrossSigningInfo.deviceId return state.devices() + ?.allSessions ?.mapNotNull { fullInfo -> fullInfo.deviceInfo.deviceId.takeUnless { it == currentDeviceId } } .orEmpty() } diff --git 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 index e0531c34dc..75d0f132bb 100644 --- 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> = Uninitialized, - val unverifiedSessionsCount: Int = 0, - val inactiveSessionsCount: Int = 0, + val devices: Async = Uninitialized, val isLoading: Boolean = false, val isShowingIpAddress: Boolean = false, ) : MavericksState + +data class DeviceFullInfoList( + val allSessions: List, + val unverifiedSessionsCount: Int, + val inactiveSessionsCount: Int, +) diff --git 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 index 15375ef679..f83b00a75e 100644 --- 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() diff --git 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 index 8f23fd06cc..b1f23eb510 100644 --- 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 } } diff --git 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 index 524858da77..79c998ff5c 100644 --- 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 { + private fun givenDeviceFullInfoList(deviceId1: String, deviceId2: String): DeviceFullInfoList { val verifiedCryptoDeviceInfo = mockk() every { verifiedCryptoDeviceInfo.trustLevel } returns DeviceTrustLevel(crossSigningVerified = true, locallyVerified = true) val unverifiedCryptoDeviceInfo = mockk() @@ -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, ) } diff --git 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 index 79dff5bc16..2e8a81bf54 100644 --- 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 From 5373771566311366b3f0a8eaaa770d5009b843d6 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Wed, 28 Dec 2022 09:55:22 +0100 Subject: [PATCH 5/5] Adding changelog entry --- changelog.d/7853.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7853.bugfix diff --git a/changelog.d/7853.bugfix b/changelog.d/7853.bugfix new file mode 100644 index 0000000000..885233553e --- /dev/null +++ b/changelog.d/7853.bugfix @@ -0,0 +1 @@ +[Session manager] Missing info when a session does not support encryption