Skip to content

Commit be7b49b

Browse files
mnaturelmnaturel
authored
Merge pull request #7736 from vector-im/fix/mna/session-without-crypto-keys
[Session manager] Sessions without encryption support should not prompt to verify (PSG-1004)
2 parents cf59c80 + 23c2682 commit be7b49b

File tree

10 files changed

+77
-33
lines changed

10 files changed

+77
-33
lines changed

‎changelog.d/7733.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[Session manager] Sessions without encryption support should not prompt to verify

‎library/ui-strings/src/main/res/values/strings.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3308,6 +3308,7 @@
33083308
<string name="device_manager_verification_status_detail_current_session_unverified">Verify your current session for enhanced secure messaging.</string>
33093309
<string name="device_manager_verification_status_detail_other_session_unverified">Verify or sign out from this session for best security and reliability.</string>
33103310
<string name="device_manager_verification_status_detail_other_session_unknown">Verify your current session to reveal this session\'s verification status.</string>
3311+
<string name="device_manager_verification_status_detail_session_encryption_not_supported">This session doesn\'t support encryption and thus can\'t be verified.</string>
33113312
<string name="device_manager_verify_session">Verify Session</string>
33123313
<string name="device_manager_view_details">View Details</string>
33133314
<string name="device_manager_other_sessions_view_all">View All (%1$d)</string>
@@ -3400,6 +3401,7 @@
34003401
<!-- TODO TO BE REMOVED -->
34013402
<string name="device_manager_learn_more_sessions_verified" tools:ignore="UnusedResources">Verified sessions have logged in with your credentials and then been verified, either using your secure passphrase or by cross-verifying.\n\nThis means they hold encryption keys for your previous messages, and confirm to other users you are communicating with that these sessions are really you.</string>
34023403
<string name="device_manager_learn_more_sessions_verified_description">Verified sessions are anywhere you are using this account after entering your passphrase or confirming your identity with another verified session.\n\nThis means that you have all the keys needed to unlock your encrypted messages and confirm to other users that you trust this session.</string>
3404+
<string name="device_manager_learn_more_sessions_encryption_not_supported">This session doesn\'t support encryption, so it can\'t be verified.\n\nYou won\'t be able to participate in rooms where encryption is enabled when using this session.\n\nFor best security and privacy, it is recommended to use Matrix clients that support encryption.</string>
34033405
<string name="device_manager_learn_more_session_rename_title">Renaming sessions</string>
34043406
<string name="device_manager_learn_more_session_rename">Other users in direct messages and rooms that you join are able to view a full list of your sessions.\n\nThis provides them with confidence that they are really speaking to you, but it also means they can see the session name you enter here.</string>
34053407
<string name="labs_enable_session_manager_title">Enable new session manager</string>

‎vector/src/main/java/im/vector/app/core/ui/views/ShieldImageView.kt

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,26 @@ class ShieldImageView @JvmOverloads constructor(
4040

4141
/**
4242
* Renders device shield with the support of unknown shields instead of black shields which is used for rooms.
43-
* @param roomEncryptionTrustLevel trust level that is usally calculated with [im.vector.app.features.settings.devices.TrustUtils.shieldForTrust]
43+
* @param roomEncryptionTrustLevel trust level that is usually calculated with [im.vector.app.features.settings.devices.TrustUtils.shieldForTrust]
4444
* @param borderLess if true then the shield icon with border around is used
4545
*/
4646
fun renderDeviceShield(roomEncryptionTrustLevel: RoomEncryptionTrustLevel?, borderLess: Boolean = false) {
47-
isVisible = roomEncryptionTrustLevel != null
48-
49-
if (roomEncryptionTrustLevel == RoomEncryptionTrustLevel.Default) {
50-
contentDescription = context.getString(R.string.a11y_trust_level_default)
51-
setImageResource(
52-
if (borderLess) R.drawable.ic_shield_unknown_no_border
53-
else R.drawable.ic_shield_unknown
54-
)
55-
} else {
56-
render(roomEncryptionTrustLevel, borderLess)
47+
when (roomEncryptionTrustLevel) {
48+
null -> {
49+
contentDescription = context.getString(R.string.a11y_trust_level_warning)
50+
setImageResource(
51+
if (borderLess) R.drawable.ic_shield_warning_no_border
52+
else R.drawable.ic_shield_warning
53+
)
54+
}
55+
RoomEncryptionTrustLevel.Default -> {
56+
contentDescription = context.getString(R.string.a11y_trust_level_default)
57+
setImageResource(
58+
if (borderLess) R.drawable.ic_shield_unknown_no_border
59+
else R.drawable.ic_shield_unknown
60+
)
61+
}
62+
else -> render(roomEncryptionTrustLevel, borderLess)
5763
}
5864
}
5965

‎vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(
104104
// Timber.v("## Detector trigger canCrossSign ${pInfo.get().selfSigned != null}")
105105
infoList
106106
.filter { info ->
107-
// filter verified session, by checking the crypto device info
107+
// filter out verified sessions or those which do not support encryption (i.e. without crypto info)
108108
cryptoList.firstOrNull { info.deviceId == it.deviceId }?.isVerified?.not().orFalse()
109109
}
110110
// filter out ignored devices

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ data class DevicesViewState(
8888
data class DeviceFullInfo(
8989
val deviceInfo: DeviceInfo,
9090
val cryptoDeviceInfo: CryptoDeviceInfo?,
91-
val trustLevelForShield: RoomEncryptionTrustLevel,
91+
val trustLevelForShield: RoomEncryptionTrustLevel?,
9292
val isInactive: Boolean,
9393
)
9494

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
2525
data class DeviceFullInfo(
2626
val deviceInfo: DeviceInfo,
2727
val cryptoDeviceInfo: CryptoDeviceInfo?,
28-
val roomEncryptionTrustLevel: RoomEncryptionTrustLevel,
28+
val roomEncryptionTrustLevel: RoomEncryptionTrustLevel?,
2929
val isInactive: Boolean,
3030
val isCurrentDevice: Boolean,
3131
val deviceExtendedInfo: DeviceExtendedInfo,

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,14 @@ class SessionInfoView @JvmOverloads constructor(
8585
}
8686

8787
private fun renderVerificationStatus(
88-
encryptionTrustLevel: RoomEncryptionTrustLevel,
88+
encryptionTrustLevel: RoomEncryptionTrustLevel?,
8989
isCurrentSession: Boolean,
9090
hasLearnMoreLink: Boolean,
9191
isVerifyButtonVisible: Boolean,
9292
) {
9393
views.sessionInfoVerificationStatusImageView.renderDeviceShield(encryptionTrustLevel)
9494
when {
95+
encryptionTrustLevel == null -> renderCrossSigningEncryptionNotSupported()
9596
encryptionTrustLevel == RoomEncryptionTrustLevel.Trusted -> renderCrossSigningVerified(isCurrentSession)
9697
encryptionTrustLevel == RoomEncryptionTrustLevel.Default && !isCurrentSession -> renderCrossSigningUnknown()
9798
else -> renderCrossSigningUnverified(isCurrentSession, isVerifyButtonVisible)
@@ -149,6 +150,14 @@ class SessionInfoView @JvmOverloads constructor(
149150
views.sessionInfoVerifySessionButton.isVisible = false
150151
}
151152

153+
private fun renderCrossSigningEncryptionNotSupported() {
154+
views.sessionInfoVerificationStatusTextView.text = context.getString(R.string.device_manager_verification_status_unverified)
155+
views.sessionInfoVerificationStatusTextView.setTextColor(ThemeUtils.getColor(context, R.attr.colorError))
156+
views.sessionInfoVerificationStatusDetailTextView.text =
157+
context.getString(R.string.device_manager_verification_status_detail_session_encryption_not_supported)
158+
views.sessionInfoVerifySessionButton.isVisible = false
159+
}
160+
152161
private fun renderDeviceInfo(sessionName: String, deviceType: DeviceType, stringProvider: StringProvider) {
153162
setDeviceTypeIconUseCase.execute(deviceType, views.sessionInfoDeviceTypeImageView, stringProvider)
154163
views.sessionInfoNameTextView.text = sessionName

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

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ class SessionOverviewFragment :
229229
)
230230
views.sessionOverviewInfo.render(infoViewState, dateFormatter, drawableProvider, colorProvider, stringProvider)
231231
views.sessionOverviewInfo.onLearnMoreClickListener = {
232-
showLearnMoreInfoVerificationStatus(deviceInfo.roomEncryptionTrustLevel == RoomEncryptionTrustLevel.Trusted)
232+
showLearnMoreInfoVerificationStatus(deviceInfo.roomEncryptionTrustLevel)
233233
}
234234
} else {
235235
views.sessionOverviewInfo.isVisible = false
@@ -293,21 +293,28 @@ class SessionOverviewFragment :
293293
}
294294
}
295295

296-
private fun showLearnMoreInfoVerificationStatus(isVerified: Boolean) {
297-
val titleResId = if (isVerified) {
298-
R.string.device_manager_verification_status_verified
299-
} else {
300-
R.string.device_manager_verification_status_unverified
301-
}
302-
val descriptionResId = if (isVerified) {
303-
R.string.device_manager_learn_more_sessions_verified_description
304-
} else {
305-
R.string.device_manager_learn_more_sessions_unverified
296+
private fun showLearnMoreInfoVerificationStatus(roomEncryptionTrustLevel: RoomEncryptionTrustLevel?) {
297+
val args = when (roomEncryptionTrustLevel) {
298+
null -> {
299+
// encryption not supported
300+
SessionLearnMoreBottomSheet.Args(
301+
title = getString(R.string.device_manager_verification_status_unverified),
302+
description = getString(R.string.device_manager_learn_more_sessions_encryption_not_supported),
303+
)
304+
}
305+
RoomEncryptionTrustLevel.Trusted -> {
306+
SessionLearnMoreBottomSheet.Args(
307+
title = getString(R.string.device_manager_verification_status_verified),
308+
description = getString(R.string.device_manager_learn_more_sessions_verified_description),
309+
)
310+
}
311+
else -> {
312+
SessionLearnMoreBottomSheet.Args(
313+
title = getString(R.string.device_manager_verification_status_unverified),
314+
description = getString(R.string.device_manager_learn_more_sessions_unverified),
315+
)
316+
}
306317
}
307-
val args = SessionLearnMoreBottomSheet.Args(
308-
title = getString(titleResId),
309-
description = getString(descriptionResId),
310-
)
311318
SessionLearnMoreBottomSheet.show(childFragmentManager, args)
312319
}
313320
}

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,15 @@ class GetEncryptionTrustLevelForDeviceUseCase @Inject constructor(
2525
private val getEncryptionTrustLevelForOtherDeviceUseCase: GetEncryptionTrustLevelForOtherDeviceUseCase,
2626
) {
2727

28-
fun execute(currentSessionCrossSigningInfo: CurrentSessionCrossSigningInfo, cryptoDeviceInfo: CryptoDeviceInfo?): RoomEncryptionTrustLevel {
28+
fun execute(currentSessionCrossSigningInfo: CurrentSessionCrossSigningInfo, cryptoDeviceInfo: CryptoDeviceInfo?): RoomEncryptionTrustLevel? {
29+
if (cryptoDeviceInfo == null) {
30+
return null
31+
}
32+
2933
val legacyMode = !currentSessionCrossSigningInfo.isCrossSigningInitialized
3034
val trustMSK = currentSessionCrossSigningInfo.isCrossSigningVerified
31-
val isCurrentDevice = !cryptoDeviceInfo?.deviceId.isNullOrEmpty() && cryptoDeviceInfo?.deviceId == currentSessionCrossSigningInfo.deviceId
32-
val deviceTrustLevel = cryptoDeviceInfo?.trustLevel
35+
val isCurrentDevice = !cryptoDeviceInfo.deviceId.isNullOrEmpty() && cryptoDeviceInfo.deviceId == currentSessionCrossSigningInfo.deviceId
36+
val deviceTrustLevel = cryptoDeviceInfo.trustLevel
3337

3438
return when {
3539
isCurrentDevice -> getEncryptionTrustLevelForCurrentDeviceUseCase.execute(trustMSK, legacyMode)

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package im.vector.app.features.settings.devices.v2.verification
1919
import io.mockk.every
2020
import io.mockk.mockk
2121
import io.mockk.verify
22+
import org.amshove.kluent.shouldBe
2223
import org.amshove.kluent.shouldBeEqualTo
2324
import org.junit.Test
2425
import org.matrix.android.sdk.api.session.crypto.crosssigning.DeviceTrustLevel
@@ -89,6 +90,20 @@ class GetEncryptionTrustLevelForDeviceUseCaseTest {
8990
}
9091
}
9192

93+
@Test
94+
fun `given no crypto device info when computing trust level then result is null`() {
95+
val currentSessionCrossSigningInfo = givenCurrentSessionCrossSigningInfo(
96+
deviceId = A_DEVICE_ID,
97+
isCrossSigningInitialized = true,
98+
isCrossSigningVerified = false
99+
)
100+
val cryptoDeviceInfo = null
101+
102+
val result = getEncryptionTrustLevelForDeviceUseCase.execute(currentSessionCrossSigningInfo, cryptoDeviceInfo)
103+
104+
result shouldBe null
105+
}
106+
92107
private fun givenCurrentSessionCrossSigningInfo(
93108
deviceId: String,
94109
isCrossSigningInitialized: Boolean,

0 commit comments

Comments
 (0)