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 63f7defb07
commit 1045f99d18
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
44 changed files with 386 additions and 123 deletions

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,
)