From 2be46edc176996ed1accc9b0011dbda128a5c6d3 Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 9 Sep 2024 10:30:17 +0200 Subject: [PATCH 1/2] Self verification : makes sure only one controller is created --- .../RustSessionVerificationService.kt | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) 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 d5f52cc8dc..c4d75e3050 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 @@ -26,6 +26,8 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withTimeout import org.matrix.rustcomponents.sdk.Client import org.matrix.rustcomponents.sdk.Encryption @@ -99,10 +101,7 @@ class RustSessionVerificationService( } override suspend fun requestVerification() = tryOrFail { - if (!this::verificationController.isInitialized) { - verificationController = client.getSessionVerificationController() - verificationController.setDelegate(this) - } + initVerificationControllerIfNeeded() verificationController.requestVerification() } @@ -193,6 +192,16 @@ class RustSessionVerificationService( } } + private var initControllerMutex = Mutex() + + private suspend fun initVerificationControllerIfNeeded() = initControllerMutex.withLock { + if (!this::verificationController.isInitialized) { + tryOrFail { + verificationController = client.getSessionVerificationController() + verificationController.setDelegate(this) + } + } + } private suspend fun updateVerificationStatus() { if (verificationFlowState.value == VerificationFlowState.Finished) { // Calling `encryptionService.verificationState()` performs a network call and it will deadlock if there is no network @@ -212,10 +221,7 @@ class RustSessionVerificationService( // Otherwise, just check the current verification status from the session verification controller instead Timber.d("Updating verification status: flow is pending or was finished some time ago") runCatching { - if (!this@RustSessionVerificationService::verificationController.isInitialized) { - verificationController = client.getSessionVerificationController() - verificationController.setDelegate(this@RustSessionVerificationService) - } + initVerificationControllerIfNeeded() _sessionVerifiedStatus.value = if (verificationController.isVerified()) { SessionVerifiedStatus.Verified } else { From 82a31d3ede29bac0d9685f6cbf03a851e7c767b5 Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 9 Sep 2024 10:30:35 +0200 Subject: [PATCH 2/2] Self verification : makes sure cancellation works properly --- .../impl/VerifySelfSessionView.kt | 38 +++++++++---------- .../RustSessionVerificationService.kt | 8 +++- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionView.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionView.kt index c9ec53b376..6616b075db 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionView.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionView.kt @@ -70,8 +70,17 @@ fun VerifySelfSessionView( onSuccessLogout: (String?) -> Unit, modifier: Modifier = Modifier, ) { - fun resetFlow() { - state.eventSink(VerifySelfSessionViewEvents.Reset) + fun cancelOrResetFlow() { + when (state.verificationFlowStep) { + is FlowStep.Canceled -> state.eventSink(VerifySelfSessionViewEvents.Reset) + is FlowStep.AwaitingOtherDeviceResponse, FlowStep.Ready -> state.eventSink(VerifySelfSessionViewEvents.Cancel) + is FlowStep.Verifying -> { + if (!state.verificationFlowStep.state.isLoading()) { + state.eventSink(VerifySelfSessionViewEvents.DeclineVerification) + } + } + else -> Unit + } } val latestOnFinish by rememberUpdatedState(newValue = onFinish) @@ -81,16 +90,7 @@ fun VerifySelfSessionView( } } BackHandler { - when (state.verificationFlowStep) { - is FlowStep.Canceled -> resetFlow() - is FlowStep.AwaitingOtherDeviceResponse, FlowStep.Ready -> state.eventSink(VerifySelfSessionViewEvents.Cancel) - is FlowStep.Verifying -> { - if (!state.verificationFlowStep.state.isLoading()) { - state.eventSink(VerifySelfSessionViewEvents.DeclineVerification) - } - } - else -> Unit - } + cancelOrResetFlow() } val verificationFlowStep = state.verificationFlowStep @@ -133,9 +133,9 @@ fun VerifySelfSessionView( footer = { BottomMenu( screenState = state, - goBack = ::resetFlow, + onCancelClick = ::cancelOrResetFlow, onEnterRecoveryKey = onEnterRecoveryKey, - onFinish = onFinish, + onContinueClick = onFinish, onResetKey = onResetKey, ) } @@ -268,8 +268,8 @@ private fun BottomMenu( screenState: VerifySelfSessionState, onEnterRecoveryKey: () -> Unit, onResetKey: () -> Unit, - goBack: () -> Unit, - onFinish: () -> Unit, + onCancelClick: () -> Unit, + onContinueClick: () -> Unit, ) { val verificationViewState = screenState.verificationFlowStep val eventSink = screenState.eventSink @@ -316,7 +316,7 @@ private fun BottomMenu( TextButton( modifier = Modifier.fillMaxWidth(), text = stringResource(CommonStrings.action_cancel), - onClick = goBack, + onClick = onCancelClick, ) } } @@ -330,7 +330,7 @@ private fun BottomMenu( TextButton( modifier = Modifier.fillMaxWidth(), text = stringResource(CommonStrings.action_cancel), - onClick = goBack, + onClick = onCancelClick, ) } } @@ -375,7 +375,7 @@ private fun BottomMenu( Button( modifier = Modifier.fillMaxWidth(), text = stringResource(CommonStrings.action_continue), - onClick = onFinish, + onClick = onContinueClick, ) // Placeholder so the 1st button keeps its vertical position Spacer(modifier = Modifier.height(48.dp)) 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 c4d75e3050..9d55d3402c 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 @@ -97,7 +97,7 @@ class RustSessionVerificationService( updateVerificationStatus() } } - .launchIn(sessionCoroutineScope) + .launchIn(sessionCoroutineScope) } override suspend fun requestVerification() = tryOrFail { @@ -105,7 +105,11 @@ class RustSessionVerificationService( verificationController.requestVerification() } - override suspend fun cancelVerification() = tryOrFail { verificationController.cancelVerification() } + override suspend fun cancelVerification() = tryOrFail { + verificationController.cancelVerification() + // We need to manually set the state to canceled, as the Rust SDK doesn't always call `didCancel` when it should + didCancel() + } override suspend fun approveVerification() = tryOrFail { verificationController.approveVerification() }