From c670fc9e9c07fcc69b9564e2f1254a5ceb63aa97 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Thu, 24 Aug 2023 13:43:36 +0200 Subject: [PATCH] Prevent verification while initial sync is in progress (#1138) * Prevent verification while initial sync is in progress * Add `canVerifySessionFlow` to simplify the check --- changelog.d/1131.bugfix | 1 + .../impl/root/PreferencesRootPresenter.kt | 11 +++-------- .../features/roomlist/impl/RoomListPresenter.kt | 5 ++--- .../roomlist/impl/RoomListPresenterTests.kt | 2 +- .../verification/SessionVerificationService.kt | 6 ++++++ .../libraries/matrix/impl/RustMatrixClient.kt | 17 +++++++---------- .../RustSessionVerificationService.kt | 11 ++++++++++- .../FakeSessionVerificationService.kt | 11 ++++++++--- 8 files changed, 38 insertions(+), 26 deletions(-) create mode 100644 changelog.d/1131.bugfix diff --git a/changelog.d/1131.bugfix b/changelog.d/1131.bugfix new file mode 100644 index 0000000000..23649a19d0 --- /dev/null +++ b/changelog.d/1131.bugfix @@ -0,0 +1 @@ +Only display verification prompt after initial sync is done. diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt index b33940b21e..af82a3ab2a 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt @@ -20,7 +20,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState 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 @@ -34,7 +33,6 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.api.user.getCurrentUser import io.element.android.libraries.matrix.api.verification.SessionVerificationService -import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.services.analytics.api.AnalyticsService import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -62,11 +60,8 @@ class PreferencesRootPresenter @Inject constructor( val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState() val hasAnalyticsProviders = remember { analyticsService.getAvailableAnalyticsProviders().isNotEmpty() } - // Session verification status (unknown, not verified, verified) - val sessionVerifiedStatus by sessionVerificationService.sessionVerifiedStatus.collectAsState() - val sessionIsNotVerified by remember { - derivedStateOf { sessionVerifiedStatus == SessionVerifiedStatus.NotVerified } - } + // We should display the 'complete verification' option if the current session can be verified + val showCompleteVerification by sessionVerificationService.canVerifySessionFlow.collectAsState(false) val accountManagementUrl: MutableState = remember { mutableStateOf(null) @@ -82,7 +77,7 @@ class PreferencesRootPresenter @Inject constructor( logoutState = logoutState, myUser = matrixUser.value, version = versionFormatter.get(), - showCompleteVerification = sessionIsNotVerified, + showCompleteVerification = showCompleteVerification, accountManagementUrl = accountManagementUrl.value, showAnalyticsSettings = hasAnalyticsProviders, showDeveloperSettings = showDeveloperSettings, diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt index dbef9c799b..e069634d49 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt @@ -39,7 +39,6 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.api.user.getCurrentUser import io.element.android.libraries.matrix.api.verification.SessionVerificationService -import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import javax.inject.Inject @@ -73,11 +72,11 @@ class RoomListPresenter @Inject constructor( } // Session verification status (unknown, not verified, verified) - val sessionVerifiedStatus by sessionVerificationService.sessionVerifiedStatus.collectAsState() + val canVerifySession by sessionVerificationService.canVerifySessionFlow.collectAsState(initial = false) var verificationPromptDismissed by rememberSaveable { mutableStateOf(false) } // We combine both values to only display the prompt if the session is not verified and it wasn't dismissed val displayVerificationPrompt by remember { - derivedStateOf { sessionVerifiedStatus == SessionVerifiedStatus.NotVerified && !verificationPromptDismissed } + derivedStateOf { canVerifySession && !verificationPromptDismissed } } var displaySearchResults by rememberSaveable { mutableStateOf(false) } diff --git a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt index eaa3801e12..d8524d5834 100644 --- a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt +++ b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt @@ -202,7 +202,7 @@ class RoomListPresenterTests { fun `present - handle DismissRequestVerificationPrompt`() = runTest { val roomListService = FakeRoomListService() val matrixClient = FakeMatrixClient( - roomListService = roomListService + roomListService = roomListService, ) val presenter = createRoomListPresenter( client = matrixClient, 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 b2f79c0750..4cb1918ce1 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 @@ -16,6 +16,7 @@ package io.element.android.libraries.matrix.api.verification +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow interface SessionVerificationService { @@ -37,6 +38,11 @@ interface SessionVerificationService { */ val sessionVerifiedStatus: StateFlow + /** + * Returns whether the current session needs to be verified and the SDK is ready to start the verification. + */ + val canVerifySessionFlow: Flow + /** * Request verification 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 c68913b705..4b0afb0b4f 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 @@ -35,7 +35,6 @@ import io.element.android.libraries.matrix.api.room.RoomMembershipObserver import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.roomlist.awaitLoaded import io.element.android.libraries.matrix.api.sync.SyncService -import io.element.android.libraries.matrix.api.sync.SyncState import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.api.verification.SessionVerificationService @@ -93,8 +92,8 @@ class RustMatrixClient constructor( private val innerRoomListService = syncService.roomListService() private val sessionDispatcher = dispatchers.io.limitedParallelism(64) private val sessionCoroutineScope = appCoroutineScope.childScope(dispatchers.main, "Session-${sessionId}") - private val verificationService = RustSessionVerificationService() private val rustSyncService = RustSyncService(syncService, sessionCoroutineScope) + private val verificationService = RustSessionVerificationService(rustSyncService) private val pushersService = RustPushersService( client = client, dispatchers = dispatchers, @@ -149,13 +148,11 @@ class RustMatrixClient constructor( init { client.setDelegate(clientDelegate) - rustSyncService.syncState - .onEach { syncState -> - if (syncState == SyncState.Running) { - onSlidingSyncUpdate() - } + roomListService.state.onEach { state -> + if (state == RoomListService.State.Running) { + setupVerificationControllerIfNeeded() } - .launchIn(sessionCoroutineScope) + }.launchIn(sessionCoroutineScope) } override suspend fun getRoom(roomId: RoomId): MatrixRoom? = withContext(sessionDispatcher) { @@ -338,8 +335,8 @@ class RustMatrixClient constructor( } } - private fun onSlidingSyncUpdate() { - if (!verificationService.isReady.value) { + private fun setupVerificationControllerIfNeeded() { + if (verificationService.verificationController == null) { try { verificationService.verificationController = client.getSessionVerificationController() } catch (e: Throwable) { 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 dc65bb74a6..29797ed3c4 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 @@ -17,20 +17,25 @@ package io.element.android.libraries.matrix.impl.verification import io.element.android.libraries.core.data.tryOrNull +import io.element.android.libraries.matrix.api.sync.SyncState 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.impl.sync.RustSyncService import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.combine import org.matrix.rustcomponents.sdk.SessionVerificationController import org.matrix.rustcomponents.sdk.SessionVerificationControllerDelegate import org.matrix.rustcomponents.sdk.SessionVerificationControllerInterface import org.matrix.rustcomponents.sdk.SessionVerificationEmoji import javax.inject.Inject -class RustSessionVerificationService @Inject constructor() : SessionVerificationService, SessionVerificationControllerDelegate { +class RustSessionVerificationService @Inject constructor( + private val syncService: RustSyncService, +) : SessionVerificationService, SessionVerificationControllerDelegate { var verificationController: SessionVerificationControllerInterface? = null set(value) { @@ -52,6 +57,10 @@ class RustSessionVerificationService @Inject constructor() : SessionVerification private val _sessionVerifiedStatus = MutableStateFlow(SessionVerifiedStatus.Unknown) override val sessionVerifiedStatus: StateFlow = _sessionVerifiedStatus.asStateFlow() + override val canVerifySessionFlow = combine(sessionVerifiedStatus, syncService.syncState) { verificationStatus, syncState -> + syncState == SyncState.Running && verificationStatus == SessionVerifiedStatus.NotVerified + } + override suspend fun requestVerification() = tryOrFail { verificationController?.requestVerification() } 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 2f7887c537..62b0b39adf 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,6 +20,7 @@ import io.element.android.libraries.matrix.api.verification.SessionVerificationS import io.element.android.libraries.matrix.api.verification.VerificationFlowState import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.matrix.api.verification.VerificationEmoji +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -27,13 +28,13 @@ 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) private var emojiList = emptyList() var shouldFail = false - override val verificationFlowState: StateFlow - get() = _verificationFlowState - + override val verificationFlowState: StateFlow =_verificationFlowState override val sessionVerifiedStatus: StateFlow = _sessionVerifiedStatus + override val canVerifySessionFlow: Flow = _canVerifySessionFlow override val isReady: StateFlow = _isReady @@ -77,6 +78,10 @@ class FakeSessionVerificationService : SessionVerificationService { _verificationFlowState.value = state } + fun givenCanVerifySession(canVerify: Boolean) { + _canVerifySessionFlow.value = canVerify + } + fun givenIsReady(value: Boolean) { _isReady.value = value }