From 1de679767315674b5db42898b703bae715dee98c Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Wed, 24 Apr 2024 15:55:25 +0200 Subject: [PATCH] Remove `SessionData.needsVerification` as the source of truth for session verification status (#2748) * Remove `SessionData.needsVerification` as the source of truth for session verification status. - Use the Rust SDK `EncryptionService.verificationState()` instead, but always waiting for the first 'known' result (either verified or not, discarding 'unknown'). - Add a workaround in the super rare case when reading this value gets stuck somehow. We'll assume the user is not verified in that case. - Make `DefaultFtueService.getNextStep` and dependent checks `suspend`. - Make the `skip` button use a value in the session preferences instead. * Log exception when the verification status can't be loaded Co-authored-by: Benoit Marty * Fix review comments --------- Co-authored-by: Benoit Marty --- changelog.d/2718.bugfix | 1 + features/ftue/impl/build.gradle.kts | 2 + .../features/ftue/impl/FtueFlowNode.kt | 2 +- .../ftue/impl/state/DefaultFtueService.kt | 47 ++++++++++++------ .../ftue/impl/DefaultFtueServiceTests.kt | 6 ++- .../signedout/impl/SignedOutStateProvider.kt | 2 - features/verifysession/impl/build.gradle.kts | 2 + .../impl/VerifySelfSessionPresenter.kt | 10 ++-- .../impl/VerifySelfSessionPresenterTests.kt | 12 +++-- .../SessionVerificationService.kt | 16 ------ .../libraries/matrix/impl/RustMatrixClient.kt | 4 -- .../auth/RustMatrixAuthenticationService.kt | 2 - .../impl/encryption/RustEncryptionService.kt | 7 +-- .../libraries/matrix/impl/mapper/Session.kt | 2 - .../RustSessionVerificationService.kt | 23 ++------- .../FakeSessionVerificationService.kt | 15 +----- .../api/store/SessionPreferencesStore.kt | 3 ++ .../store/DefaultSessionPreferencesStore.kt | 4 ++ .../test/InMemorySessionPreferencesStore.kt | 10 ++++ .../sessionstorage/api/SessionData.kt | 2 - .../sessionstorage/impl/SessionDataMapper.kt | 2 - .../impl/src/main/sqldelight/databases/7.db | Bin 0 -> 12288 bytes .../libraries/matrix/session/SessionData.sq | 4 +- .../impl/src/main/sqldelight/migrations/6.sqm | 20 ++++++++ .../impl/DatabaseSessionStoreTests.kt | 6 --- .../libraries/sessionstorage/impl/Fixtures.kt | 1 - 26 files changed, 99 insertions(+), 106 deletions(-) create mode 100644 changelog.d/2718.bugfix create mode 100644 libraries/session-storage/impl/src/main/sqldelight/databases/7.db create mode 100644 libraries/session-storage/impl/src/main/sqldelight/migrations/6.sqm diff --git a/changelog.d/2718.bugfix b/changelog.d/2718.bugfix new file mode 100644 index 0000000000..999d47b59b --- /dev/null +++ b/changelog.d/2718.bugfix @@ -0,0 +1 @@ +Fix session verification being asked again for already verified users. diff --git a/features/ftue/impl/build.gradle.kts b/features/ftue/impl/build.gradle.kts index 06251cfe5d..e42763d97c 100644 --- a/features/ftue/impl/build.gradle.kts +++ b/features/ftue/impl/build.gradle.kts @@ -39,6 +39,7 @@ dependencies { implementation(projects.libraries.matrix.api) implementation(projects.libraries.matrixui) implementation(projects.libraries.designsystem) + implementation(projects.libraries.preferences.api) implementation(projects.libraries.uiStrings) implementation(projects.libraries.testtags) implementation(projects.features.analytics.api) @@ -60,6 +61,7 @@ dependencies { testImplementation(projects.services.analytics.test) testImplementation(projects.libraries.permissions.impl) testImplementation(projects.libraries.permissions.test) + testImplementation(projects.libraries.preferences.test) testImplementation(projects.features.lockscreen.test) testImplementation(projects.tests.testutils) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt index cb77100d94..3616faf2ae 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt @@ -143,7 +143,7 @@ class FtueFlowNode @AssistedInject constructor( } } - private fun moveToNextStep() { + private fun moveToNextStep() = lifecycleScope.launch { when (ftueState.getNextStep()) { FtueStep.SessionVerification -> { backstack.newRoot(NavTarget.SessionVerification) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt index 87b7c3a7ee..0e8aa54f81 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt @@ -23,18 +23,26 @@ import com.squareup.anvil.annotations.ContributesBinding import io.element.android.features.ftue.api.state.FtueService import io.element.android.features.ftue.api.state.FtueState import io.element.android.features.lockscreen.api.LockScreenService +import io.element.android.features.preferences.api.store.SessionPreferencesStore 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 import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.catch +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.timeout import kotlinx.coroutines.runBlocking +import timber.log.Timber import javax.inject.Inject +import kotlin.time.Duration.Companion.seconds @ContributesBinding(SessionScope::class) class DefaultFtueService @Inject constructor( @@ -44,6 +52,7 @@ class DefaultFtueService @Inject constructor( private val permissionStateProvider: PermissionStateProvider, private val lockScreenService: LockScreenService, private val sessionVerificationService: SessionVerificationService, + private val sessionPreferencesStore: SessionPreferencesStore, ) : FtueService { override val state = MutableStateFlow(FtueState.Unknown) @@ -55,7 +64,7 @@ class DefaultFtueService @Inject constructor( } init { - sessionVerificationService.needsVerificationFlow + sessionVerificationService.sessionVerifiedStatus .onEach { updateState() } .launchIn(coroutineScope) @@ -64,7 +73,7 @@ class DefaultFtueService @Inject constructor( .launchIn(coroutineScope) } - fun getNextStep(currentStep: FtueStep? = null): FtueStep? = + suspend fun getNextStep(currentStep: FtueStep? = null): FtueStep? = when (currentStep) { null -> if (isSessionNotVerified()) { FtueStep.SessionVerification @@ -89,8 +98,8 @@ class DefaultFtueService @Inject constructor( FtueStep.AnalyticsOptIn -> null } - private fun isAnyStepIncomplete(): Boolean { - return listOf( + private suspend fun isAnyStepIncomplete(): Boolean { + return listOf Boolean>( { isSessionNotVerified() }, { shouldAskNotificationPermissions() }, { needsAnalyticsOptIn() }, @@ -98,16 +107,28 @@ class DefaultFtueService @Inject constructor( ).any { it() } } - private fun isSessionNotVerified(): Boolean { - return sessionVerificationService.needsVerificationFlow.value + @OptIn(FlowPreview::class) + private suspend fun isSessionNotVerified(): Boolean { + // Wait for the first known (or ready) verification status + val readyVerifiedSessionStatus = sessionVerificationService.sessionVerifiedStatus + .filter { it != SessionVerifiedStatus.Unknown } + // This is not ideal, but there are some very rare cases when reading the flow seems to get stuck + .timeout(5.seconds) + .catch { + Timber.e(it, "Failed to get session verification status, assume it's not verified") + emit(SessionVerifiedStatus.NotVerified) + } + .first() + val skipVerification = suspend { sessionPreferencesStore.isSessionVerificationSkipped().first() } + return readyVerifiedSessionStatus == SessionVerifiedStatus.NotVerified && !skipVerification() } - private fun needsAnalyticsOptIn(): Boolean { + private suspend fun needsAnalyticsOptIn(): Boolean { // We need this function to not be suspend, so we need to load the value through runBlocking - return runBlocking { analyticsService.didAskUserConsent().first().not() } + return analyticsService.didAskUserConsent().first().not() } - private fun shouldAskNotificationPermissions(): Boolean { + private suspend fun shouldAskNotificationPermissions(): Boolean { return if (sdkVersionProvider.isAtLeast(Build.VERSION_CODES.TIRAMISU)) { val permission = Manifest.permission.POST_NOTIFICATIONS val isPermissionDenied = runBlocking { permissionStateProvider.isPermissionDenied(permission).first() } @@ -118,14 +139,12 @@ class DefaultFtueService @Inject constructor( } } - private fun shouldDisplayLockscreenSetup(): Boolean { - return runBlocking { - lockScreenService.isSetupRequired().first() - } + private suspend fun shouldDisplayLockscreenSetup(): Boolean { + return lockScreenService.isSetupRequired().first() } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal fun updateState() { + internal suspend fun updateState() { state.value = when { isAnyStepIncomplete() -> FtueState.Incomplete else -> FtueState.Complete diff --git a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTests.kt b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTests.kt index d381d945a9..9cc3db8d29 100644 --- a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTests.kt +++ b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTests.kt @@ -24,6 +24,7 @@ import io.element.android.features.ftue.impl.state.DefaultFtueService import io.element.android.features.ftue.impl.state.FtueStep import io.element.android.features.lockscreen.api.LockScreenService import io.element.android.features.lockscreen.test.FakeLockScreenService +import io.element.android.libraries.featureflag.test.InMemorySessionPreferencesStore import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService import io.element.android.libraries.permissions.impl.FakePermissionStateProvider @@ -90,7 +91,6 @@ class DefaultFtueServiceTests { fun `traverse flow`() = runTest { val sessionVerificationService = FakeSessionVerificationService().apply { givenVerifiedStatus(SessionVerifiedStatus.NotVerified) - givenNeedsVerification(true) } val analyticsService = FakeAnalyticsService() val permissionStateProvider = FakePermissionStateProvider(permissionGranted = false) @@ -108,7 +108,7 @@ class DefaultFtueServiceTests { // Session verification steps.add(state.getNextStep(steps.lastOrNull())) - sessionVerificationService.givenNeedsVerification(false) + sessionVerificationService.givenVerifiedStatus(SessionVerifiedStatus.NotVerified) // Notifications opt in steps.add(state.getNextStep(steps.lastOrNull())) @@ -200,6 +200,7 @@ class DefaultFtueServiceTests { analyticsService: AnalyticsService = FakeAnalyticsService(), permissionStateProvider: FakePermissionStateProvider = FakePermissionStateProvider(permissionGranted = false), lockScreenService: LockScreenService = FakeLockScreenService(), + sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(), // First version where notification permission is required sdkIntVersion: Int = Build.VERSION_CODES.TIRAMISU, ) = DefaultFtueService( @@ -209,5 +210,6 @@ class DefaultFtueServiceTests { analyticsService = analyticsService, permissionStateProvider = permissionStateProvider, lockScreenService = lockScreenService, + sessionPreferencesStore = sessionPreferencesStore, ) } diff --git a/features/signedout/impl/src/main/kotlin/io/element/android/features/signedout/impl/SignedOutStateProvider.kt b/features/signedout/impl/src/main/kotlin/io/element/android/features/signedout/impl/SignedOutStateProvider.kt index 9827d8d720..df3549b0f8 100644 --- a/features/signedout/impl/src/main/kotlin/io/element/android/features/signedout/impl/SignedOutStateProvider.kt +++ b/features/signedout/impl/src/main/kotlin/io/element/android/features/signedout/impl/SignedOutStateProvider.kt @@ -38,7 +38,6 @@ fun aSignedOutState() = SignedOutState( fun aSessionData( sessionId: SessionId = SessionId("@alice:server.org"), isTokenValid: Boolean = false, - needsVerification: Boolean = false, ): SessionData { return SessionData( userId = sessionId.value, @@ -52,6 +51,5 @@ fun aSessionData( isTokenValid = isTokenValid, loginType = LoginType.UNKNOWN, passphrase = null, - needsVerification = needsVerification, ) } diff --git a/features/verifysession/impl/build.gradle.kts b/features/verifysession/impl/build.gradle.kts index 3f62c6898e..38cb453a05 100644 --- a/features/verifysession/impl/build.gradle.kts +++ b/features/verifysession/impl/build.gradle.kts @@ -42,6 +42,7 @@ dependencies { implementation(projects.libraries.matrix.api) implementation(projects.libraries.matrixui) implementation(projects.libraries.designsystem) + implementation(projects.libraries.preferences.api) implementation(projects.libraries.uiStrings) api(libs.statemachine) api(projects.features.verifysession.api) @@ -53,6 +54,7 @@ dependencies { testImplementation(libs.test.truth) testImplementation(libs.test.turbine) testImplementation(projects.libraries.matrix.test) + testImplementation(projects.libraries.preferences.test) testImplementation(projects.tests.testutils) testImplementation(libs.androidx.compose.ui.test.junit) testReleaseImplementation(libs.androidx.compose.ui.test.manifest) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt index c09b48946d..e414f1b746 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt @@ -23,11 +23,11 @@ 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.features.preferences.api.store.SessionPreferencesStore import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.meta.BuildMeta @@ -49,6 +49,7 @@ class VerifySelfSessionPresenter @Inject constructor( private val encryptionService: EncryptionService, private val stateMachine: VerifySelfSessionStateMachine, private val buildMeta: BuildMeta, + private val sessionPreferencesStore: SessionPreferencesStore, ) : Presenter { @Composable override fun present(): VerifySelfSessionState { @@ -59,8 +60,8 @@ class VerifySelfSessionPresenter @Inject constructor( } val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState() val stateAndDispatch = stateMachine.rememberStateAndDispatch() - var skipVerification by remember { mutableStateOf(false) } - val needsVerification by sessionVerificationService.needsVerificationFlow.collectAsState() + val skipVerification by sessionPreferencesStore.isSessionVerificationSkipped().collectAsState(initial = false) + val needsVerification by sessionVerificationService.canVerifySessionFlow.collectAsState(initial = true) val verificationFlowStep by remember { derivedStateOf { when { @@ -86,8 +87,7 @@ class VerifySelfSessionPresenter @Inject constructor( VerifySelfSessionViewEvents.Cancel -> stateAndDispatch.dispatchAction(StateMachineEvent.Cancel) VerifySelfSessionViewEvents.Reset -> stateAndDispatch.dispatchAction(StateMachineEvent.Reset) VerifySelfSessionViewEvents.SkipVerification -> coroutineScope.launch { - sessionVerificationService.saveVerifiedState(true) - skipVerification = true + sessionPreferencesStore.setSkipSessionVerification(true) } } } diff --git a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt index 06f69e1628..9100cc65c9 100644 --- a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt +++ b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTests.kt @@ -24,6 +24,7 @@ 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.featureflag.test.InMemorySessionPreferencesStore 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 @@ -35,7 +36,6 @@ 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 @@ -289,7 +289,6 @@ class VerifySelfSessionPresenterTests { }.test { val state = requestVerificationAndAwaitVerifyingState(service) state.eventSink(VerifySelfSessionViewEvents.SkipVerification) - service.saveVerifiedStateResult.assertions().isCalledOnce().with(value(true)) assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Skipped) } } @@ -297,12 +296,16 @@ class VerifySelfSessionPresenterTests { @Test fun `present - When verification is not needed, the flow is completed`() = runTest { val service = FakeSessionVerificationService().apply { - givenNeedsVerification(false) + givenCanVerifySession(false) + givenIsReady(true) + givenVerifiedStatus(SessionVerifiedStatus.Verified) + givenVerificationFlowState(VerificationFlowState.Finished) } val presenter = createVerifySelfSessionPresenter(service) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Completed) } } @@ -334,7 +337,6 @@ class VerifySelfSessionPresenterTests { private fun unverifiedSessionService(): FakeSessionVerificationService { return FakeSessionVerificationService().apply { givenVerifiedStatus(SessionVerifiedStatus.NotVerified) - givenNeedsVerification(true) } } @@ -342,12 +344,14 @@ class VerifySelfSessionPresenterTests { service: SessionVerificationService = unverifiedSessionService(), encryptionService: EncryptionService = FakeEncryptionService(), buildMeta: BuildMeta = aBuildMeta(), + sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(), ): VerifySelfSessionPresenter { return VerifySelfSessionPresenter( sessionVerificationService = service, encryptionService = encryptionService, stateMachine = VerifySelfSessionStateMachine(service, encryptionService), buildMeta = buildMeta, + sessionPreferencesStore = sessionPreferencesStore, ) } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt index b82dd23188..8d22fc174e 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/verification/SessionVerificationService.kt @@ -21,17 +21,6 @@ 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 - /** * State of the current verification flow ([VerificationFlowState.Initial] if not started). */ @@ -83,11 +72,6 @@ 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. */ diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index a24c4a35bb..91036df1cc 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -160,7 +160,6 @@ class RustMatrixClient( syncService = rustSyncService, sessionCoroutineScope = sessionCoroutineScope, dispatchers = dispatchers, - sessionStore = sessionStore, ) private val roomDirectoryService = RustRoomDirectoryService( @@ -188,7 +187,6 @@ class RustMatrixClient( isTokenValid = false, loginType = existingData.loginType, passphrase = existingData.passphrase, - needsVerification = existingData.needsVerification, ) sessionStore.updateData(newData) Timber.d("Removed session data with token: '...$anonymizedToken'.") @@ -216,7 +214,6 @@ 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'.") @@ -242,7 +239,6 @@ class RustMatrixClient( client = client, isSyncServiceReady = rustSyncService.syncState.map { it == SyncState.Running }, sessionCoroutineScope = sessionCoroutineScope, - sessionStore = sessionStore, ) private val eventFilters = TimelineConfig.excludedEvents diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt index 0ac0ce94f3..50d82c1e3a 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt @@ -138,7 +138,6 @@ class RustMatrixAuthenticationService @Inject constructor( isTokenValid = true, loginType = LoginType.PASSWORD, passphrase = pendingPassphrase, - needsVerification = true, ) } sessionStore.storeData(sessionData) @@ -187,7 +186,6 @@ class RustMatrixAuthenticationService @Inject constructor( isTokenValid = true, loginType = LoginType.OIDC, passphrase = pendingPassphrase, - needsVerification = true, ) } pendingOidcAuthenticationData?.close() diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt index 25881c2174..f5a6390989 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt @@ -25,7 +25,6 @@ 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 @@ -49,11 +48,10 @@ import org.matrix.rustcomponents.sdk.EnableRecoveryProgress as RustEnableRecover import org.matrix.rustcomponents.sdk.SteadyStateException as RustSteadyStateException internal class RustEncryptionService( - private val client: Client, + client: Client, syncService: RustSyncService, sessionCoroutineScope: CoroutineScope, private val dispatchers: CoroutineDispatchers, - private val sessionStore: SessionStore, ) : EncryptionService { private val service: Encryption = client.encryption() @@ -188,9 +186,6 @@ internal class RustEncryptionService( override suspend fun recover(recoveryKey: String): Result = 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() } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/mapper/Session.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/mapper/Session.kt index 3c1e3c40ec..aea838b705 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/mapper/Session.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/mapper/Session.kt @@ -25,7 +25,6 @@ internal fun Session.toSessionData( isTokenValid: Boolean, loginType: LoginType, passphrase: String?, - needsVerification: Boolean, ) = SessionData( userId = userId, deviceId = deviceId, @@ -38,5 +37,4 @@ internal fun Session.toSessionData( isTokenValid = isTokenValid, loginType = loginType, passphrase = passphrase, - needsVerification = needsVerification, ) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt index 3d2fe0cc40..88403e8928 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/verification/RustSessionVerificationService.kt @@ -16,7 +16,6 @@ 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 @@ -24,7 +23,6 @@ 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 @@ -33,10 +31,7 @@ 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 @@ -58,7 +53,6 @@ class RustSessionVerificationService( private val client: Client, isSyncServiceReady: Flow, private val sessionCoroutineScope: CoroutineScope, - private val sessionStore: SessionStore, ) : SessionVerificationService, SessionVerificationControllerDelegate { private val encryptionService: Encryption = client.encryption() private lateinit var verificationController: SessionVerificationController @@ -80,11 +74,6 @@ class RustSessionVerificationService( } }) - override val needsVerificationFlow: StateFlow = sessionStore.sessionsFlow() - .map { sessions -> sessions.firstOrNull { it.userId == client.userId() }?.needsVerification.orFalse() } - .distinctUntilChanged() - .stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false) - private val _verificationFlowState = MutableStateFlow(VerificationFlowState.Initial) override val verificationFlowState = _verificationFlowState.asStateFlow() @@ -98,6 +87,9 @@ class RustSessionVerificationService( } init { + // Update initial state in case sliding sync isn't ready + updateVerificationStatus(encryptionService.verificationState()) + isReady.onEach { isReady -> if (isReady) { Timber.d("Starting verification service") @@ -165,7 +157,6 @@ class RustSessionVerificationService( } } .onSuccess { - saveVerifiedState(true) updateVerificationStatus(VerificationState.VERIFIED) _verificationFlowState.value = VerificationFlowState.Finished } @@ -195,14 +186,6 @@ 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") verificationStateListenerTaskHandle.cancelAndDestroy() diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt index 7823910263..64c7c8ab97 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/verification/FakeSessionVerificationService.kt @@ -20,22 +20,17 @@ 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( - var saveVerifiedStateResult: LambdaOneParamRecorder = lambdaRecorder {} -) : SessionVerificationService { +class FakeSessionVerificationService : SessionVerificationService { private val _isReady = MutableStateFlow(false) private val _sessionVerifiedStatus = MutableStateFlow(SessionVerifiedStatus.Unknown) private var _verificationFlowState = MutableStateFlow(VerificationFlowState.Initial) private var _canVerifySessionFlow = MutableStateFlow(true) var shouldFail = false - override val needsVerificationFlow: MutableStateFlow = MutableStateFlow(false) override val verificationFlowState: StateFlow = _verificationFlowState override val sessionVerifiedStatus: StateFlow = _sessionVerifiedStatus override val canVerifySessionFlow: Flow = _canVerifySessionFlow @@ -94,15 +89,7 @@ class FakeSessionVerificationService( _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) - } } diff --git a/libraries/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/store/SessionPreferencesStore.kt b/libraries/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/store/SessionPreferencesStore.kt index 948e9acb75..9bbeadc709 100644 --- a/libraries/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/store/SessionPreferencesStore.kt +++ b/libraries/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/store/SessionPreferencesStore.kt @@ -34,5 +34,8 @@ interface SessionPreferencesStore { suspend fun setRenderTypingNotifications(enabled: Boolean) fun isRenderTypingNotificationsEnabled(): Flow + suspend fun setSkipSessionVerification(skip: Boolean) + fun isSessionVerificationSkipped(): Flow + suspend fun clear() } diff --git a/libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt b/libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt index 05bbe9c048..41b52d87f0 100644 --- a/libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt +++ b/libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt @@ -49,6 +49,7 @@ class DefaultSessionPreferencesStore( private val renderReadReceiptsKey = booleanPreferencesKey("renderReadReceipts") private val sendTypingNotificationsKey = booleanPreferencesKey("sendTypingNotifications") private val renderTypingNotificationsKey = booleanPreferencesKey("renderTypingNotifications") + private val skipSessionVerification = booleanPreferencesKey("skipSessionVerification") private val dataStoreFile = storeFile(context, sessionId) private val store = PreferenceDataStoreFactory.create( @@ -86,6 +87,9 @@ class DefaultSessionPreferencesStore( override suspend fun setRenderTypingNotifications(enabled: Boolean) = update(renderTypingNotificationsKey, enabled) override fun isRenderTypingNotificationsEnabled(): Flow = get(renderTypingNotificationsKey) { true } + override suspend fun setSkipSessionVerification(skip: Boolean) = update(skipSessionVerification, skip) + override fun isSessionVerificationSkipped(): Flow = get(skipSessionVerification) { false } + override suspend fun clear() { dataStoreFile.safeDelete() } diff --git a/libraries/preferences/test/src/main/kotlin/io/element/android/libraries/featureflag/test/InMemorySessionPreferencesStore.kt b/libraries/preferences/test/src/main/kotlin/io/element/android/libraries/featureflag/test/InMemorySessionPreferencesStore.kt index 9e9fc7ba2a..16f98d0870 100644 --- a/libraries/preferences/test/src/main/kotlin/io/element/android/libraries/featureflag/test/InMemorySessionPreferencesStore.kt +++ b/libraries/preferences/test/src/main/kotlin/io/element/android/libraries/featureflag/test/InMemorySessionPreferencesStore.kt @@ -26,12 +26,14 @@ class InMemorySessionPreferencesStore( isRenderReadReceiptsEnabled: Boolean = true, isSendTypingNotificationsEnabled: Boolean = true, isRenderTypingNotificationsEnabled: Boolean = true, + isSessionVerificationSkipped: Boolean = false, ) : SessionPreferencesStore { private val isSharePresenceEnabled = MutableStateFlow(isSharePresenceEnabled) private val isSendPublicReadReceiptsEnabled = MutableStateFlow(isSendPublicReadReceiptsEnabled) private val isRenderReadReceiptsEnabled = MutableStateFlow(isRenderReadReceiptsEnabled) private val isSendTypingNotificationsEnabled = MutableStateFlow(isSendTypingNotificationsEnabled) private val isRenderTypingNotificationsEnabled = MutableStateFlow(isRenderTypingNotificationsEnabled) + private val isSessionVerificationSkipped = MutableStateFlow(isSessionVerificationSkipped) var clearCallCount = 0 private set @@ -65,6 +67,14 @@ class InMemorySessionPreferencesStore( override fun isRenderTypingNotificationsEnabled(): Flow = isRenderTypingNotificationsEnabled + override suspend fun setSkipSessionVerification(skip: Boolean) { + isSessionVerificationSkipped.tryEmit(skip) + } + + override fun isSessionVerificationSkipped(): Flow { + return isSessionVerificationSkipped + } + override suspend fun clear() { clearCallCount++ isSendPublicReadReceiptsEnabled.tryEmit(true) diff --git a/libraries/session-storage/api/src/main/kotlin/io/element/android/libraries/sessionstorage/api/SessionData.kt b/libraries/session-storage/api/src/main/kotlin/io/element/android/libraries/sessionstorage/api/SessionData.kt index dffc1d1bbb..76bd7f743a 100644 --- a/libraries/session-storage/api/src/main/kotlin/io/element/android/libraries/sessionstorage/api/SessionData.kt +++ b/libraries/session-storage/api/src/main/kotlin/io/element/android/libraries/sessionstorage/api/SessionData.kt @@ -44,6 +44,4 @@ data class SessionData( 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, ) diff --git a/libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/SessionDataMapper.kt b/libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/SessionDataMapper.kt index 6b79e867d0..3824def48c 100644 --- a/libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/SessionDataMapper.kt +++ b/libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/SessionDataMapper.kt @@ -34,7 +34,6 @@ internal fun SessionData.toDbModel(): DbSessionData { isTokenValid = if (isTokenValid) 1L else 0L, loginType = loginType.name, passphrase = passphrase, - needsVerification = if (needsVerification) 1L else 0L, ) } @@ -51,6 +50,5 @@ internal fun DbSessionData.toApiModel(): SessionData { isTokenValid = isTokenValid == 1L, loginType = LoginType.fromName(loginType ?: LoginType.UNKNOWN.name), passphrase = passphrase, - needsVerification = needsVerification == 1L, ) } diff --git a/libraries/session-storage/impl/src/main/sqldelight/databases/7.db b/libraries/session-storage/impl/src/main/sqldelight/databases/7.db new file mode 100644 index 0000000000000000000000000000000000000000..61805e1317fc2c710b044978b9edfce2c5e30ef4 GIT binary patch literal 12288 zcmeI#%TB^T6b9faUPD4)$Mz&_G*J^T>_x4HSZ`2_uE^L9FtN1QDFoIoOnfV!gx7HC zOdBE^4T($hPcnt!oHLiNY^HhR$V5;liu*jFeO6+Y#SV!WW0vkqy3d*!FV6=0(~K82 zi|veF3&nSqD@<8oT6|knfH(vo009U<00Izz00bZa0SNp_;JIzBl`9qNWhLPcfoKY) zWE7t8gwK7=9XITnYm-|$a%`Fl&_oi$77M)nA-e)}fxfk_?UV0?rRvi3PrvmBAu-lx3-gO*} zCuvSryS+`^_k~X~Bt3ySHSMMQ45F?KU8$!|cz;0kOV>WL8^2fWr&W>kJ;SzOiGws* zW=Fl{dVjM8KIKmA(^|`MX*XLl+s|Ym7EE1Dryg*n2ECZ8EIq$j&8?Q}6^qHx7i0Al z=-Ygo50fYzx947Fdv|VN?)B^DA5}1ql6jy(00Izz00bZa0SG_<0uX=z1R(HV1@h)# fxc~px*NbyO00Izz00bZa0SG_<0uX=z1X6(y0Z^;Q literal 0 HcmV?d00001 diff --git a/libraries/session-storage/impl/src/main/sqldelight/io/element/android/libraries/matrix/session/SessionData.sq b/libraries/session-storage/impl/src/main/sqldelight/io/element/android/libraries/matrix/session/SessionData.sq index 56b036ec96..c33b4d7c7e 100644 --- a/libraries/session-storage/impl/src/main/sqldelight/io/element/android/libraries/matrix/session/SessionData.sq +++ b/libraries/session-storage/impl/src/main/sqldelight/io/element/android/libraries/matrix/session/SessionData.sq @@ -23,9 +23,7 @@ CREATE TABLE SessionData ( isTokenValid INTEGER NOT NULL DEFAULT 1, loginType TEXT, -- added in version 5 - passphrase TEXT, - -- added in version 6 - needsVerification INTEGER NOT NULL DEFAULT 0 + passphrase TEXT ); diff --git a/libraries/session-storage/impl/src/main/sqldelight/migrations/6.sqm b/libraries/session-storage/impl/src/main/sqldelight/migrations/6.sqm new file mode 100644 index 0000000000..e4eccfdb8d --- /dev/null +++ b/libraries/session-storage/impl/src/main/sqldelight/migrations/6.sqm @@ -0,0 +1,20 @@ +-- Migrate DB from version 6 +-- Remove DB value for verified status, we're back to using the Rust SDK as a source of truth + +CREATE TABLE SessionData_bak ( + userId TEXT NOT NULL PRIMARY KEY, + deviceId TEXT NOT NULL, + accessToken TEXT NOT NULL, + refreshToken TEXT, + homeserverUrl TEXT NOT NULL, + slidingSyncProxy TEXT, + loginTimestamp INTEGER, + oidcData TEXT, + isTokenValid INTEGER NOT NULL DEFAULT 1, + loginType TEXT, + passphrase TEXT +); + +INSERT INTO SessionData_bak SELECT userId, deviceId, accessToken, refreshToken, homeserverUrl, slidingSyncProxy, loginTimestamp, oidcData, isTokenValid, loginType, passphrase FROM SessionData; +DROP TABLE SessionData; +ALTER TABLE SessionData_bak RENAME TO SessionData; diff --git a/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/DatabaseSessionStoreTests.kt b/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/DatabaseSessionStoreTests.kt index df35ec5944..46e90f6d52 100644 --- a/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/DatabaseSessionStoreTests.kt +++ b/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/DatabaseSessionStoreTests.kt @@ -144,7 +144,6 @@ class DatabaseSessionStoreTests { isTokenValid = 1, loginType = null, passphrase = "aPassphrase", - needsVerification = 1L, ) val secondSessionData = SessionData( userId = "userId", @@ -158,7 +157,6 @@ class DatabaseSessionStoreTests { isTokenValid = 1, loginType = null, passphrase = "aPassphraseAltered", - needsVerification = 0L, ) assertThat(firstSessionData.userId).isEqualTo(secondSessionData.userId) assertThat(firstSessionData.loginTimestamp).isNotEqualTo(secondSessionData.loginTimestamp) @@ -179,7 +177,6 @@ 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 @@ -196,7 +193,6 @@ class DatabaseSessionStoreTests { isTokenValid = 1, loginType = null, passphrase = "aPassphrase", - needsVerification = 1L, ) val secondSessionData = SessionData( userId = "userIdUnknown", @@ -210,7 +206,6 @@ class DatabaseSessionStoreTests { isTokenValid = 1, loginType = null, passphrase = "aPassphraseAltered", - needsVerification = 0L, ) assertThat(firstSessionData.userId).isNotEqualTo(secondSessionData.userId) @@ -229,6 +224,5 @@ 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) } } diff --git a/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/Fixtures.kt b/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/Fixtures.kt index 1397e260b9..341e5e0e92 100644 --- a/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/Fixtures.kt +++ b/libraries/session-storage/impl/src/test/kotlin/io/element/android/libraries/sessionstorage/impl/Fixtures.kt @@ -31,5 +31,4 @@ internal fun aSessionData() = SessionData( isTokenValid = 1, loginType = LoginType.UNKNOWN.name, passphrase = null, - needsVerification = 0L, )