Skip to content
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.

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

Implement user verification #4294

Merged
merged 19 commits into from
Mar 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
73454b6
Add support for starting verification of a user
jmartinesp Feb 19, 2025
3480d44
Add support for replying to incoming user verification requests
jmartinesp Feb 21, 2025
87f32b4
Add reset recovery key button and previews to `ChooseSelfVerification…
jmartinesp Feb 21, 2025
e2f0302
Add and fix tests, fix lint issues
jmartinesp Feb 21, 2025
13d8f66
Add 'Profile' item in room details screen
jmartinesp Feb 21, 2025
88befce
Delay displaying the incoming user verification until the UI is ready
jmartinesp Feb 21, 2025
320323f
Fix issues after rebase
jmartinesp Mar 6, 2025
53bf1a7
Update screenshots
ElementBot Mar 6, 2025
34fcd79
Use right copy for 'verify user' action
jmartinesp Mar 6, 2025
a653a54
Add padding between text items in the incoming user verification requ…
jmartinesp Mar 6, 2025
f95748d
Fix icons in the outgoing user verification
jmartinesp Mar 6, 2025
a15ece9
Fix icons in the incoming user verification screens
jmartinesp Mar 6, 2025
a973dd9
Remove `showDeviceVerifiedScreen` parameter from `NavTarget.UseAnothe…
jmartinesp Mar 6, 2025
2b6a3e8
Fix minor naming/visibility issues
jmartinesp Mar 6, 2025
3f87591
Split event and state for `ChooseSelfVerificationModePresenter` into …
jmartinesp Mar 6, 2025
18b8a36
Allow exiting the FTUE flow, which will close the app. The previous s…
jmartinesp Mar 6, 2025
54babec
When outgoing verification fails, move to the `Canceled` state.
jmartinesp Mar 7, 2025
995a122
Remove some dead code
jmartinesp Mar 7, 2025
e1b03bc
Update screenshots
ElementBot Mar 7, 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
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,21 @@ import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.core.toRoomIdOrAlias
import io.element.android.libraries.matrix.api.permalink.PermalinkData
import io.element.android.libraries.matrix.api.verification.SessionVerificationRequestDetails
import io.element.android.libraries.matrix.api.verification.SessionVerificationServiceListener
import io.element.android.libraries.matrix.api.verification.VerificationRequest
import io.element.android.services.appnavstate.api.AppNavigationStateService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.MainScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.parcelize.Parcelize
import timber.log.Timber
import java.util.Optional
import java.util.UUID
import kotlin.time.Duration.Companion.milliseconds

@ContributesNode(SessionScope::class)
class LoggedInFlowNode @AssistedInject constructor(
Expand Down Expand Up @@ -127,8 +131,18 @@ class LoggedInFlowNode @AssistedInject constructor(
)

private val verificationListener = object : SessionVerificationServiceListener {
override fun onIncomingSessionRequest(sessionVerificationRequestDetails: SessionVerificationRequestDetails) {
backstack.singleTop(NavTarget.IncomingVerificationRequest(sessionVerificationRequestDetails))
override fun onIncomingSessionRequest(verificationRequest: VerificationRequest.Incoming) {
// Without this launch the rendering and actual state of this Appyx node's children gets out of sync, resulting in a crash.
// This might be because this method is called back from Rust in a background thread.
MainScope().launch {
Copy link
Member

Choose a reason for hiding this comment

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

Can RustSessionVerificationService ensure that onIncomingSessionRequest is invoked on the same thread that was used when registering the listener?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, this is due to how JNA works, it'll create a separate thread for this callback and use it when receiving data from Rust. Maybe we could somehow retain the thread instance and force the callback to somehow run some runnable in it, but it seems like an overkill.

Copy link
Member

Choose a reason for hiding this comment

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

retain the thread instance and force the callback to somehow run some runnable in it

This is what I had in mind, but OK to keep it like this :)

// Wait until the app is in foreground to display the incoming verification request
appNavigationStateService.appNavigationState.first { it.isInForeground }
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 a timeout?

Copy link
Member Author

@jmartinesp jmartinesp Mar 6, 2025

Choose a reason for hiding this comment

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

Maybe, but I'm not sure what a sensible value would be. Maybe instead of the timeout we should try to check if the request hasn't been cancelled.


// Wait for the UI to be ready
delay(500.milliseconds)

backstack.singleTop(NavTarget.IncomingVerificationRequest(verificationRequest))
Copy link
Member

Choose a reason for hiding this comment

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

We should probably ignore new incoming request if there is already a verification in progress. But maybe this is handled by the SDK?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I can't remember the behaviour, but it was either new requests cancelled previous ones or were ignored, according to the docs.

}
}
}

Expand Down Expand Up @@ -218,7 +232,7 @@ class LoggedInFlowNode @AssistedInject constructor(
data object LogoutForNativeSlidingSyncMigrationNeeded : NavTarget

@Parcelize
data class IncomingVerificationRequest(val data: SessionVerificationRequestDetails) : NavTarget
data class IncomingVerificationRequest(val data: VerificationRequest.Incoming) : NavTarget
}

override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
Expand Down
12 changes: 12 additions & 0 deletions features/ftue/impl/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import extension.setupAnvil
import org.gradle.kotlin.dsl.test

/*
* Copyright 2023, 2024 New Vector Ltd.
Expand All @@ -14,6 +15,12 @@ plugins {

android {
namespace = "io.element.android.features.ftue.impl"

testOptions {
unitTests {
isIncludeAndroidResources = true
}
}
}

setupAnvil()
Expand All @@ -30,19 +37,24 @@ dependencies {
implementation(projects.libraries.uiStrings)
implementation(projects.libraries.testtags)
implementation(projects.features.analytics.api)
implementation(projects.features.logout.api)
implementation(projects.features.securebackup.api)
implementation(projects.features.verifysession.api)
implementation(projects.services.analytics.api)
implementation(projects.features.lockscreen.api)
implementation(projects.libraries.permissions.api)
implementation(projects.libraries.permissions.noop)
implementation(projects.services.toolbox.api)
implementation(projects.appconfig)

testImplementation(libs.test.junit)
testImplementation(libs.coroutines.test)
testImplementation(libs.molecule.runtime)
testImplementation(libs.test.truth)
testImplementation(libs.test.turbine)
testImplementation(libs.test.robolectric)
testImplementation(libs.androidx.compose.ui.test.junit)
testReleaseImplementation(libs.androidx.compose.ui.test.manifest)
testImplementation(projects.libraries.matrix.test)
testImplementation(projects.services.analytics.test)
testImplementation(projects.services.analytics.noop)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import androidx.compose.ui.Modifier
import androidx.lifecycle.lifecycleScope
import com.bumble.appyx.core.lifecycle.subscribe
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.navigation.backpresshandlerstrategies.BaseBackPressHandlerStrategy
import com.bumble.appyx.core.node.Node
import com.bumble.appyx.core.plugin.Plugin
import com.bumble.appyx.navmodel.backstack.BackStack
Expand All @@ -38,8 +37,6 @@ import io.element.android.libraries.designsystem.theme.components.CircularProgre
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.SessionScope
import io.element.android.services.analytics.api.AnalyticsService
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.launchIn
Expand All @@ -59,7 +56,6 @@ class FtueFlowNode @AssistedInject constructor(
backstack = BackStack(
initialElement = NavTarget.Placeholder,
savedStateMap = buildContext.savedStateMap,
backPressHandler = NoOpBackstackHandlerStrategy(),
),
buildContext = buildContext,
plugins = plugins,
Expand Down Expand Up @@ -104,7 +100,7 @@ class FtueFlowNode @AssistedInject constructor(
NavTarget.Placeholder -> {
createNode<PlaceholderNode>(buildContext)
}
NavTarget.SessionVerification -> {
is NavTarget.SessionVerification -> {
val callback = object : FtueSessionVerificationFlowNode.Callback {
override fun onDone() {
moveToNextStepIfNeeded()
Expand Down Expand Up @@ -175,11 +171,3 @@ class FtueFlowNode @AssistedInject constructor(
}
}
}

private class NoOpBackstackHandlerStrategy<NavTarget : Any> : BaseBackPressHandlerStrategy<NavTarget, BackStack.State>() {
override val canHandleBackPressFlow: StateFlow<Boolean> = MutableStateFlow(true)

override fun onBackPressed() {
// No-op
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/

package io.element.android.features.ftue.impl.di

import com.squareup.anvil.annotations.ContributesTo
import dagger.Binds
import dagger.Module
import io.element.android.features.ftue.impl.sessionverification.choosemode.ChooseSelfVerificationModePresenter
import io.element.android.features.ftue.impl.sessionverification.choosemode.ChooseSelfVerificationModeState
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.di.SessionScope

@ContributesTo(SessionScope::class)
@Module
interface FtueModule {
@Binds
fun bindChooseSelfVerificationMethodPresenter(presenter: ChooseSelfVerificationModePresenter): Presenter<ChooseSelfVerificationModeState>
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package io.element.android.features.ftue.impl.sessionverification

import android.os.Parcelable
import androidx.compose.runtime.Composable
import androidx.compose.runtime.mutableStateOf
import androidx.compose.ui.Modifier
import androidx.lifecycle.lifecycleScope
import com.bumble.appyx.core.modality.BuildContext
Expand All @@ -17,15 +18,21 @@ import com.bumble.appyx.core.plugin.Plugin
import com.bumble.appyx.core.plugin.plugins
import com.bumble.appyx.navmodel.backstack.BackStack
import com.bumble.appyx.navmodel.backstack.operation.newRoot
import com.bumble.appyx.navmodel.backstack.operation.pop
import com.bumble.appyx.navmodel.backstack.operation.push
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import io.element.android.anvilannotations.ContributesNode
import io.element.android.appconfig.LearnMoreConfig
import io.element.android.features.ftue.impl.sessionverification.choosemode.ChooseSelfVerificationModeNode
import io.element.android.features.securebackup.api.SecureBackupEntryPoint
import io.element.android.features.verifysession.api.VerifySessionEntryPoint
import io.element.android.libraries.architecture.BackstackView
import io.element.android.libraries.architecture.BaseFlowNode
import io.element.android.libraries.architecture.createNode
import io.element.android.libraries.designsystem.utils.OpenUrlInTabView
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.matrix.api.verification.VerificationRequest
import kotlinx.coroutines.launch
import kotlinx.parcelize.Parcelize

Expand All @@ -37,15 +44,18 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
private val secureBackupEntryPoint: SecureBackupEntryPoint,
) : BaseFlowNode<FtueSessionVerificationFlowNode.NavTarget>(
backstack = BackStack(
initialElement = NavTarget.Root(showDeviceVerifiedScreen = false),
initialElement = NavTarget.Root,
savedStateMap = buildContext.savedStateMap,
),
buildContext = buildContext,
plugins = plugins,
) {
sealed interface NavTarget : Parcelable {
@Parcelize
data class Root(val showDeviceVerifiedScreen: Boolean) : NavTarget
data object Root : NavTarget

@Parcelize
data object UseAnotherDevice : NavTarget

@Parcelize
data object EnterRecoveryKey : NavTarget
Expand All @@ -62,27 +72,51 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
override fun onDone() {
lifecycleScope.launch {
// Move to the completed state view in the verification flow
backstack.newRoot(NavTarget.Root(showDeviceVerifiedScreen = true))
backstack.newRoot(NavTarget.UseAnotherDevice)
}
}
}

override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
return when (navTarget) {
is NavTarget.Root -> {
val callback = object : ChooseSelfVerificationModeNode.Callback {
override fun onUseAnotherDevice() {
backstack.push(NavTarget.UseAnotherDevice)
}

override fun onUseRecoveryKey() {
backstack.push(NavTarget.EnterRecoveryKey)
}

override fun onResetKey() {
backstack.push(NavTarget.ResetIdentity)
}

override fun onLearnMoreAboutEncryption() {
learnMoreUrl.value = LearnMoreConfig.ENCRYPTION_URL
}
}

createNode<ChooseSelfVerificationModeNode>(buildContext, plugins = listOf(callback))
}
is NavTarget.UseAnotherDevice -> {
verifySessionEntryPoint.nodeBuilder(this, buildContext)
.params(VerifySessionEntryPoint.Params(navTarget.showDeviceVerifiedScreen))
.params(VerifySessionEntryPoint.Params(
showDeviceVerifiedScreen = true,
verificationRequest = VerificationRequest.Outgoing.CurrentSession,
))
.callback(object : VerifySessionEntryPoint.Callback {
override fun onEnterRecoveryKey() {
backstack.push(NavTarget.EnterRecoveryKey)
}

override fun onDone() {
plugins<Callback>().forEach { it.onDone() }
}

override fun onResetKey() {
backstack.push(NavTarget.ResetIdentity)
override fun onBack() {
backstack.pop()
}

override fun onLearnMoreAboutEncryption() {
learnMoreUrl.value = LearnMoreConfig.ENCRYPTION_URL
}
})
.build()
Expand All @@ -106,8 +140,12 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
}
}

private val learnMoreUrl = mutableStateOf<String?>(null)

@Composable
override fun View(modifier: Modifier) {
BackstackView()

OpenUrlInTabView(learnMoreUrl)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/

package io.element.android.features.ftue.impl.sessionverification.choosemode

sealed interface ChooseSelfVerificationModeEvent {
data object SignOut : ChooseSelfVerificationModeEvent
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/

package io.element.android.features.ftue.impl.sessionverification.choosemode

import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
import com.bumble.appyx.core.plugin.Plugin
import com.bumble.appyx.core.plugin.plugins
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import io.element.android.anvilannotations.ContributesNode
import io.element.android.features.logout.api.direct.DirectLogoutView
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.di.SessionScope

@ContributesNode(SessionScope::class)
class ChooseSelfVerificationModeNode @AssistedInject constructor(
@Assisted buildContext: BuildContext,
@Assisted plugins: List<Plugin>,
private val presenter: Presenter<ChooseSelfVerificationModeState>,
private val directLogoutView: DirectLogoutView,
) : Node(buildContext, plugins = plugins) {
interface Callback : Plugin {
fun onUseAnotherDevice()
fun onUseRecoveryKey()
fun onResetKey()
fun onLearnMoreAboutEncryption()
}

private val callback = plugins<Callback>().first()

@Composable
override fun View(modifier: Modifier) {
val state = presenter.present()

ChooseSelfVerificationModeView(
state = state,
onUseAnotherDevice = callback::onUseAnotherDevice,
onUseRecoveryKey = callback::onUseRecoveryKey,
onResetKey = callback::onResetKey,
onLearnMore = callback::onLearnMoreAboutEncryption,
modifier = modifier,
)

directLogoutView.Render(state = state.directLogoutState)
}
}
Loading
Loading