Skip to content

Commit f1bd9b2

Browse files
mnaturelmnaturel
authored
Merge pull request element-hq#7854 from vector-im/fix/mna/info-session-without-crypto-support
[Session manager] Missing info when a session does not support encryption (PSG-1074)
2 parents c6a0a03 + 5373771 commit f1bd9b2

15 files changed

+167
-75
lines changed

‎changelog.d/7853.bugfix

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[Session manager] Missing info when a session does not support encryption

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ class DevicesViewModel @AssistedInject constructor(
142142
.map { deviceInfo ->
143143
val cryptoDeviceInfo = cryptoList.firstOrNull { it.deviceId == deviceInfo.deviceId }
144144
val trustLevelForShield = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoDeviceInfo)
145-
val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs ?: 0)
145+
val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs)
146146
DeviceFullInfo(deviceInfo, cryptoDeviceInfo, trustLevelForShield, isInactive)
147147
}
148148
}

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt

+21-21
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package im.vector.app.features.settings.devices.v2
1818

1919
import android.content.SharedPreferences
2020
import com.airbnb.mvrx.MavericksViewModelFactory
21-
import com.airbnb.mvrx.Success
2221
import dagger.assisted.Assisted
2322
import dagger.assisted.AssistedFactory
2423
import dagger.assisted.AssistedInject
@@ -32,10 +31,10 @@ import im.vector.app.features.settings.devices.v2.signout.SignoutSessionsReAuthN
3231
import im.vector.app.features.settings.devices.v2.signout.SignoutSessionsUseCase
3332
import im.vector.app.features.settings.devices.v2.verification.CheckIfCurrentSessionCanBeVerifiedUseCase
3433
import im.vector.app.features.settings.devices.v2.verification.GetCurrentSessionCrossSigningInfoUseCase
34+
import kotlinx.coroutines.flow.combine
3535
import kotlinx.coroutines.flow.launchIn
3636
import kotlinx.coroutines.flow.onEach
3737
import kotlinx.coroutines.launch
38-
import org.matrix.android.sdk.api.extensions.orFalse
3938
import org.matrix.android.sdk.api.session.uia.DefaultBaseAuth
4039
import timber.log.Timber
4140

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

105104
private fun observeDevices() {
106-
getDeviceFullInfoListUseCase.execute(
105+
val allSessionsFlow = getDeviceFullInfoListUseCase.execute(
107106
filterType = DeviceManagerFilterType.ALL_SESSIONS,
108-
excludeCurrentDevice = false
107+
excludeCurrentDevice = false,
109108
)
110-
.execute { async ->
111-
if (async is Success) {
112-
val deviceFullInfoList = async.invoke()
113-
val unverifiedSessionsCount = deviceFullInfoList.count { !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse() }
114-
val inactiveSessionsCount = deviceFullInfoList.count { it.isInactive }
115-
116-
copy(
117-
devices = async,
118-
unverifiedSessionsCount = unverifiedSessionsCount,
119-
inactiveSessionsCount = inactiveSessionsCount,
120-
)
121-
} else {
122-
copy(
123-
devices = async
124-
)
125-
}
126-
}
109+
val unverifiedSessionsFlow = getDeviceFullInfoListUseCase.execute(
110+
filterType = DeviceManagerFilterType.UNVERIFIED,
111+
excludeCurrentDevice = true,
112+
)
113+
val inactiveSessionsFlow = getDeviceFullInfoListUseCase.execute(
114+
filterType = DeviceManagerFilterType.INACTIVE,
115+
excludeCurrentDevice = true,
116+
)
117+
118+
combine(allSessionsFlow, unverifiedSessionsFlow, inactiveSessionsFlow) { allSessions, unverifiedSessions, inactiveSessions ->
119+
DeviceFullInfoList(
120+
allSessions = allSessions,
121+
unverifiedSessionsCount = unverifiedSessions.size,
122+
inactiveSessionsCount = inactiveSessions.size,
123+
)
124+
}
125+
.execute { async -> copy(devices = async) }
127126
}
128127

129128
private fun refreshDevicesOnCryptoDevicesChange() {
@@ -185,6 +184,7 @@ class DevicesViewModel @AssistedInject constructor(
185184
private fun getDeviceIdsOfOtherSessions(state: DevicesViewState): List<String> {
186185
val currentDeviceId = state.currentSessionCrossSigningInfo.deviceId
187186
return state.devices()
187+
?.allSessions
188188
?.mapNotNull { fullInfo -> fullInfo.deviceInfo.deviceId.takeUnless { it == currentDeviceId } }
189189
.orEmpty()
190190
}

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewState.kt

+7-3
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ import im.vector.app.features.settings.devices.v2.verification.CurrentSessionCro
2323

2424
data class DevicesViewState(
2525
val currentSessionCrossSigningInfo: CurrentSessionCrossSigningInfo = CurrentSessionCrossSigningInfo(),
26-
val devices: Async<List<DeviceFullInfo>> = Uninitialized,
27-
val unverifiedSessionsCount: Int = 0,
28-
val inactiveSessionsCount: Int = 0,
26+
val devices: Async<DeviceFullInfoList> = Uninitialized,
2927
val isLoading: Boolean = false,
3028
val isShowingIpAddress: Boolean = false,
3129
) : MavericksState
30+
31+
data class DeviceFullInfoList(
32+
val allSessions: List<DeviceFullInfo>,
33+
val unverifiedSessionsCount: Int,
34+
val inactiveSessionsCount: Int,
35+
)

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/GetDeviceFullInfoListUseCase.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class GetDeviceFullInfoListUseCase @Inject constructor(
7575
.map { deviceInfo ->
7676
val cryptoDeviceInfo = cryptoList.firstOrNull { it.deviceId == deviceInfo.deviceId }
7777
val roomEncryptionTrustLevel = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoDeviceInfo)
78-
val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs ?: 0)
78+
val isInactive = checkIfSessionIsInactiveUseCase.execute(deviceInfo.lastSeenTs)
7979
val isCurrentDevice = currentSessionCrossSigningInfo.deviceId == cryptoDeviceInfo?.deviceId
8080
val deviceExtendedInfo = parseDeviceUserAgentUseCase.execute(deviceInfo.getBestLastSeenUserAgent())
8181
val matrixClientInfo = deviceInfo.deviceId

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/VectorSettingsDevicesFragment.kt

+6-6
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ import im.vector.app.features.settings.devices.v2.signout.BuildConfirmSignoutDia
5555
import im.vector.app.features.workers.signout.SignOutUiWorker
5656
import org.matrix.android.sdk.api.auth.data.LoginFlowTypes
5757
import org.matrix.android.sdk.api.extensions.orFalse
58-
import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
5958
import javax.inject.Inject
6059

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

283282
override fun invalidate() = withState(viewModel) { state ->
284283
if (state.devices is Success) {
285-
val devices = state.devices()
284+
val deviceFullInfoList = state.devices()
285+
val devices = deviceFullInfoList?.allSessions
286286
val currentDeviceId = state.currentSessionCrossSigningInfo.deviceId
287287
val currentDeviceInfo = devices?.firstOrNull { it.deviceInfo.deviceId == currentDeviceId }
288-
val isCurrentSessionVerified = currentDeviceInfo?.roomEncryptionTrustLevel == RoomEncryptionTrustLevel.Trusted
289288
val otherDevices = devices?.filter { it.deviceInfo.deviceId != currentDeviceId }
289+
val inactiveSessionsCount = deviceFullInfoList?.inactiveSessionsCount ?: 0
290+
val unverifiedSessionsCount = deviceFullInfoList?.unverifiedSessionsCount ?: 0
290291

291-
renderSecurityRecommendations(state.inactiveSessionsCount, state.unverifiedSessionsCount, isCurrentSessionVerified)
292+
renderSecurityRecommendations(inactiveSessionsCount, unverifiedSessionsCount)
292293
renderCurrentSessionView(currentDeviceInfo, hasOtherDevices = otherDevices?.isNotEmpty().orFalse())
293294
renderOtherSessionsView(otherDevices, state.isShowingIpAddress)
294295
} else {
@@ -303,9 +304,8 @@ class VectorSettingsDevicesFragment :
303304
private fun renderSecurityRecommendations(
304305
inactiveSessionsCount: Int,
305306
unverifiedSessionsCount: Int,
306-
isCurrentSessionVerified: Boolean,
307307
) {
308-
val isUnverifiedSectionVisible = unverifiedSessionsCount > 0 && isCurrentSessionVerified
308+
val isUnverifiedSectionVisible = unverifiedSessionsCount > 0
309309
val isInactiveSectionVisible = inactiveSessionsCount > 0
310310
if (isUnverifiedSectionVisible.not() && isInactiveSectionVisible.not()) {
311311
hideSecurityRecommendations()

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/filter/FilterDevicesUseCase.kt

+3-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ class FilterDevicesUseCase @Inject constructor() {
3737
// when current session is not verified, other session status cannot be trusted
3838
DeviceManagerFilterType.VERIFIED -> isCurrentSessionVerified && it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse()
3939
// when current session is not verified, other session status cannot be trusted
40-
DeviceManagerFilterType.UNVERIFIED -> isCurrentSessionVerified && !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse()
40+
DeviceManagerFilterType.UNVERIFIED ->
41+
(isCurrentSessionVerified && !it.cryptoDeviceInfo?.trustLevel?.isCrossSigningVerified().orFalse()) ||
42+
it.cryptoDeviceInfo == null
4143
DeviceManagerFilterType.INACTIVE -> it.isInactive
4244
}
4345
}

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/list/CheckIfSessionIsInactiveUseCase.kt

+8-6
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@ class CheckIfSessionIsInactiveUseCase @Inject constructor(
2424
private val clock: Clock,
2525
) {
2626

27-
fun execute(lastSeenTs: Long): Boolean {
28-
// In case of the server doesn't send the last seen date.
29-
if (lastSeenTs == 0L) return true
30-
31-
val diffMilliseconds = clock.epochMillis() - lastSeenTs
32-
return diffMilliseconds >= TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong())
27+
fun execute(lastSeenTsMillis: Long?): Boolean {
28+
return if (lastSeenTsMillis == null || lastSeenTsMillis <= 0) {
29+
// in these situations we cannot say anything about the inactivity of the session
30+
false
31+
} else {
32+
val diffMilliseconds = clock.epochMillis() - lastSeenTsMillis
33+
diffMilliseconds >= TimeUnit.DAYS.toMillis(SESSION_IS_MARKED_AS_INACTIVE_AFTER_DAYS.toLong())
34+
}
3335
}
3436
}

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/list/OtherSessionsController.kt

+2-1
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,13 @@ class OtherSessionsController @Inject constructor(
6363
}
6464
val drawableColor = host.colorProvider.getColorFromAttribute(R.attr.vctr_content_secondary)
6565
val descriptionDrawable = if (device.isInactive) host.drawableProvider.getDrawable(R.drawable.ic_inactive_sessions, drawableColor) else null
66+
val sessionName = device.deviceInfo.displayName ?: device.deviceInfo.deviceId
6667

6768
otherSessionItem {
6869
id(device.deviceInfo.deviceId)
6970
deviceType(device.deviceExtendedInfo.deviceType)
7071
roomEncryptionTrustLevel(device.roomEncryptionTrustLevel)
71-
sessionName(device.deviceInfo.displayName)
72+
sessionName(sessionName)
7273
sessionDescription(description)
7374
sessionDescriptionDrawable(descriptionDrawable)
7475
sessionDescriptionColor(descriptionColor)

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/list/SessionInfoView.kt

+4-3
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,10 @@ class SessionInfoView @JvmOverloads constructor(
6262
stringProvider: StringProvider,
6363
) {
6464
renderDeviceInfo(
65-
sessionInfoViewState.deviceFullInfo.deviceInfo.displayName.orEmpty(),
66-
sessionInfoViewState.deviceFullInfo.deviceExtendedInfo.deviceType,
67-
stringProvider,
65+
sessionName = sessionInfoViewState.deviceFullInfo.deviceInfo.displayName
66+
?: sessionInfoViewState.deviceFullInfo.deviceInfo.deviceId.orEmpty(),
67+
deviceType = sessionInfoViewState.deviceFullInfo.deviceExtendedInfo.deviceType,
68+
stringProvider = stringProvider,
6869
)
6970
renderVerificationStatus(
7071
sessionInfoViewState.deviceFullInfo.roomEncryptionTrustLevel,

‎vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/GetDeviceFullInfoUseCase.kt

+3-3
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,10 @@ class GetDeviceFullInfoUseCase @Inject constructor(
4949
) { currentSessionCrossSigningInfo, deviceInfo, cryptoDeviceInfo ->
5050
val info = deviceInfo.getOrNull()
5151
val cryptoInfo = cryptoDeviceInfo.getOrNull()
52-
val fullInfo = if (info != null && cryptoInfo != null) {
52+
val fullInfo = if (info != null) {
5353
val roomEncryptionTrustLevel = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoInfo)
54-
val isInactive = checkIfSessionIsInactiveUseCase.execute(info.lastSeenTs ?: 0)
55-
val isCurrentDevice = currentSessionCrossSigningInfo.deviceId == cryptoInfo.deviceId
54+
val isInactive = checkIfSessionIsInactiveUseCase.execute(info.lastSeenTs)
55+
val isCurrentDevice = currentSessionCrossSigningInfo.deviceId == info.deviceId
5656
val deviceUserAgent = parseDeviceUserAgentUseCase.execute(info.getBestLastSeenUserAgent())
5757
val matrixClientInfo = info.deviceId
5858
?.takeIf { it.isNotEmpty() }

‎vector/src/test/java/im/vector/app/features/settings/devices/v2/DevicesViewModelTest.kt

+12-11
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import com.airbnb.mvrx.Success
2121
import com.airbnb.mvrx.test.MavericksTestRule
2222
import im.vector.app.core.session.clientinfo.MatrixClientInfoContent
2323
import im.vector.app.features.settings.devices.v2.details.extended.DeviceExtendedInfo
24+
import im.vector.app.features.settings.devices.v2.filter.DeviceManagerFilterType
2425
import im.vector.app.features.settings.devices.v2.list.DeviceType
2526
import im.vector.app.features.settings.devices.v2.verification.CheckIfCurrentSessionCanBeVerifiedUseCase
2627
import im.vector.app.features.settings.devices.v2.verification.CurrentSessionCrossSigningInfo
@@ -176,10 +177,7 @@ class DevicesViewModelTest {
176177
val viewModelTest = createViewModel().test()
177178

178179
// Then
179-
viewModelTest.assertLatestState {
180-
it.devices is Success && it.devices.invoke() == deviceFullInfoList &&
181-
it.inactiveSessionsCount == 1 && it.unverifiedSessionsCount == 1
182-
}
180+
viewModelTest.assertLatestState { it.devices is Success && it.devices.invoke() == deviceFullInfoList }
183181
viewModelTest.finish()
184182
}
185183

@@ -403,7 +401,7 @@ class DevicesViewModelTest {
403401
/**
404402
* Generate mocked deviceFullInfo list with 1 unverified and inactive + 1 verified and active.
405403
*/
406-
private fun givenDeviceFullInfoList(deviceId1: String, deviceId2: String): List<DeviceFullInfo> {
404+
private fun givenDeviceFullInfoList(deviceId1: String, deviceId2: String): DeviceFullInfoList {
407405
val verifiedCryptoDeviceInfo = mockk<CryptoDeviceInfo>()
408406
every { verifiedCryptoDeviceInfo.trustLevel } returns DeviceTrustLevel(crossSigningVerified = true, locallyVerified = true)
409407
val unverifiedCryptoDeviceInfo = mockk<CryptoDeviceInfo>()
@@ -432,10 +430,15 @@ class DevicesViewModelTest {
432430
deviceExtendedInfo = DeviceExtendedInfo(DeviceType.MOBILE),
433431
matrixClientInfo = MatrixClientInfoContent(),
434432
)
435-
val deviceFullInfoList = listOf(deviceFullInfo1, deviceFullInfo2)
436-
val deviceFullInfoListFlow = flowOf(deviceFullInfoList)
437-
every { getDeviceFullInfoListUseCase.execute(any(), any()) } returns deviceFullInfoListFlow
438-
return deviceFullInfoList
433+
val devices = listOf(deviceFullInfo1, deviceFullInfo2)
434+
every { getDeviceFullInfoListUseCase.execute(DeviceManagerFilterType.ALL_SESSIONS, any()) } returns flowOf(devices)
435+
every { getDeviceFullInfoListUseCase.execute(DeviceManagerFilterType.UNVERIFIED, any()) } returns flowOf(listOf(deviceFullInfo2))
436+
every { getDeviceFullInfoListUseCase.execute(DeviceManagerFilterType.INACTIVE, any()) } returns flowOf(listOf(deviceFullInfo1))
437+
return DeviceFullInfoList(
438+
allSessions = devices,
439+
unverifiedSessionsCount = 1,
440+
inactiveSessionsCount = 1,
441+
)
439442
}
440443

441444
private fun givenInitialViewState(deviceId1: String, deviceId2: String): DevicesViewState {
@@ -444,8 +447,6 @@ class DevicesViewModelTest {
444447
return DevicesViewState(
445448
currentSessionCrossSigningInfo = currentSessionCrossSigningInfo,
446449
devices = Success(deviceFullInfoList),
447-
unverifiedSessionsCount = 1,
448-
inactiveSessionsCount = 1,
449450
isLoading = false,
450451
)
451452
}

‎vector/src/test/java/im/vector/app/features/settings/devices/v2/filter/FilterDevicesUseCaseTest.kt

+16-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import im.vector.app.features.settings.devices.v2.details.extended.DeviceExtende
2222
import im.vector.app.features.settings.devices.v2.list.DeviceType
2323
import im.vector.app.features.settings.devices.v2.verification.CurrentSessionCrossSigningInfo
2424
import org.amshove.kluent.shouldBeEqualTo
25+
import org.amshove.kluent.shouldContain
2526
import org.amshove.kluent.shouldContainAll
2627
import org.junit.Test
2728
import org.matrix.android.sdk.api.session.crypto.crosssigning.DeviceTrustLevel
@@ -82,11 +83,22 @@ private val inactiveUnverifiedDevice = DeviceFullInfo(
8283
matrixClientInfo = MatrixClientInfoContent(),
8384
)
8485

86+
private val deviceWithoutEncryptionSupport = DeviceFullInfo(
87+
deviceInfo = DeviceInfo(deviceId = "DEVICE_WITHOUT_ENCRYPTION_SUPPORT"),
88+
cryptoDeviceInfo = null,
89+
roomEncryptionTrustLevel = null,
90+
isInactive = false,
91+
isCurrentDevice = false,
92+
deviceExtendedInfo = DeviceExtendedInfo(DeviceType.UNKNOWN),
93+
matrixClientInfo = MatrixClientInfoContent(),
94+
)
95+
8596
private val devices = listOf(
8697
activeVerifiedDevice,
8798
inactiveVerifiedDevice,
8899
activeUnverifiedDevice,
89100
inactiveUnverifiedDevice,
101+
deviceWithoutEncryptionSupport,
90102
)
91103

92104
class FilterDevicesUseCaseTest {
@@ -123,16 +135,17 @@ class FilterDevicesUseCaseTest {
123135
val currentSessionCrossSigningInfo = givenCurrentSessionVerified(true)
124136
val filteredDeviceList = filterDevicesUseCase.execute(currentSessionCrossSigningInfo, devices, DeviceManagerFilterType.UNVERIFIED, emptyList())
125137

126-
filteredDeviceList.size shouldBeEqualTo 2
127-
filteredDeviceList shouldContainAll listOf(activeUnverifiedDevice, inactiveUnverifiedDevice)
138+
filteredDeviceList.size shouldBeEqualTo 3
139+
filteredDeviceList shouldContainAll listOf(activeUnverifiedDevice, inactiveUnverifiedDevice, deviceWithoutEncryptionSupport)
128140
}
129141

130142
@Test
131143
fun `given a device list and current session is unverified when filter type is UNVERIFIED then returns empty list of devices`() {
132144
val currentSessionCrossSigningInfo = givenCurrentSessionVerified(false)
133145
val filteredDeviceList = filterDevicesUseCase.execute(currentSessionCrossSigningInfo, devices, DeviceManagerFilterType.UNVERIFIED, emptyList())
134146

135-
filteredDeviceList.size shouldBeEqualTo 0
147+
filteredDeviceList.size shouldBeEqualTo 1
148+
filteredDeviceList shouldContain deviceWithoutEncryptionSupport
136149
}
137150

138151
@Test

0 commit comments

Comments
 (0)