Skip to content

Only offer to verify if a cross-signed device is available #5433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 6, 2025
Merged

Only offer to verify if a cross-signed device is available #5433

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e4579a5
Only offer to verify if a cross-signed device is available
uhoreg Sep 30, 2025
02b0bbd
Fix tests
uhoreg Sep 30, 2025
7b92999
use the right exception mapper
uhoreg Oct 1, 2025
c1c9a2e
adjust flag name and logic in ChooseSelfVerificationState
uhoreg Oct 1, 2025
529a3a8
add comment
uhoreg Oct 1, 2025
d43e3da
switch order of states to match previous logic
uhoreg Oct 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .../features/ftue/impl/sessionverification/choosemode/ChooseSelfVerificationModePresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ChooseSelfVerificationModePresenter(
) : Presenter<ChooseSelfVerificationModeState> {
@Composable
override fun present(): ChooseSelfVerificationModeState {
val isLastDevice by encryptionService.isLastDevice.collectAsState()
val hasDevicesToVerifyAgainst by encryptionService.hasDevicesToVerifyAgainst.collectAsState()
val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState()
val canEnterRecoveryKey by remember { derivedStateOf { recoveryState == RecoveryState.INCOMPLETE } }

Expand All @@ -39,7 +39,7 @@ class ChooseSelfVerificationModePresenter(
}

return ChooseSelfVerificationModeState(
isLastDevice = isLastDevice,
canUseAnotherDevice = hasDevicesToVerifyAgainst,
canEnterRecoveryKey = canEnterRecoveryKey,
directLogoutState = directLogoutState,
eventSink = ::eventHandler,
Expand Down
2 changes: 1 addition & 1 deletion ...roid/features/ftue/impl/sessionverification/choosemode/ChooseSelfVerificationModeState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ package io.element.android.features.ftue.impl.sessionverification.choosemode
import io.element.android.features.logout.api.direct.DirectLogoutState

data class ChooseSelfVerificationModeState(
val isLastDevice: Boolean,
val canUseAnotherDevice: Boolean,
val canEnterRecoveryKey: Boolean,
val directLogoutState: DirectLogoutState,
val eventSink: (ChooseSelfVerificationModeEvent) -> Unit,
Expand Down
12 changes: 6 additions & 6 deletions ...tures/ftue/impl/sessionverification/choosemode/ChooseSelfVerificationModeStateProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ import io.element.android.features.logout.api.direct.aDirectLogoutState
class ChooseSelfVerificationModeStateProvider :
PreviewParameterProvider<ChooseSelfVerificationModeState> {
override val values = sequenceOf(
aChooseSelfVerificationModeState(isLastDevice = true, canEnterRecoveryKey = true),
aChooseSelfVerificationModeState(isLastDevice = true, canEnterRecoveryKey = false),
aChooseSelfVerificationModeState(isLastDevice = false, canEnterRecoveryKey = true),
aChooseSelfVerificationModeState(isLastDevice = false, canEnterRecoveryKey = false),
aChooseSelfVerificationModeState(canUseAnotherDevice = true, canEnterRecoveryKey = true),
aChooseSelfVerificationModeState(canUseAnotherDevice = true, canEnterRecoveryKey = false),
aChooseSelfVerificationModeState(canUseAnotherDevice = false, canEnterRecoveryKey = true),
aChooseSelfVerificationModeState(canUseAnotherDevice = false, canEnterRecoveryKey = false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failing tests are related to the screenshot test, which are not run by default when running the test locally.
To avoid this problem, I think you need to use the opposite values for all boolean assigned to canUseAnotherDevice, and so the screenshot will be identical to the current version.

)
}

fun aChooseSelfVerificationModeState(
isLastDevice: Boolean = false,
canUseAnotherDevice: Boolean = true,
canEnterRecoveryKey: Boolean = true,
) = ChooseSelfVerificationModeState(
isLastDevice = isLastDevice,
canUseAnotherDevice = canUseAnotherDevice,
canEnterRecoveryKey = canEnterRecoveryKey,
directLogoutState = aDirectLogoutState(),
eventSink = {},
Expand Down
2 changes: 1 addition & 1 deletion ...droid/features/ftue/impl/sessionverification/choosemode/ChooseSelfVerificationModeView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fun ChooseSelfVerificationModeView(
ButtonColumnMolecule(
modifier = Modifier.padding(bottom = 16.dp)
) {
if (state.isLastDevice.not()) {
if (state.canUseAnotherDevice) {
Button(
modifier = Modifier.fillMaxWidth(),
text = stringResource(R.string.screen_identity_use_another_device),
Expand Down
6 changes: 3 additions & 3 deletions ...es/ftue/impl/sessionverification/choosemode/ChooseSessionVerificationModePresenterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ class ChooseSessionVerificationModePresenterTest {
@Test
fun `initial state - is relayed from EncryptionService`() = runTest {
val encryptionService = FakeEncryptionService().apply {
// Is last device
emitIsLastDevice(true)
// Has device to verify against
emitHasDevicesToVerifyAgainst(false)
// Can enter recovery key
emitRecoveryState(RecoveryState.INCOMPLETE)
}
val presenter = createPresenter(encryptionService = encryptionService)
presenter.test {
awaitItem().run {
assertThat(isLastDevice).isTrue()
assertThat(canUseAnotherDevice).isFalse()
assertThat(canEnterRecoveryKey).isTrue()
assertThat(directLogoutState.logoutAction.isUninitialized()).isTrue()
}
Expand Down
2 changes: 1 addition & 1 deletion ...eatures/ftue/impl/sessionverification/choosemode/ChooseSessionVerificationModeViewTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class ChooseSessionVerificationModeViewTest {
fun `clicking on use another device calls the callback`() {
ensureCalledOnce { callback ->
rule.setChooseSelfVerificationModeView(
aChooseSelfVerificationModeState(isLastDevice = false),
aChooseSelfVerificationModeState(canUseAnotherDevice = true),
onUseAnotherDevice = callback,
)
rule.clickOn(R.string.screen_identity_use_another_device)
Expand Down
1 change: 1 addition & 0 deletions ...i/src/main/kotlin/io/element/android/libraries/matrix/api/encryption/EncryptionService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface EncryptionService {
val recoveryStateStateFlow: StateFlow<RecoveryState>
val enableRecoveryProgressStateFlow: StateFlow<EnableRecoveryProgress>
val isLastDevice: StateFlow<Boolean>
val hasDevicesToVerifyAgainst: StateFlow<Boolean>

suspend fun enableBackups(): Result<Unit>

Expand Down
23 changes: 23 additions & 0 deletions .../main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import io.element.android.libraries.matrix.api.encryption.IdentityResetHandle
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.encryption.identity.IdentityState
import io.element.android.libraries.matrix.api.sync.SyncState
import io.element.android.libraries.matrix.impl.exception.mapClientException
import io.element.android.libraries.matrix.impl.sync.RustSyncService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.channels.awaitClose
Expand Down Expand Up @@ -96,6 +97,20 @@ internal class RustEncryptionService(
}
.stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false)

/**
* Check if the user has any devices available to verify against every 5 seconds.
* TODO This is a temporary workaround, when we will have a way to observe
* the sessions, this code will have to be updated.
*/
override val hasDevicesToVerifyAgainst: StateFlow<Boolean> = flow {
while (currentCoroutineContext().isActive) {
val result = hasDevicesToVerifyAgainst().getOrDefault(false)
emit(result)
delay(5_000)
}
}
.stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add the same TODO than in the KDoc of isLastDevice. It is a bit annoying to request this value every 5s, when it's only used in the session verification screen.


override suspend fun enableBackups(): Result<Unit> = withContext(dispatchers.io) {
runCatchingExceptions {
service.enableBackups()
Expand Down Expand Up @@ -171,6 +186,14 @@ internal class RustEncryptionService(
}
}

private suspend fun hasDevicesToVerifyAgainst(): Result<Boolean> = withContext(dispatchers.io) {
runCatchingExceptions {
service.hasDevicesToVerifyAgainst()
}.mapFailure {
it.mapClientException()
}
}

override suspend fun resetRecoveryKey(): Result<String> = withContext(dispatchers.io) {
runCatchingExceptions {
service.resetRecoveryKey()
Expand Down
4 changes: 4 additions & 0 deletions .../test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiEncryption.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class FakeFfiEncryption : Encryption(NoPointer) {
return false
}

override suspend fun hasDevicesToVerifyAgainst(): Boolean {
return true
}

override fun backupState(): BackupState {
return BackupState.ENABLED
}
Expand Down
5 changes: 5 additions & 0 deletions .../main/kotlin/io/element/android/libraries/matrix/test/encryption/FakeEncryptionService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class FakeEncryptionService(
override val recoveryStateStateFlow: MutableStateFlow<RecoveryState> = MutableStateFlow(RecoveryState.UNKNOWN)
override val enableRecoveryProgressStateFlow: MutableStateFlow<EnableRecoveryProgress> = MutableStateFlow(EnableRecoveryProgress.Starting)
override val isLastDevice: MutableStateFlow<Boolean> = MutableStateFlow(false)
override val hasDevicesToVerifyAgainst: MutableStateFlow<Boolean> = MutableStateFlow(true)
private var waitForBackupUploadSteadyStateFlow: Flow<BackupUploadState> = flowOf()

private var recoverFailure: Exception? = null
Expand Down Expand Up @@ -83,6 +84,10 @@ class FakeEncryptionService(
this.isLastDevice.value = isLastDevice
}

fun emitHasDevicesToVerifyAgainst(hasDevicesToVerifyAgainst: Boolean) {
this.hasDevicesToVerifyAgainst.value = hasDevicesToVerifyAgainst
}

override suspend fun resetRecoveryKey(): Result<String> = simulateLongTask {
return Result.success(FAKE_RECOVERY_KEY)
}
Expand Down
Loading