Add SessionData.needsVerification field (#2672)

* Add `SessionData.needsVerification`:
  - Allows us to add a skip button for debug builds.
  - We can have the verification state almost instantly.
  - It doesn't depend on network availability to know the verification state and display the UI.
* Add DB migration.
- Make the skip button in the verification flow skip the whole flow including the completed screen.
- Save the session as verified in `RustEncryptionService.recover(recoveryKey)`.
* Enforce session verification for existing users too.
* Fix verification confirmed screen subtitle (typo in id, was using the wrong string)
* Update screenshots

---------

Co-authored-by: ElementBot <benoitm+elementbot@element.io>
This commit is contained in:
Jorge Martin Espinosa
2024-04-09 17:28:12 +02:00
committed by GitHub
parent 8757d1b5ad
commit c8b5458878
44 changed files with 386 additions and 123 deletions

View File

@@ -7,3 +7,7 @@ appId: ${MAESTRO_APP_ID}
- inputText: ${MAESTRO_RECOVERY_KEY}
- hideKeyboard
- tapOn: "Confirm"
- extendedWaitUntil:
visible: "Device verified"
timeout: 10000
- tapOn: "Continue"

View File

@@ -19,11 +19,13 @@ package io.element.android.features.ftue.impl.sessionverification
import android.os.Parcelable
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.lifecycle.lifecycleScope
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 com.bumble.appyx.navmodel.backstack.BackStack
import com.bumble.appyx.navmodel.backstack.operation.newRoot
import com.bumble.appyx.navmodel.backstack.operation.push
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
@@ -33,6 +35,7 @@ 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.di.SessionScope
import kotlinx.coroutines.launch
import kotlinx.parcelize.Parcelize
@ContributesNode(SessionScope::class)
@@ -83,7 +86,10 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
.params(SecureBackupEntryPoint.Params(SecureBackupEntryPoint.InitialTarget.EnterRecoveryKey))
.callback(object : SecureBackupEntryPoint.Callback {
override fun onDone() {
callback.onDone()
lifecycleScope.launch {
// Move to the completed state view in the verification flow
backstack.newRoot(NavTarget.Root)
}
}
})
.build()

View File

@@ -25,7 +25,6 @@ import io.element.android.features.ftue.api.state.FtueState
import io.element.android.features.lockscreen.api.LockScreenService
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
import io.element.android.libraries.permissions.api.PermissionStateProvider
import io.element.android.services.analytics.api.AnalyticsService
import io.element.android.services.toolbox.api.sdk.BuildVersionSdkIntProvider
@@ -56,7 +55,7 @@ class DefaultFtueService @Inject constructor(
}
init {
sessionVerificationService.sessionVerifiedStatus
sessionVerificationService.needsVerificationFlow
.onEach { updateState() }
.launchIn(coroutineScope)
@@ -99,12 +98,8 @@ class DefaultFtueService @Inject constructor(
).any { it() }
}
private fun isSessionVerificationServiceReady(): Boolean {
return sessionVerificationService.sessionVerifiedStatus.value != SessionVerifiedStatus.Unknown
}
private fun isSessionNotVerified(): Boolean {
return sessionVerificationService.sessionVerifiedStatus.value == SessionVerifiedStatus.NotVerified
return sessionVerificationService.needsVerificationFlow.value
}
private fun needsAnalyticsOptIn(): Boolean {
@@ -132,7 +127,6 @@ class DefaultFtueService @Inject constructor(
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun updateState() {
state.value = when {
!isSessionVerificationServiceReady() -> FtueState.Unknown
isAnyStepIncomplete() -> FtueState.Incomplete
else -> FtueState.Complete
}

View File

@@ -90,6 +90,7 @@ class DefaultFtueServiceTests {
fun `traverse flow`() = runTest {
val sessionVerificationService = FakeSessionVerificationService().apply {
givenVerifiedStatus(SessionVerifiedStatus.NotVerified)
givenNeedsVerification(true)
}
val analyticsService = FakeAnalyticsService()
val permissionStateProvider = FakePermissionStateProvider(permissionGranted = false)
@@ -107,7 +108,7 @@ class DefaultFtueServiceTests {
// Session verification
steps.add(state.getNextStep(steps.lastOrNull()))
sessionVerificationService.givenVerifiedStatus(SessionVerifiedStatus.Verified)
sessionVerificationService.givenNeedsVerification(false)
// Notifications opt in
steps.add(state.getNextStep(steps.lastOrNull()))

View File

@@ -38,6 +38,7 @@ fun aSignedOutState() = SignedOutState(
fun aSessionData(
sessionId: SessionId = SessionId("@alice:server.org"),
isTokenValid: Boolean = false,
needsVerification: Boolean = false,
): SessionData {
return SessionData(
userId = sessionId.value,
@@ -51,5 +52,6 @@ fun aSessionData(
isTokenValid = isTokenValid,
loginType = LoginType.UNKNOWN,
passphrase = null,
needsVerification = needsVerification,
)
}

View File

@@ -23,10 +23,14 @@ import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import com.freeletics.flowredux.compose.rememberStateAndDispatch
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
@@ -35,6 +39,7 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import javax.inject.Inject
import io.element.android.features.verifysession.impl.VerifySelfSessionStateMachine.Event as StateMachineEvent
import io.element.android.features.verifysession.impl.VerifySelfSessionStateMachine.State as StateMachineState
@@ -43,20 +48,28 @@ class VerifySelfSessionPresenter @Inject constructor(
private val sessionVerificationService: SessionVerificationService,
private val encryptionService: EncryptionService,
private val stateMachine: VerifySelfSessionStateMachine,
private val buildMeta: BuildMeta,
) : Presenter<VerifySelfSessionState> {
@Composable
override fun present(): VerifySelfSessionState {
val coroutineScope = rememberCoroutineScope()
LaunchedEffect(Unit) {
// Force reset, just in case the service was left in a broken state
sessionVerificationService.reset()
}
val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState()
val stateAndDispatch = stateMachine.rememberStateAndDispatch()
var skipVerification by remember { mutableStateOf(false) }
val needsVerification by sessionVerificationService.needsVerificationFlow.collectAsState()
val verificationFlowStep by remember {
derivedStateOf {
stateAndDispatch.state.value.toVerificationStep(
canEnterRecoveryKey = recoveryState == RecoveryState.INCOMPLETE
)
when {
skipVerification -> VerifySelfSessionState.VerificationStep.Skipped
needsVerification -> stateAndDispatch.state.value.toVerificationStep(
canEnterRecoveryKey = recoveryState == RecoveryState.INCOMPLETE
)
else -> VerifySelfSessionState.VerificationStep.Completed
}
}
}
// Start this after observing state machine
@@ -72,10 +85,15 @@ class VerifySelfSessionPresenter @Inject constructor(
VerifySelfSessionViewEvents.DeclineVerification -> stateAndDispatch.dispatchAction(StateMachineEvent.DeclineChallenge)
VerifySelfSessionViewEvents.Cancel -> stateAndDispatch.dispatchAction(StateMachineEvent.Cancel)
VerifySelfSessionViewEvents.Reset -> stateAndDispatch.dispatchAction(StateMachineEvent.Reset)
VerifySelfSessionViewEvents.SkipVerification -> coroutineScope.launch {
sessionVerificationService.saveVerifiedState(true)
skipVerification = true
}
}
}
return VerifySelfSessionState(
verificationFlowStep = verificationFlowStep,
displaySkipButton = buildMeta.isDebuggable,
eventSink = ::handleEvents,
)
}

View File

@@ -24,6 +24,7 @@ import io.element.android.libraries.matrix.api.verification.SessionVerificationD
@Immutable
data class VerifySelfSessionState(
val verificationFlowStep: VerificationStep,
val displaySkipButton: Boolean,
val eventSink: (VerifySelfSessionViewEvents) -> Unit,
) {
@Stable
@@ -34,5 +35,6 @@ data class VerifySelfSessionState(
data object Ready : VerificationStep
data class Verifying(val data: SessionVerificationData, val state: AsyncData<Unit>) : VerificationStep
data object Completed : VerificationStep
data object Skipped : VerificationStep
}
}

View File

@@ -24,7 +24,7 @@ import io.element.android.libraries.matrix.api.verification.VerificationEmoji
open class VerifySelfSessionStateProvider : PreviewParameterProvider<VerifySelfSessionState> {
override val values: Sequence<VerifySelfSessionState>
get() = sequenceOf(
aVerifySelfSessionState(),
aVerifySelfSessionState(displaySkipButton = true),
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.AwaitingOtherDeviceResponse
),
@@ -46,6 +46,10 @@ open class VerifySelfSessionStateProvider : PreviewParameterProvider<VerifySelfS
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Initial(true)
),
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Completed,
displaySkipButton = true,
),
// Add other state here
)
}
@@ -64,9 +68,11 @@ private fun aDecimalsSessionVerificationData(
internal fun aVerifySelfSessionState(
verificationFlowStep: VerifySelfSessionState.VerificationStep = VerifySelfSessionState.VerificationStep.Initial(false),
displaySkipButton: Boolean = false,
eventSink: (VerifySelfSessionViewEvents) -> Unit = {},
) = VerifySelfSessionState(
verificationFlowStep = verificationFlowStep,
displaySkipButton = displaySkipButton,
eventSink = eventSink,
)

View File

@@ -28,8 +28,12 @@ import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.widthIn
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.MaterialTheme
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.painterResource
@@ -51,11 +55,13 @@ import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.designsystem.theme.components.Button
import io.element.android.libraries.designsystem.theme.components.Text
import io.element.android.libraries.designsystem.theme.components.TextButton
import io.element.android.libraries.designsystem.theme.components.TopAppBar
import io.element.android.libraries.matrix.api.verification.SessionVerificationData
import io.element.android.libraries.matrix.api.verification.VerificationEmoji
import io.element.android.libraries.ui.strings.CommonStrings
import io.element.android.features.verifysession.impl.VerifySelfSessionState.VerificationStep as FlowStep
@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun VerifySelfSessionView(
state: VerifySelfSessionState,
@@ -66,6 +72,12 @@ fun VerifySelfSessionView(
fun resetFlow() {
state.eventSink(VerifySelfSessionViewEvents.Reset)
}
val updatedOnFinished by rememberUpdatedState(newValue = onFinished)
LaunchedEffect(state.verificationFlowStep, updatedOnFinished) {
if (state.verificationFlowStep is FlowStep.Skipped) {
updatedOnFinished()
}
}
BackHandler {
when (state.verificationFlowStep) {
is FlowStep.Canceled -> resetFlow()
@@ -81,6 +93,19 @@ fun VerifySelfSessionView(
val verificationFlowStep = state.verificationFlowStep
HeaderFooterPage(
modifier = modifier,
topBar = {
TopAppBar(
title = {},
actions = {
if (state.displaySkipButton && state.verificationFlowStep != FlowStep.Completed) {
TextButton(
text = stringResource(CommonStrings.action_skip),
onClick = { state.eventSink(VerifySelfSessionViewEvents.SkipVerification) }
)
}
}
)
},
header = {
HeaderContent(verificationFlowStep = verificationFlowStep)
},
@@ -104,6 +129,7 @@ private fun HeaderContent(verificationFlowStep: FlowStep) {
FlowStep.Canceled -> BigIcon.Style.AlertSolid
FlowStep.Ready, is FlowStep.Verifying -> BigIcon.Style.Default(CompoundIcons.Reaction())
FlowStep.Completed -> BigIcon.Style.SuccessSolid
is FlowStep.Skipped -> return
}
val titleTextId = when (verificationFlowStep) {
is FlowStep.Initial, FlowStep.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_title
@@ -114,20 +140,21 @@ private fun HeaderContent(verificationFlowStep: FlowStep) {
is SessionVerificationData.Decimals -> R.string.screen_session_verification_compare_numbers_title
is SessionVerificationData.Emojis -> R.string.screen_session_verification_compare_emojis_title
}
is FlowStep.Skipped -> return
}
val subtitleTextId = when (verificationFlowStep) {
is FlowStep.Initial, FlowStep.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_subtitle
FlowStep.Canceled -> R.string.screen_session_verification_cancelled_subtitle
FlowStep.Ready -> R.string.screen_session_verification_ready_subtitle
FlowStep.Completed -> R.string.screen_identity_confirmation_subtitle
FlowStep.Completed -> R.string.screen_identity_confirmed_subtitle
is FlowStep.Verifying -> when (verificationFlowStep.data) {
is SessionVerificationData.Decimals -> R.string.screen_session_verification_compare_numbers_subtitle
is SessionVerificationData.Emojis -> R.string.screen_session_verification_compare_emojis_subtitle
}
is FlowStep.Skipped -> return
}
PageTitle(
modifier = Modifier.padding(top = 60.dp),
iconStyle = iconStyle,
title = stringResource(id = titleTextId),
subtitle = stringResource(id = subtitleTextId)
@@ -137,9 +164,8 @@ private fun HeaderContent(verificationFlowStep: FlowStep) {
@Composable
private fun Content(flowState: FlowStep) {
Column(Modifier.fillMaxHeight(), verticalArrangement = Arrangement.Center) {
when (flowState) {
is FlowStep.Initial, FlowStep.AwaitingOtherDeviceResponse, FlowStep.Ready, FlowStep.Canceled, FlowStep.Completed -> Unit
is FlowStep.Verifying -> ContentVerifying(flowState)
if (flowState is FlowStep.Verifying) {
ContentVerifying(flowState)
}
}
}
@@ -264,6 +290,7 @@ private fun BottomMenu(
onPositiveButtonClicked = onFinished,
)
}
is FlowStep.Skipped -> return
}
}

View File

@@ -23,4 +23,5 @@ sealed interface VerifySelfSessionViewEvents {
data object DeclineVerification : VerifySelfSessionViewEvents
data object Cancel : VerifySelfSessionViewEvents
data object Reset : VerifySelfSessionViewEvents
data object SkipVerification : VerifySelfSessionViewEvents
}

View File

@@ -23,15 +23,19 @@ import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.features.verifysession.impl.VerifySelfSessionState.VerificationStep
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.verification.SessionVerificationData
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
import io.element.android.libraries.matrix.api.verification.VerificationEmoji
import io.element.android.libraries.matrix.api.verification.VerificationFlowState
import io.element.android.libraries.matrix.test.core.aBuildMeta
import io.element.android.libraries.matrix.test.encryption.FakeEncryptionService
import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService
import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.lambda.value
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.junit.Rule
@@ -48,7 +52,21 @@ class VerifySelfSessionPresenterTests {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
awaitItem().run {
assertThat(verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
assertThat(displaySkipButton).isTrue()
}
}
}
@Test
fun `present - hides skip verification button on non-debuggable builds`() = runTest {
val buildMeta = aBuildMeta(isDebuggable = false)
val presenter = createVerifySelfSessionPresenter(buildMeta = buildMeta)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
assertThat(awaitItem().displaySkipButton).isFalse()
}
}
@@ -68,7 +86,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - Handles requestVerification`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -79,7 +97,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - Handles startSasVerification`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -113,7 +131,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - A failure when verifying cancels it`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -130,7 +148,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - A fail when requesting verification resets the state to the initial one`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -145,7 +163,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - Canceling the flow once it's verifying cancels it`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -158,7 +176,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - When verifying, if we receive another challenge we ignore it`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -171,7 +189,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - Restart after cancelation returns to requesting verification`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -188,7 +206,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - Go back after cancelation returns to initial state`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -208,7 +226,7 @@ class VerifySelfSessionPresenterTests {
val emojis = listOf(
VerificationEmoji(number = 30, emoji = "😀", description = "Smiley")
)
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -230,7 +248,7 @@ class VerifySelfSessionPresenterTests {
@Test
fun `present - When verification is declined, the flow is canceled`() = runTest {
val service = FakeSessionVerificationService()
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
@@ -247,6 +265,33 @@ class VerifySelfSessionPresenterTests {
}
}
@Test
fun `present - Skip event skips the flow`() = runTest {
val service = unverifiedSessionService()
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val state = requestVerificationAndAwaitVerifyingState(service)
state.eventSink(VerifySelfSessionViewEvents.SkipVerification)
service.saveVerifiedStateResult.assertions().isCalledOnce().with(value(true))
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Skipped)
}
}
@Test
fun `present - When verification is not needed, the flow is completed`() = runTest {
val service = FakeSessionVerificationService().apply {
givenNeedsVerification(false)
}
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Completed)
}
}
private suspend fun ReceiveTurbine<VerifySelfSessionState>.requestVerificationAndAwaitVerifyingState(
fakeService: FakeSessionVerificationService,
sessionVerificationData: SessionVerificationData = SessionVerificationData.Emojis(emptyList()),
@@ -271,14 +316,23 @@ class VerifySelfSessionPresenterTests {
return state
}
private fun unverifiedSessionService(): FakeSessionVerificationService {
return FakeSessionVerificationService().apply {
givenVerifiedStatus(SessionVerifiedStatus.NotVerified)
givenNeedsVerification(true)
}
}
private fun createVerifySelfSessionPresenter(
service: SessionVerificationService = FakeSessionVerificationService(),
service: SessionVerificationService = unverifiedSessionService(),
encryptionService: EncryptionService = FakeEncryptionService(),
buildMeta: BuildMeta = aBuildMeta(),
): VerifySelfSessionPresenter {
return VerifySelfSessionPresenter(
sessionVerificationService = service,
encryptionService = encryptionService,
stateMachine = VerifySelfSessionStateMachine(service, encryptionService),
buildMeta = buildMeta,
)
}
}

View File

@@ -22,6 +22,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.ui.strings.CommonStrings
import io.element.android.tests.testutils.EnsureNeverCalled
import io.element.android.tests.testutils.EnsureNeverCalledWithParam
import io.element.android.tests.testutils.EventsRecorder
import io.element.android.tests.testutils.clickOn
import io.element.android.tests.testutils.ensureCalledOnce
@@ -126,6 +127,23 @@ class VerifySelfSessionViewTest {
eventsRecorder.assertEmpty()
}
@Test
fun `back key pressed - on Completed step does nothing`() {
val eventsRecorder = EventsRecorder<VerifySelfSessionViewEvents>()
rule.setContent {
VerifySelfSessionView(
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Completed,
eventSink = eventsRecorder
),
onEnterRecoveryKey = EnsureNeverCalled(),
onFinished = EnsureNeverCalled(),
)
}
rule.pressBackKey()
eventsRecorder.assertEmpty()
}
@Test
fun `when flow is completed and the user clicks on the continue button, the expected callback is invoked`() {
val eventsRecorder = EventsRecorder<VerifySelfSessionViewEvents>(expectEvents = false)
@@ -202,4 +220,39 @@ class VerifySelfSessionViewTest {
rule.clickOn(R.string.screen_session_verification_they_dont_match)
eventsRecorder.assertSingle(VerifySelfSessionViewEvents.DeclineVerification)
}
@Test
fun `clicking on 'Skip' emits the expected event`() {
val eventsRecorder = EventsRecorder<VerifySelfSessionViewEvents>()
rule.setContent {
VerifySelfSessionView(
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Initial(canEnterRecoveryKey = true),
displaySkipButton = true,
eventSink = eventsRecorder
),
onEnterRecoveryKey = EnsureNeverCalled(),
onFinished = EnsureNeverCalled(),
)
}
rule.clickOn(CommonStrings.action_skip)
eventsRecorder.assertSingle(VerifySelfSessionViewEvents.SkipVerification)
}
@Test
fun `on Skipped step - onFinished callback is called immediately`() {
ensureCalledOnce { callback ->
rule.setContent {
VerifySelfSessionView(
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Skipped,
displaySkipButton = true,
eventSink = EnsureNeverCalledWithParam(),
),
onEnterRecoveryKey = EnsureNeverCalled(),
onFinished = callback,
)
}
}
}
}

View File

@@ -21,6 +21,17 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.StateFlow
interface SessionVerificationService {
/**
* This flow stores the local verification status of the current session.
*
* We should ideally base the verified status in the Rust SDK info, but there are several issues with that approach:
*
* - The SDK takes a while to report this value, resulting in a delay of 1-2s in displaying the UI.
* - We need to add a 'Skip' option for testing purposes, which would not be possible if we relied only on the SDK.
* - The SDK sometimes doesn't report the verification state if there is no network connection when the app boots.
*/
val needsVerificationFlow: StateFlow<Boolean>
/**
* State of the current verification flow ([VerificationFlowState.Initial] if not started).
*/
@@ -72,6 +83,11 @@ interface SessionVerificationService {
* Returns the verification service state to the initial step.
*/
suspend fun reset()
/**
* Saves the current session state as [verified].
*/
suspend fun saveVerifiedState(verified: Boolean)
}
/** Verification status of the current session. */

View File

@@ -150,6 +150,7 @@ class RustMatrixClient(
syncService = rustSyncService,
sessionCoroutineScope = sessionCoroutineScope,
dispatchers = dispatchers,
sessionStore = sessionStore,
)
private val roomDirectoryService = RustRoomDirectoryService(
@@ -177,6 +178,7 @@ class RustMatrixClient(
isTokenValid = false,
loginType = existingData.loginType,
passphrase = existingData.passphrase,
needsVerification = existingData.needsVerification,
)
sessionStore.updateData(newData)
Timber.d("Removed session data with token: '...$anonymizedToken'.")
@@ -204,6 +206,7 @@ class RustMatrixClient(
isTokenValid = true,
loginType = existingData.loginType,
passphrase = existingData.passphrase,
needsVerification = existingData.needsVerification,
)
sessionStore.updateData(newData)
Timber.d("Saved new session data with token: '...$anonymizedToken'.")
@@ -229,6 +232,7 @@ class RustMatrixClient(
client = client,
isSyncServiceReady = rustSyncService.syncState.map { it == SyncState.Running },
sessionCoroutineScope = sessionCoroutineScope,
sessionStore = sessionStore,
)
private val eventFilters = TimelineConfig.excludedEvents

View File

@@ -148,6 +148,7 @@ class RustMatrixAuthenticationService @Inject constructor(
isTokenValid = true,
loginType = LoginType.PASSWORD,
passphrase = pendingPassphrase,
needsVerification = true,
)
}
sessionStore.storeData(sessionData)
@@ -195,7 +196,8 @@ class RustMatrixAuthenticationService @Inject constructor(
it.session().toSessionData(
isTokenValid = true,
loginType = LoginType.OIDC,
passphrase = pendingPassphrase
passphrase = pendingPassphrase,
needsVerification = true,
)
}
pendingOidcAuthenticationData?.close()

View File

@@ -25,6 +25,7 @@ import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.sync.SyncState
import io.element.android.libraries.matrix.impl.sync.RustSyncService
import io.element.android.libraries.sessionstorage.api.SessionStore
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.channels.awaitClose
import kotlinx.coroutines.currentCoroutineContext
@@ -48,10 +49,11 @@ import org.matrix.rustcomponents.sdk.EnableRecoveryProgress as RustEnableRecover
import org.matrix.rustcomponents.sdk.SteadyStateException as RustSteadyStateException
internal class RustEncryptionService(
client: Client,
private val client: Client,
syncService: RustSyncService,
sessionCoroutineScope: CoroutineScope,
private val dispatchers: CoroutineDispatchers,
private val sessionStore: SessionStore,
) : EncryptionService {
private val service: Encryption = client.encryption()
@@ -186,6 +188,9 @@ internal class RustEncryptionService(
override suspend fun recover(recoveryKey: String): Result<Unit> = withContext(dispatchers.io) {
runCatching {
service.recover(recoveryKey)
val existingSession = sessionStore.getSession(client.userId())
?: error("Failed to save verification state. No session with id ${client.userId()}")
sessionStore.updateData(existingSession.copy(needsVerification = false))
}.mapFailure {
it.mapRecoveryException()
}

View File

@@ -25,6 +25,7 @@ internal fun Session.toSessionData(
isTokenValid: Boolean,
loginType: LoginType,
passphrase: String?,
needsVerification: Boolean,
) = SessionData(
userId = userId,
deviceId = deviceId,
@@ -37,4 +38,5 @@ internal fun Session.toSessionData(
isTokenValid = isTokenValid,
loginType = loginType,
passphrase = passphrase,
needsVerification = needsVerification,
)

View File

@@ -16,6 +16,7 @@
package io.element.android.libraries.matrix.impl.verification
import io.element.android.libraries.core.bool.orFalse
import io.element.android.libraries.core.data.tryOrNull
import io.element.android.libraries.matrix.api.verification.SessionVerificationData
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
@@ -23,108 +24,97 @@ import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatu
import io.element.android.libraries.matrix.api.verification.VerificationEmoji
import io.element.android.libraries.matrix.api.verification.VerificationFlowState
import io.element.android.libraries.matrix.impl.util.cancelAndDestroy
import io.element.android.libraries.sessionstorage.api.SessionStore
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeout
import org.matrix.rustcomponents.sdk.Client
import org.matrix.rustcomponents.sdk.Encryption
import org.matrix.rustcomponents.sdk.RecoveryState
import org.matrix.rustcomponents.sdk.RecoveryStateListener
import org.matrix.rustcomponents.sdk.SessionVerificationController
import org.matrix.rustcomponents.sdk.SessionVerificationControllerDelegate
import org.matrix.rustcomponents.sdk.TaskHandle
import org.matrix.rustcomponents.sdk.VerificationState
import org.matrix.rustcomponents.sdk.VerificationStateListener
import org.matrix.rustcomponents.sdk.use
import timber.log.Timber
import kotlin.time.Duration.Companion.seconds
import org.matrix.rustcomponents.sdk.SessionVerificationData as RustSessionVerificationData
class RustSessionVerificationService(
client: Client,
private val client: Client,
isSyncServiceReady: Flow<Boolean>,
sessionCoroutineScope: CoroutineScope,
private val sessionCoroutineScope: CoroutineScope,
private val sessionStore: SessionStore,
) : SessionVerificationService, SessionVerificationControllerDelegate {
private var verificationStateListenerTaskHandle: TaskHandle? = null
private var recoveryStateListenerTaskHandle: TaskHandle? = null
private val encryptionService: Encryption = client.encryption()
private lateinit var verificationController: SessionVerificationController
// Listen for changes in verification status and update accordingly
private val verificationStateListenerTaskHandle = encryptionService.verificationStateListener(object : VerificationStateListener {
override fun onUpdate(status: VerificationState) {
Timber.d("New verification state: $status")
updateVerificationStatus(status)
}
})
// In case we enter the recovery key instead we check changes in the recovery state, since the listener above won't be triggered
private val recoveryStateListenerTaskHandle = encryptionService.recoveryStateListener(object : RecoveryStateListener {
override fun onUpdate(status: RecoveryState) {
Timber.d("New recovery state: $status")
// We could check the `RecoveryState`, but it's easier to just use the verification state directly
updateVerificationStatus(encryptionService.verificationState())
}
})
override val needsVerificationFlow: StateFlow<Boolean> = sessionStore.sessionsFlow()
.map { sessions -> sessions.firstOrNull { it.userId == client.userId() }?.needsVerification.orFalse() }
.distinctUntilChanged()
.stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false)
private val _verificationFlowState = MutableStateFlow<VerificationFlowState>(VerificationFlowState.Initial)
override val verificationFlowState = _verificationFlowState.asStateFlow()
private val _sessionVerifiedStatus = MutableStateFlow<SessionVerifiedStatus>(SessionVerifiedStatus.Unknown)
override val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus> = _sessionVerifiedStatus.asStateFlow()
override val isReady = MutableStateFlow(false)
override val isReady = isSyncServiceReady.stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false)
override val canVerifySessionFlow = combine(sessionVerifiedStatus, isReady) { verificationStatus, isReady ->
isReady && verificationStatus == SessionVerifiedStatus.NotVerified
}
init {
isSyncServiceReady
.onEach { syncServiceReady ->
if (syncServiceReady) {
isReady.value = true
runCatching {
// If the controller was failed to initialize before, we try to get it again
if (!this::verificationController.isInitialized) {
verificationController = client.getSessionVerificationController()
}
}
.onFailure {
isReady.value = false
Timber.e(it, "Failed to get verification controller. Trying again in next sync.")
}
} else {
isReady.value = false
}
}
.launchIn(sessionCoroutineScope)
isReady.onEach { isReady ->
if (isReady) {
Timber.d("Starting verification service")
// Setup delegate
verificationController.setDelegate(this)
// Immediate status update
updateVerificationStatus(encryptionService.verificationState())
// Listen for changes in verification status and update accordingly
verificationStateListenerTaskHandle?.cancelAndDestroy()
verificationStateListenerTaskHandle = encryptionService.verificationStateListener(object : VerificationStateListener {
override fun onUpdate(status: VerificationState) {
Timber.d("New verification state: $status")
updateVerificationStatus(status)
}
})
// In case we enter the recovery key instead we check changes in the recovery state, since the listener above won't be triggered
recoveryStateListenerTaskHandle?.cancelAndDestroy()
recoveryStateListenerTaskHandle = encryptionService.recoveryStateListener(object : RecoveryStateListener {
override fun onUpdate(status: RecoveryState) {
Timber.d("New recovery state: $status")
// We could check the `RecoveryState`, but it's easier to just use the verification state directly
updateVerificationStatus(encryptionService.verificationState())
}
})
} else {
Timber.d("Stopping verification service")
if (this::verificationController.isInitialized) {
verificationController.setDelegate(null)
}
}
}
.launchIn(sessionCoroutineScope)
}
override suspend fun requestVerification() = tryOrFail {
if (!this::verificationController.isInitialized) {
verificationController = client.getSessionVerificationController()
verificationController.setDelegate(this)
}
verificationController.requestVerification()
}
@@ -164,9 +154,26 @@ class RustSessionVerificationService(
}
override fun didFinish() {
_verificationFlowState.value = VerificationFlowState.Finished
// Ideally this should be `verificationController?.isVerified().orFalse()` but for some reason it always returns false
updateVerificationStatus(VerificationState.VERIFIED)
sessionCoroutineScope.launch {
// Ideally this should be `verificationController?.isVerified().orFalse()` but for some reason it returns false if run immediately
// It also sometimes unexpectedly fails to report the session as verified, so we have to handle that possibility and fail if needed
runCatching {
withTimeout(30.seconds) {
while (!verificationController.isVerified()) {
delay(100)
}
}
}
.onSuccess {
saveVerifiedState(true)
updateVerificationStatus(VerificationState.VERIFIED)
_verificationFlowState.value = VerificationFlowState.Finished
}
.onFailure {
Timber.e(it, "Verification finished, but the Rust SDK still reports the session as unverified.")
didFail()
}
}
}
override fun didReceiveVerificationData(data: RustSessionVerificationData) {
@@ -188,12 +195,21 @@ class RustSessionVerificationService(
_verificationFlowState.value = VerificationFlowState.Initial
}
override suspend fun saveVerifiedState(verified: Boolean) = tryOrFail {
val existingSession = sessionStore.getSession(client.userId())
?: error("Failed to save verification state. No session with id ${client.userId()}")
sessionStore.updateData(existingSession.copy(needsVerification = !verified))
// Wait until the new state is saved
needsVerificationFlow.first { needsVerification -> !needsVerification }
}
fun destroy() {
Timber.d("Destroying RustSessionVerificationService")
recoveryStateListenerTaskHandle?.cancelAndDestroy()
verificationStateListenerTaskHandle.cancelAndDestroy()
recoveryStateListenerTaskHandle.cancelAndDestroy()
if (this::verificationController.isInitialized) {
verificationController.setDelegate(null)
(verificationController as? SessionVerificationController)?.destroy()
verificationController.destroy()
}
}

View File

@@ -20,17 +20,22 @@ import io.element.android.libraries.matrix.api.verification.SessionVerificationD
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
import io.element.android.libraries.matrix.api.verification.VerificationFlowState
import io.element.android.tests.testutils.lambda.LambdaOneParamRecorder
import io.element.android.tests.testutils.lambda.lambdaRecorder
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
class FakeSessionVerificationService : SessionVerificationService {
class FakeSessionVerificationService(
var saveVerifiedStateResult: LambdaOneParamRecorder<Boolean, Unit> = lambdaRecorder<Boolean, Unit> {}
) : SessionVerificationService {
private val _isReady = MutableStateFlow(false)
private val _sessionVerifiedStatus = MutableStateFlow<SessionVerifiedStatus>(SessionVerifiedStatus.Unknown)
private var _verificationFlowState = MutableStateFlow<VerificationFlowState>(VerificationFlowState.Initial)
private var _canVerifySessionFlow = MutableStateFlow(true)
var shouldFail = false
override val needsVerificationFlow: MutableStateFlow<Boolean> = MutableStateFlow(false)
override val verificationFlowState: StateFlow<VerificationFlowState> = _verificationFlowState
override val sessionVerifiedStatus: StateFlow<SessionVerifiedStatus> = _sessionVerifiedStatus
override val canVerifySessionFlow: Flow<Boolean> = _canVerifySessionFlow
@@ -89,7 +94,15 @@ class FakeSessionVerificationService : SessionVerificationService {
_isReady.value = value
}
fun givenNeedsVerification(value: Boolean) {
needsVerificationFlow.value = value
}
override suspend fun reset() {
_verificationFlowState.value = VerificationFlowState.Initial
}
override suspend fun saveVerifiedState(verified: Boolean) {
saveVerifiedStateResult(verified)
}
}

View File

@@ -18,16 +18,32 @@ package io.element.android.libraries.sessionstorage.api
import java.util.Date
/**
* Data class representing the session data to store locally.
*/
data class SessionData(
/** The user ID of the logged in user. */
val userId: String,
/** The device ID of the session. */
val deviceId: String,
/** The current access token of the session. */
val accessToken: String,
/** The optional current refresh token of the session. */
val refreshToken: String?,
/** The homeserver URL of the session. */
val homeserverUrl: String,
/** The Open ID Connect info for this session, if any. */
val oidcData: String?,
/** The Sliding Sync Proxy URL for this session, if any. */
val slidingSyncProxy: String?,
/** The timestamp of the last login. May be `null` in very old sessions. */
val loginTimestamp: Date?,
/** Whether the [accessToken] is valid or not. */
val isTokenValid: Boolean,
/** The login type used to authenticate the session. */
val loginType: LoginType,
/** The optional passphrase used to encrypt data in the SDK local store. */
val passphrase: String?,
/** Whether the session needs verification. */
val needsVerification: Boolean,
)

View File

@@ -34,6 +34,7 @@ internal fun SessionData.toDbModel(): DbSessionData {
isTokenValid = if (isTokenValid) 1L else 0L,
loginType = loginType.name,
passphrase = passphrase,
needsVerification = if (needsVerification) 1L else 0L,
)
}
@@ -50,5 +51,6 @@ internal fun DbSessionData.toApiModel(): SessionData {
isTokenValid = isTokenValid == 1L,
loginType = LoginType.fromName(loginType ?: LoginType.UNKNOWN.name),
passphrase = passphrase,
needsVerification = needsVerification == 1L,
)
}

View File

@@ -23,7 +23,9 @@ CREATE TABLE SessionData (
isTokenValid INTEGER NOT NULL DEFAULT 1,
loginType TEXT,
-- added in version 5
passphrase TEXT
passphrase TEXT,
-- added in version 6
needsVerification INTEGER NOT NULL DEFAULT 0
);

View File

@@ -0,0 +1,4 @@
-- Migrate DB from version 5
-- For users migrating previously logged in sessions, we force them to verify them too
ALTER TABLE SessionData ADD COLUMN needsVerification INTEGER NOT NULL DEFAULT 1;

View File

@@ -144,6 +144,7 @@ class DatabaseSessionStoreTests {
isTokenValid = 1,
loginType = null,
passphrase = "aPassphrase",
needsVerification = 1L,
)
val secondSessionData = SessionData(
userId = "userId",
@@ -157,6 +158,7 @@ class DatabaseSessionStoreTests {
isTokenValid = 1,
loginType = null,
passphrase = "aPassphraseAltered",
needsVerification = 0L,
)
assertThat(firstSessionData.userId).isEqualTo(secondSessionData.userId)
assertThat(firstSessionData.loginTimestamp).isNotEqualTo(secondSessionData.loginTimestamp)
@@ -177,6 +179,7 @@ class DatabaseSessionStoreTests {
assertThat(alteredSession.loginTimestamp).isEqualTo(firstSessionData.loginTimestamp)
assertThat(alteredSession.oidcData).isEqualTo(secondSessionData.oidcData)
assertThat(alteredSession.passphrase).isEqualTo(secondSessionData.passphrase)
assertThat(alteredSession.needsVerification).isEqualTo(secondSessionData.needsVerification)
}
@Test
@@ -193,6 +196,7 @@ class DatabaseSessionStoreTests {
isTokenValid = 1,
loginType = null,
passphrase = "aPassphrase",
needsVerification = 1L,
)
val secondSessionData = SessionData(
userId = "userIdUnknown",
@@ -206,6 +210,7 @@ class DatabaseSessionStoreTests {
isTokenValid = 1,
loginType = null,
passphrase = "aPassphraseAltered",
needsVerification = 0L,
)
assertThat(firstSessionData.userId).isNotEqualTo(secondSessionData.userId)
@@ -224,5 +229,6 @@ class DatabaseSessionStoreTests {
assertThat(notAlteredSession.loginTimestamp).isEqualTo(firstSessionData.loginTimestamp)
assertThat(notAlteredSession.oidcData).isEqualTo(firstSessionData.oidcData)
assertThat(notAlteredSession.passphrase).isEqualTo(firstSessionData.passphrase)
assertThat(notAlteredSession.needsVerification).isEqualTo(firstSessionData.needsVerification)
}
}

View File

@@ -31,4 +31,5 @@ internal fun aSessionData() = SessionData(
isTokenValid = 1,
loginType = LoginType.UNKNOWN.name,
passphrase = null,
needsVerification = 0L,
)