-
Notifications
You must be signed in to change notification settings - Fork 186
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
Changes from all commits
73454b6
3480d44
87f32b4
e2f0302
13d8f66
88befce
320323f
53bf1a7
34fcd79
a653a54
f95748d
a15ece9
a973dd9
2b6a3e8
3f87591
18b8a36
54babec
995a122
e1b03bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
|
@@ -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 { | ||
// Wait until the app is in foreground to display the incoming verification request | ||
appNavigationStateService.appNavigationState.first { it.isInForeground } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add a timeout? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} | ||
} | ||
} | ||
|
||
|
@@ -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 { | ||
|
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 |
---|---|---|
@@ -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) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can
RustSessionVerificationService
ensure thatonIncomingSessionRequest
is invoked on the same thread that was used when registering the listener?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I had in mind, but OK to keep it like this :)