diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index 1477a77a8d..df159edb23 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -2517,6 +2517,7 @@ Verification has been canceled. You can start verification again. + The verification request was not found. It may have been cancelled, or handled by another session. This QR code looks malformed. Please try to verify with another method. Verification Canceled diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationActor.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationActor.kt index 66a54bfdc5..a6f5a6e302 100644 --- a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationActor.kt +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationActor.kt @@ -1663,16 +1663,22 @@ internal class VerificationActor @AssistedInject constructor( dispatchUpdate(VerificationEvent.TransactionUpdated(it)) } - cancelRequest(request.requestId, request.roomId, request.otherUserId, request.otherDeviceId(), code) + cancelRequest( + request.requestId, + request.roomId, + request.otherUserId, + request.otherDeviceId()?.let { listOf(it) } ?: request.targetDevices ?: emptyList(), + code + ) } - private suspend fun cancelRequest(transactionId: String, roomId: String?, otherUserId: String?, otherDeviceId: String?, code: CancelCode) { + private suspend fun cancelRequest(transactionId: String, roomId: String?, otherUserId: String?, otherDeviceIds: List, code: CancelCode) { try { if (roomId == null) { cancelTransactionToDevice( transactionId, otherUserId.orEmpty(), - otherDeviceId.orEmpty(), + otherDeviceIds, code ) } else { @@ -1688,7 +1694,7 @@ internal class VerificationActor @AssistedInject constructor( } } - private suspend fun cancelTransactionToDevice(transactionId: String, otherUserId: String, otherUserDeviceId: String?, code: CancelCode) { + private suspend fun cancelTransactionToDevice(transactionId: String, otherUserId: String, otherUserDeviceIds: List, code: CancelCode) { Timber.d("## SAS canceling transaction $transactionId for reason $code") val cancelMessage = KeyVerificationCancel.create(transactionId, code) // val contentMap = MXUsersDevicesMap() @@ -1697,7 +1703,7 @@ internal class VerificationActor @AssistedInject constructor( messageType = EventType.KEY_VERIFICATION_CANCEL, toSendToDeviceObject = cancelMessage, otherUserId = otherUserId, - targetDevices = otherUserDeviceId?.let { listOf(it) } ?: emptyList() + targetDevices = otherUserDeviceIds ) // sendToDeviceTask // .execute(SendToDeviceTask.Params(EventType.KEY_VERIFICATION_CANCEL, contentMap)) diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationTransportLayer.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationTransportLayer.kt index 275ad752b8..b1648a1e71 100644 --- a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationTransportLayer.kt +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationTransportLayer.kt @@ -93,7 +93,7 @@ internal class VerificationTransportLayer @Inject constructor( } suspend fun sendToDeviceEvent(messageType: String, toSendToDeviceObject: SendToDeviceObject, otherUserId: String, targetDevices: List) { - // TODO currently to device verification messages are sent unencrypted + // currently to device verification messages are sent unencrypted // as per spec not recommended // > verification messages may be sent unencrypted, though this is not encouraged. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt index d44a15b1b7..f26214ba34 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationListenersHolder.kt @@ -44,15 +44,15 @@ internal class VerificationListenersHolder @Inject constructor( } fun dispatchTxUpdated(tx: VerificationTransaction) { - Timber.v("## SAS dispatchTxUpdated txId:${tx.transactionId} $tx") scope.launch { + Timber.v("## SAS dispatchTxUpdated txId:${tx.transactionId} $tx") eventFlow.emit(VerificationEvent.TransactionUpdated(tx)) } } fun dispatchRequestAdded(verificationRequest: PendingVerificationRequest) { - Timber.v("## SAS dispatchRequestAdded txId:${verificationRequest.transactionId} $verificationRequest") scope.launch { + Timber.v("## SAS dispatchRequestAdded txId:${verificationRequest.transactionId} $verificationRequest") eventFlow.emit(VerificationEvent.RequestAdded(verificationRequest)) } } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt index 8d99cdad48..a66ff3283f 100755 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt @@ -588,7 +588,7 @@ internal class RustCryptoService @Inject constructor( // Notify the our listeners about room keys so decryption is retried. toDeviceEvents.events.orEmpty().forEach { event -> - Timber.tag(loggerTag.value).d("Processed ToDevice event msgid:${event.toDeviceTracingId()}") + Timber.tag(loggerTag.value).d("Processed ToDevice event msgid:${event.toDeviceTracingId()} id:${event.eventId} type:${event.type}") if (event.getClearType() == EventType.ENCRYPTED) { // rust failed to decrypt it diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/SasVerification.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/SasVerification.kt index 2a20bc090c..8ac566a99c 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/SasVerification.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/SasVerification.kt @@ -29,7 +29,6 @@ import org.matrix.android.sdk.api.session.crypto.verification.SasTransactionStat import org.matrix.android.sdk.api.session.crypto.verification.SasVerificationTransaction import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod import org.matrix.android.sdk.api.session.crypto.verification.safeValueOf -import org.matrix.android.sdk.internal.crypto.OlmMachine import org.matrix.android.sdk.internal.crypto.network.RequestSender import org.matrix.rustcomponents.sdk.crypto.CryptoStoreException import org.matrix.rustcomponents.sdk.crypto.Sas @@ -39,7 +38,7 @@ import org.matrix.rustcomponents.sdk.crypto.SasState /** Class representing a short auth string verification flow */ internal class SasVerification @AssistedInject constructor( @Assisted private var inner: Sas, - private val olmMachine: OlmMachine, +// private val olmMachine: OlmMachine, private val sender: RequestSender, private val coroutineDispatchers: MatrixCoroutineDispatchers, private val verificationListenersHolder: VerificationListenersHolder, @@ -254,6 +253,7 @@ internal class SasVerification @AssistedInject constructor( override fun onChange(state: SasState) { innerState = state + verificationListenersHolder.dispatchTxUpdated(this) } override fun toString(): String { diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt index 133e7f6524..d46a9f5a38 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationRequest.kt @@ -23,6 +23,7 @@ import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.withContext import org.matrix.android.sdk.api.MatrixCoroutineDispatchers import org.matrix.android.sdk.api.extensions.tryOrNull +import org.matrix.android.sdk.api.session.crypto.verification.CancelCode import org.matrix.android.sdk.api.session.crypto.verification.EVerificationState import org.matrix.android.sdk.api.session.crypto.verification.PendingVerificationRequest import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod @@ -260,7 +261,11 @@ internal class VerificationRequest @AssistedInject constructor( private fun state(): EVerificationState { if (innerVerificationRequest.isCancelled()) { - return EVerificationState.Cancelled + return if (innerVerificationRequest.cancelInfo()?.cancelCode == CancelCode.AcceptedByAnotherDevice.value) { + EVerificationState.HandledByOtherSession + } else { + EVerificationState.Cancelled + } } if (innerVerificationRequest.isPassive()) { return EVerificationState.HandledByOtherSession diff --git a/vector-app/src/androidTest/java/im/vector/app/VerificationTestBase.kt b/vector-app/src/androidTest/java/im/vector/app/VerificationTestBase.kt index bfb9a8401f..a5f0a7ab53 100644 --- a/vector-app/src/androidTest/java/im/vector/app/VerificationTestBase.kt +++ b/vector-app/src/androidTest/java/im/vector/app/VerificationTestBase.kt @@ -17,21 +17,36 @@ package im.vector.app import android.net.Uri +import android.view.View import androidx.lifecycle.Observer -import im.vector.app.ui.robot.ElementRobot +import androidx.test.espresso.Espresso +import androidx.test.espresso.assertion.ViewAssertions +import androidx.test.espresso.matcher.ViewMatchers +import im.vector.app.espresso.tools.waitUntilActivityVisible +import im.vector.app.espresso.tools.waitUntilViewVisible +import im.vector.app.features.home.HomeActivity +import im.vector.app.ui.robot.AnalyticsRobot import im.vector.app.ui.robot.OnboardingRobot +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout +import org.hamcrest.CoreMatchers import org.junit.Assert import org.matrix.android.sdk.api.Matrix import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig import org.matrix.android.sdk.api.auth.registration.RegistrationResult +import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.crypto.verification.PendingVerificationRequest +import org.matrix.android.sdk.api.session.crypto.verification.getRequest import org.matrix.android.sdk.api.session.sync.SyncState import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit @@ -42,7 +57,8 @@ abstract class VerificationTestBase { val homeServerUrl: String = "http://10.0.2.2:8080" protected val uiTestBase = OnboardingRobot() - protected val elementRobot = ElementRobot() + + protected val testScope = CoroutineScope(SupervisorJob()) fun createAccountAndSync( matrix: Matrix, @@ -136,4 +152,67 @@ abstract class VerificationTestBase { lock.await(20_000, TimeUnit.MILLISECONDS) } + + protected fun loginAndClickVerifyToast(userId: String): Session { + uiTestBase.login(userId = userId, password = password, homeServerUrl = homeServerUrl) + + tryOrNull { + val analyticsRobot = AnalyticsRobot() + analyticsRobot.optOut() + } + + waitUntilActivityVisible { + waitUntilViewVisible(ViewMatchers.withId(R.id.roomListContainer)) + } + val activity = EspressoHelper.getCurrentActivity()!! + val uiSession = (activity as HomeActivity).activeSessionHolder.getActiveSession() + withIdlingResource(initialSyncIdlingResource(uiSession)) { + waitUntilViewVisible(ViewMatchers.withId(R.id.roomListContainer)) + } + + // THIS IS THE ONLY WAY I FOUND TO CLICK ON ALERTERS... :( + // Cannot wait for view because of alerter animation? ... + Espresso.onView(ViewMatchers.isRoot()) + .perform(waitForView(ViewMatchers.withId(com.tapadoo.alerter.R.id.llAlertBackground))) + + Thread.sleep(1000) + val popup = activity.findViewById(com.tapadoo.alerter.R.id.llAlertBackground) + activity.runOnUiThread { + popup.performClick() + } + + Espresso.onView(ViewMatchers.isRoot()) + .perform(waitForView(ViewMatchers.withId(R.id.bottomSheetFragmentContainer))) + + Espresso.onView(ViewMatchers.withText(R.string.verification_verify_identity)) + .check(ViewAssertions.matches(ViewMatchers.isDisplayed())) + + // 4S is not setup so passphrase option should be hidden + Espresso.onView(ViewMatchers.withId(R.id.bottomSheetVerificationRecyclerView)) + .check(ViewAssertions.matches(CoreMatchers.not(ViewMatchers.hasDescendant(ViewMatchers.withText(R.string.verification_cannot_access_other_session))))) + + Espresso.onView(ViewMatchers.withId(R.id.bottomSheetVerificationRecyclerView)) + .check(ViewAssertions.matches(ViewMatchers.hasDescendant(ViewMatchers.withText(R.string.verification_verify_with_another_device)))) + + Espresso.onView(ViewMatchers.withId(R.id.bottomSheetVerificationRecyclerView)) + .check(ViewAssertions.matches(ViewMatchers.hasDescendant(ViewMatchers.withText(R.string.bad_passphrase_key_reset_all_action)))) + + return uiSession + } + + protected fun deferredRequestUntil(session: Session, block: ((PendingVerificationRequest) -> Boolean)): CompletableDeferred { + val completableDeferred = CompletableDeferred() + + testScope.launch { + session.cryptoService().verificationService().requestEventFlow().collect { + val request = it.getRequest() + if (request != null && block(request)) { + completableDeferred.complete(request) + return@collect cancel() + } + } + } + + return completableDeferred + } } diff --git a/vector-app/src/androidTest/java/im/vector/app/VerifySessionInteractiveTest.kt b/vector-app/src/androidTest/java/im/vector/app/VerifySessionInteractiveTest.kt index 773ae489c0..d726cbe4ff 100644 --- a/vector-app/src/androidTest/java/im/vector/app/VerifySessionInteractiveTest.kt +++ b/vector-app/src/androidTest/java/im/vector/app/VerifySessionInteractiveTest.kt @@ -16,38 +16,24 @@ package im.vector.app -import android.view.View +import androidx.preference.PreferenceManager import androidx.recyclerview.widget.RecyclerView import androidx.test.espresso.Espresso.onView -import androidx.test.espresso.IdlingResource import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.contrib.RecyclerViewActions.actionOnItem import androidx.test.espresso.matcher.ViewMatchers.hasDescendant -import androidx.test.espresso.matcher.ViewMatchers.isDisplayed -import androidx.test.espresso.matcher.ViewMatchers.isRoot import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.rules.ActivityScenarioRule import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.LargeTest import im.vector.app.core.utils.getMatrixInstance -import im.vector.app.espresso.tools.waitUntilActivityVisible -import im.vector.app.espresso.tools.waitUntilViewVisible import im.vector.app.features.MainActivity -import im.vector.app.features.home.HomeActivity -import im.vector.app.ui.robot.AnalyticsRobot import im.vector.app.ui.robot.ElementRobot -import kotlinx.coroutines.CompletableDeferred -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel -import kotlinx.coroutines.flow.filter -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runTest import org.amshove.kluent.internal.assertEquals -import org.hamcrest.CoreMatchers.not +import org.junit.After import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule @@ -58,12 +44,8 @@ import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor import org.matrix.android.sdk.api.auth.UserPasswordAuth import org.matrix.android.sdk.api.auth.registration.RegistrationFlowResponse import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.session.crypto.verification.PendingVerificationRequest -import org.matrix.android.sdk.api.session.crypto.verification.SasTransactionState import org.matrix.android.sdk.api.session.crypto.verification.SasVerificationTransaction import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod -import org.matrix.android.sdk.api.session.crypto.verification.getRequest -import org.matrix.android.sdk.api.session.crypto.verification.getTransaction import kotlin.coroutines.Continuation import kotlin.coroutines.resume import kotlin.random.Random @@ -77,8 +59,6 @@ class VerifySessionInteractiveTest : VerificationTestBase() { @get:Rule val activityRule = ActivityScenarioRule(MainActivity::class.java) - private val testScope = CoroutineScope(SupervisorJob()) - @Before fun createSessionWithCrossSigning() { val matrix = getMatrixInstance() @@ -102,60 +82,33 @@ class VerifySessionInteractiveTest : VerificationTestBase() { } } + @After + fun cleanUp() { + runTest { + existingSession?.signOutService()?.signOut(true) + } + val app = EspressoHelper.getCurrentActivity()!!.application as VectorApplication + while (app.authenticationService.getLastAuthenticatedSession() != null) { + val session = app.authenticationService.getLastAuthenticatedSession()!! + runTest { + session.signOutService().signOut(true) + } + } + + val activity = EspressoHelper.getCurrentActivity()!! + val editor = PreferenceManager.getDefaultSharedPreferences(activity).edit() + editor.clear() + editor.commit() + } + @Test fun checkVerifyPopup() { val userId: String = existingSession!!.myUserId - uiTestBase.login(userId = userId, password = password, homeServerUrl = homeServerUrl) + val uiSession = loginAndClickVerifyToast(userId) - val analyticsRobot = AnalyticsRobot() - analyticsRobot.optOut() - - waitUntilActivityVisible { - waitUntilViewVisible(withId(R.id.roomListContainer)) - } - val activity = EspressoHelper.getCurrentActivity()!! - val uiSession = (activity as HomeActivity).activeSessionHolder.getActiveSession() - withIdlingResource(initialSyncIdlingResource(uiSession)) { - waitUntilViewVisible(withId(R.id.roomListContainer)) - } - - // THIS IS THE ONLY WAY I FOUND TO CLICK ON ALERTERS... :( - // Cannot wait for view because of alerter animation? ... - onView(isRoot()) - .perform(waitForView(withId(com.tapadoo.alerter.R.id.llAlertBackground))) - - Thread.sleep(1000) - val popup = activity.findViewById(com.tapadoo.alerter.R.id.llAlertBackground) - activity.runOnUiThread { - popup.performClick() - } - - onView(isRoot()) - .perform(waitForView(withId(R.id.bottomSheetFragmentContainer))) - - onView(withText(R.string.verification_verify_identity)) - .check(matches(isDisplayed())) - - // 4S is not setup so passphrase option should be hidden - onView(withId(R.id.bottomSheetVerificationRecyclerView)) - .check(matches(not(hasDescendant(withText(R.string.verification_cannot_access_other_session))))) - - onView(withId(R.id.bottomSheetVerificationRecyclerView)) - .check(matches(hasDescendant(withText(R.string.verification_verify_with_another_device)))) - - onView(withId(R.id.bottomSheetVerificationRecyclerView)) - .check(matches(hasDescendant(withText(R.string.bad_passphrase_key_reset_all_action)))) - - val otherRequest = CompletableDeferred() - - testScope.launch { - existingSession!!.cryptoService().verificationService().requestEventFlow().collect { - if (it.getRequest() != null) { - otherRequest.complete(it.getRequest()!!) - return@collect cancel() - } - } + val otherRequest = deferredRequestUntil(existingSession!!) { + true } // Send out a self verification request @@ -261,55 +214,4 @@ class VerifySessionInteractiveTest : VerificationTestBase() { ElementRobot().signout(false) } - - fun signout() { - onView(withId(R.id.groupToolbarAvatarImageView)) - .perform(click()) - - onView(withId(R.id.homeDrawerHeaderSettingsView)) - .perform(click()) - - onView(withText("General")) - .perform(click()) - } - - fun verificationStateIdleResource(transactionId: String, checkForState: SasTransactionState, session: Session): IdlingResource { - val scope = CoroutineScope(SupervisorJob()) - - val idle = object : IdlingResource { - private var callback: IdlingResource.ResourceCallback? = null - - private var currentState: SasTransactionState? = null - - override fun getName() = "verificationSuccessIdle" - - override fun isIdleNow(): Boolean { - return currentState == checkForState - } - - override fun registerIdleTransitionCallback(callback: IdlingResource.ResourceCallback?) { - this.callback = callback - } - - fun update(state: SasTransactionState) { - currentState = state - if (state == checkForState) { - callback?.onTransitionToIdle() - scope.cancel() - } - } - } - - session.cryptoService().verificationService() - .requestEventFlow() - .filter { - it.transactionId == transactionId - } - .onEach { - (it.getTransaction() as? SasVerificationTransaction)?.state()?.let { - idle.update(it) - } - }.launchIn(scope) - return idle - } } diff --git a/vector-app/src/androidTest/java/im/vector/app/VerifySessionNavigationTest.kt b/vector-app/src/androidTest/java/im/vector/app/VerifySessionNavigationTest.kt new file mode 100644 index 0000000000..a2a59e536e --- /dev/null +++ b/vector-app/src/androidTest/java/im/vector/app/VerifySessionNavigationTest.kt @@ -0,0 +1,136 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app + +import androidx.recyclerview.widget.RecyclerView +import androidx.test.espresso.Espresso +import androidx.test.espresso.action.ViewActions +import androidx.test.espresso.assertion.ViewAssertions +import androidx.test.espresso.contrib.RecyclerViewActions +import androidx.test.espresso.matcher.ViewMatchers +import androidx.test.ext.junit.rules.ActivityScenarioRule +import im.vector.app.core.utils.getMatrixInstance +import im.vector.app.espresso.tools.waitUntilActivityVisible +import im.vector.app.espresso.tools.waitUntilViewVisible +import im.vector.app.features.MainActivity +import im.vector.app.features.home.HomeActivity +import im.vector.app.ui.robot.ElementRobot +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.matrix.android.sdk.api.auth.UIABaseAuth +import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor +import org.matrix.android.sdk.api.auth.UserPasswordAuth +import org.matrix.android.sdk.api.auth.registration.RegistrationFlowResponse +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.crypto.verification.EVerificationState +import kotlin.coroutines.Continuation +import kotlin.coroutines.resume +import kotlin.random.Random + +class VerifySessionNavigationTest : VerificationTestBase() { + + var existingSession: Session? = null + + @get:Rule + val activityRule = ActivityScenarioRule(MainActivity::class.java) + + @Before + fun createSessionWithCrossSigning() { + val matrix = getMatrixInstance() + val userName = "foobar_${Random.nextLong()}" + existingSession = createAccountAndSync(matrix, userName, password, true) + runTest { + existingSession!!.cryptoService().crossSigningService() + .initializeCrossSigning( + object : UserInteractiveAuthInterceptor { + override fun performStage(flowResponse: RegistrationFlowResponse, errCode: String?, promise: Continuation) { + promise.resume( + UserPasswordAuth( + user = existingSession!!.myUserId, + password = "password", + session = flowResponse.session + ) + ) + } + } + ) + } + } + + @After + fun cleanUp() { + runTest { + existingSession?.signOutService()?.signOut(true) + } + } + + @Test + fun testStartThenCancelRequest() { + val userId: String = existingSession!!.myUserId + + loginAndClickVerifyToast(userId) + + val otherRequest = deferredRequestUntil(existingSession!!) { + true + } + + // Send out a self verification request + Espresso.onView(ViewMatchers.withId(R.id.bottomSheetVerificationRecyclerView)) + .perform( + RecyclerViewActions.actionOnItem( + ViewMatchers.hasDescendant(ViewMatchers.withText(R.string.verification_verify_with_another_device)), + ViewActions.click() + ) + ) + + Espresso.onView(ViewMatchers.withId(R.id.bottomSheetVerificationRecyclerView)) + .check(ViewAssertions.matches(ViewMatchers.hasDescendant(ViewMatchers.withText(R.string.verification_request_was_sent)))) + + val txId = runBlockingTest { + otherRequest.await().transactionId + } + + // if we press back it should cancel + + val otherGetCancelledRequest = deferredRequestUntil(existingSession!!) { + it.transactionId == txId && it.state == EVerificationState.Cancelled + } + + Espresso.pressBack() + + // Should go back to main verification options + Espresso.onView(ViewMatchers.isRoot()) + .perform(waitForView(ViewMatchers.withId(R.id.bottomSheetFragmentContainer))) + + Espresso.onView(ViewMatchers.withId(R.id.bottomSheetVerificationRecyclerView)) + .check(ViewAssertions.matches(ViewMatchers.hasDescendant(ViewMatchers.withText(R.string.verification_verify_with_another_device)))) + + runBlockingTest { + otherGetCancelledRequest.await() + } + + Espresso.pressBack() + waitUntilActivityVisible { + waitUntilViewVisible(ViewMatchers.withId(R.id.roomListContainer)) + } + + ElementRobot().signout(false) + } +} diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt index 121592f5be..b450c6ea81 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt @@ -224,6 +224,7 @@ class IncomingVerificationRequestHandler @Inject constructor( override fun verificationRequestUpdated(pr: PendingVerificationRequest) { // If an incoming request is readied (by another device?) we should discard the alert if (pr.isIncoming && (pr.state == EVerificationState.HandledByOtherSession || + pr.state == EVerificationState.Cancelled || pr.state == EVerificationState.Started || pr.state == EVerificationState.WeStarted)) { popupAlertManager.cancelAlert(uniqueIdForVerificationRequest(pr)) diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationAction.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationAction.kt index cc99a94000..617e31d8fa 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationAction.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationAction.kt @@ -38,4 +38,5 @@ sealed class VerificationAction : VectorViewModelAction { data class GotResultFromSsss(val cypherData: String, val alias: String) : VerificationAction() object CancelledFromSsss : VerificationAction() object SecuredStorageHasBeenReset : VerificationAction() + object SelfVerificationWasNotMe : VerificationAction() } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewEvents.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewEvents.kt index 82fe94cbdb..e52fb0e2cd 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewEvents.kt @@ -23,8 +23,10 @@ import im.vector.app.core.platform.VectorViewEvents */ sealed class VerificationBottomSheetViewEvents : VectorViewEvents { object Dismiss : VerificationBottomSheetViewEvents() + object DismissAndOpenDeviceSettings : VerificationBottomSheetViewEvents() object AccessSecretStore : VerificationBottomSheetViewEvents() object ResetAll : VerificationBottomSheetViewEvents() object GoToSettings : VerificationBottomSheetViewEvents() data class ModalError(val errorMessage: CharSequence) : VerificationBottomSheetViewEvents() + data class RequestNotFound(val transactionId: String) : VerificationBottomSheetViewEvents() } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt index 6628c39248..67dc115827 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt @@ -17,8 +17,10 @@ package im.vector.app.features.crypto.verification.self import android.app.Activity +import android.app.Dialog import android.os.Bundle import android.os.Parcelable +import android.view.KeyEvent import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -30,6 +32,7 @@ import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R import im.vector.app.core.extensions.commitTransaction import im.vector.app.core.extensions.registerStartForActivityResult +import im.vector.app.core.extensions.singletonEntryPoint import im.vector.app.core.extensions.toMvRxBundle import im.vector.app.core.platform.VectorBaseBottomSheetDialogFragment import im.vector.app.databinding.BottomSheetVerificationBinding @@ -37,6 +40,7 @@ import im.vector.app.features.crypto.quads.SharedSecureStorageActivity import im.vector.app.features.crypto.quads.SharedSecureStorageViewState import im.vector.app.features.crypto.verification.VerificationAction import im.vector.app.features.crypto.verification.VerificationBottomSheetViewEvents +import im.vector.app.features.settings.VectorSettingsActivity import kotlinx.parcelize.Parcelize import org.matrix.android.sdk.api.session.crypto.crosssigning.KEYBACKUP_SECRET_SSSS_NAME import org.matrix.android.sdk.api.session.crypto.crosssigning.MASTER_KEY_SSSS_NAME @@ -58,6 +62,11 @@ class SelfVerificationBottomSheet : VectorBaseBottomSheetDialogFragment + if (keyCode == KeyEvent.KEYCODE_BACK && keyEvent.action == KeyEvent.ACTION_UP) { + viewModel.queryCancel() + true + } else { + false + } + } + } + } + + private fun observeViewModelEvents() { viewModel.observeViewEvents { event -> when (event) { VerificationBottomSheetViewEvents.AccessSecretStore -> { @@ -122,6 +148,16 @@ class SelfVerificationBottomSheet : VectorBaseBottomSheetDialogFragment { + dismiss() + requireActivity().singletonEntryPoint().navigator().openSettings( + requireActivity(), + VectorSettingsActivity.EXTRA_DIRECT_ACCESS_SECURITY_PRIVACY_MANAGE_SESSIONS + ) + } + is VerificationBottomSheetViewEvents.RequestNotFound -> { + dismiss() + } } } } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt index cf67c8861e..57e10c7fc4 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationController.kt @@ -58,8 +58,19 @@ class SelfVerificationController @Inject constructor( fun onClickResetSecurity() fun onDoneFrom4S() fun keysNotIn4S() + fun confirmCancelRequest(confirm: Boolean) + + /** + * An incoming request, when I am the verified request + */ + fun wasNotMe() } + private val selfVerificationListener: InteractionListener? + get() { + return listener as? InteractionListener + } + var state: SelfVerificationViewState? = null fun update(state: SelfVerificationViewState) { @@ -70,6 +81,18 @@ class SelfVerificationController @Inject constructor( override fun buildModels() { val state = this.state ?: return + + // actions have priority + if (state.activeAction !is UserAction.None) { + when (state.activeAction) { + UserAction.ConfirmCancel -> { + renderConfirmCancel(state) + } + UserAction.None -> {} + } + return + } + when (state.pendingRequest) { Uninitialized -> { renderBaseNoActiveRequest(state) @@ -80,6 +103,50 @@ class SelfVerificationController @Inject constructor( } } + private fun renderConfirmCancel(state: SelfVerificationViewState) { + val host = this + if (state.currentDeviceCanCrossSign) { + bottomSheetVerificationNoticeItem { + id("notice") + notice(host.stringProvider.getString(R.string.verify_cancel_self_verification_from_trusted).toEpoxyCharSequence()) + } + } else { + bottomSheetVerificationNoticeItem { + id("notice") + notice(host.stringProvider.getString(R.string.verify_cancel_self_verification_from_untrusted).toEpoxyCharSequence()) + } + } + bottomSheetDividerItem { + id("sep0") + } + + bottomSheetVerificationActionItem { + id("cancel") + title(host.stringProvider.getString(R.string.action_skip)) + titleColor(host.colorProvider.getColorFromAttribute(R.attr.colorError)) + iconRes(R.drawable.ic_arrow_right) + iconColor(host.colorProvider.getColorFromAttribute(R.attr.colorError)) + listener { +// host.listener?.confirmCancelRequest(true) + } + } + + bottomSheetDividerItem { + id("sep1") + } + + bottomSheetVerificationActionItem { + id("continue") + title(host.stringProvider.getString(R.string._continue)) + titleColor(host.colorProvider.getColorFromAttribute(R.attr.colorPrimary)) + iconRes(R.drawable.ic_arrow_right) + iconColor(host.colorProvider.getColorFromAttribute(R.attr.colorPrimary)) + listener { +// host.listener?.confirmCancelRequest(false) + } + } + } + private fun renderBaseNoActiveRequest(state: SelfVerificationViewState) { if (state.verifyingFrom4SAction !is Uninitialized) { render4SCheckState(state) @@ -142,15 +209,16 @@ class SelfVerificationController @Inject constructor( EVerificationState.Requested -> { // add accept buttons? renderAcceptDeclineRequest() - bottomSheetVerificationActionItem { - id("not me") - title(host.stringProvider.getString(R.string.verify_new_session_was_not_me)) - titleColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary)) - iconRes(R.drawable.ic_arrow_right) - iconColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary)) - listener { - TODO() -// host.listener?.wasNotMe() + if (state.isThisSessionVerified) { + bottomSheetVerificationActionItem { + id("not me") + title(host.stringProvider.getString(R.string.verify_new_session_was_not_me)) + titleColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary)) + iconRes(R.drawable.ic_arrow_right) + iconColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary)) + listener { + host.selfVerificationListener?.wasNotMe() + } } } } @@ -172,14 +240,27 @@ class SelfVerificationController @Inject constructor( } EVerificationState.Cancelled -> { renderCancel(pendingRequest.cancelConclusion ?: CancelCode.User) + gotIt { + listener?.onDone(false) + } } EVerificationState.HandledByOtherSession -> { // we should dismiss + renderCancel(pendingRequest.cancelConclusion ?: CancelCode.User) + gotIt { + listener?.onDone(false) + } } } } is Fail -> { - // TODO + bottomSheetVerificationNoticeItem { + id("request_fail") + notice(host.stringProvider.getString(R.string.verification_not_found).toEpoxyCharSequence()) + } + gotIt { + listener?.onDone(false) + } } } } @@ -202,7 +283,7 @@ class SelfVerificationController @Inject constructor( // subTitle(host.stringProvider.getString(R.string.verification_request_start_notice)) iconRes(R.drawable.ic_arrow_right) iconColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary)) - listener { (host.listener as? InteractionListener)?.onClickOnVerificationStart() } + listener { host.selfVerificationListener?.onClickOnVerificationStart() } } bottomSheetDividerItem { @@ -218,7 +299,7 @@ class SelfVerificationController @Inject constructor( subTitle(host.stringProvider.getString(R.string.verification_use_passphrase)) iconRes(R.drawable.ic_arrow_right) iconColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary)) - listener { (host.listener as? InteractionListener)?.onClickRecoverFromPassphrase() } + listener { host.selfVerificationListener?.onClickRecoverFromPassphrase() } } bottomSheetDividerItem { @@ -234,7 +315,7 @@ class SelfVerificationController @Inject constructor( subTitle(host.stringProvider.getString(R.string.secure_backup_reset_all_no_other_devices)) iconRes(R.drawable.ic_arrow_right) iconColor(host.colorProvider.getColorFromAttribute(R.attr.vctr_content_primary)) - listener { (host.listener as? InteractionListener)?.onClickResetSecurity() } + listener { host.selfVerificationListener?.onClickResetSecurity() } } if (!state.isVerificationRequired) { @@ -248,7 +329,7 @@ class SelfVerificationController @Inject constructor( titleColor(host.colorProvider.getColorFromAttribute(R.attr.colorError)) iconRes(R.drawable.ic_arrow_right) iconColor(host.colorProvider.getColorFromAttribute(R.attr.colorError)) - listener { (host.listener as? InteractionListener)?.onClickSkip() } + listener { host.selfVerificationListener?.onClickSkip() } } } } @@ -268,7 +349,7 @@ class SelfVerificationController @Inject constructor( val value = action.invoke() if (value) { verifiedSuccessTile() - bottomDone { (host.listener as? InteractionListener)?.onDoneFrom4S() } + bottomDone { host.selfVerificationListener?.onDoneFrom4S() } } else { bottomSheetVerificationNoticeItem { id("notice_4s_failed'") @@ -279,7 +360,7 @@ class SelfVerificationController @Inject constructor( .toEpoxyCharSequence() ) } - gotIt { (host.listener as? InteractionListener)?.keysNotIn4S() } + gotIt { host.selfVerificationListener?.keysNotIn4S() } } } else -> { diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationFragment.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationFragment.kt index aa87b98a7f..ae20955d45 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationFragment.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationFragment.kt @@ -92,6 +92,14 @@ class SelfVerificationFragment : VectorBaseFragment = Uninitialized, // need something immutable for state to work properly, VerificationTransaction is not val startedTransaction: Async = Uninitialized, @@ -110,9 +119,19 @@ class SelfVerificationViewModel @AssistedInject constructor( copy(pendingRequest = Loading()) } viewModelScope.launch { - session.cryptoService().verificationService().getExistingVerificationRequest(session.myUserId, initialState.transactionId)?.let { + val matchingRequest = session.cryptoService().verificationService().getExistingVerificationRequest(session.myUserId, initialState.transactionId) + if (matchingRequest == null) { + // it's unexpected for now dismiss. + // Could happen when you click on the popup for an incoming request + // that has been deleted by the time you clicked on it + // maybe give some feedback? setState { - copy(pendingRequest = Success(it)) + copy(pendingRequest = Fail(NotFoundException())) + } + _viewEvents.post(VerificationBottomSheetViewEvents.RequestNotFound(initialState.transactionId)) + } else { + setState { + copy(pendingRequest = Success(matchingRequest)) } } } @@ -204,8 +223,13 @@ class SelfVerificationViewModel @AssistedInject constructor( } } is VerificationAction.GotItConclusion -> { - // just dismiss - _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) + withState { state -> + if (action.verified || state.isThisSessionVerified) { + _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) + } else { + queryCancel() + } + } } is VerificationAction.GotResultFromSsss -> handleSecretBackFromSSSS(action) VerificationAction.OtherUserDidNotScanned -> { @@ -292,7 +316,7 @@ class SelfVerificationViewModel @AssistedInject constructor( } } VerificationAction.SkipVerification -> { - _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) + queryCancel() } VerificationAction.ForgotResetAll -> { _viewEvents.post(VerificationBottomSheetViewEvents.ResetAll) @@ -324,6 +348,87 @@ class SelfVerificationViewModel @AssistedInject constructor( VerificationAction.RequestVerificationByDM -> { // not applicable in self verification } + VerificationAction.SelfVerificationWasNotMe -> { + // cancel transaction then open settings + withState { state -> + val request = state.pendingRequest.invoke() ?: return@withState + session.coroutineScope.launch { + tryOrNull { + session.cryptoService().verificationService().cancelVerificationRequest(request) + } + } + _viewEvents.post(VerificationBottomSheetViewEvents.DismissAndOpenDeviceSettings) + } + } + } + } + + fun queryCancel() = withState { state -> + when (state.pendingRequest) { + is Uninitialized -> { + // No active request currently + if (state.isVerificationRequired) { + // we can't let you dismiss :) + } else { + // verification is not required so just dismiss + _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) + } + } + is Success -> { + val activeRequest = state.pendingRequest.invoke() + // there is an active request or transaction, we need confirmation to cancel it + if (activeRequest.state == EVerificationState.Cancelled) { + // equivalent of got it? + if (state.isThisSessionVerified) { + // we can always dismiss?? + _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) + } else { + // go back to main verification screen + setState { + copy(pendingRequest = Uninitialized) + } + } + return@withState + } else if (activeRequest.state == EVerificationState.Done) { + _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) + } else { + if (state.isThisSessionVerified) { + // we are the verified session, so just dismiss? + // do we want to prompt confirm?? + } else { + // cancel the active request and go back? + setState { + copy( + transactionId = null, + pendingRequest = Uninitialized, + ) + } + viewModelScope.launch(Dispatchers.IO) { + session.cryptoService().verificationService().cancelVerificationRequest( + activeRequest + ) + setState { + copy( + transactionId = null, + pendingRequest = Uninitialized + ) + } + } + } + } +// if (state.isThisSessionVerified) { +// // we can always dismiss?? +// _viewEvents.post(VerificationBottomSheetViewEvents.Dismiss) +// } +// setState { +// copy( +// activeAction = UserAction.ConfirmCancel +// ) +// } + } + else -> { + // just ignore? + } } } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationBottomSheet.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationBottomSheet.kt index 31faec1f4b..8efda06991 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationBottomSheet.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationBottomSheet.kt @@ -90,7 +90,11 @@ class UserVerificationBottomSheet : VectorBaseBottomSheetDialogFragment { + VerificationBottomSheetViewEvents.ResetAll, + VerificationBottomSheetViewEvents.DismissAndOpenDeviceSettings -> { + // no-op for user verification + } + is VerificationBottomSheetViewEvents.RequestNotFound -> { // no-op for user verification } } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationViewModel.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationViewModel.kt index 24ec5ae22e..8940d056cd 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/user/UserVerificationViewModel.kt @@ -381,10 +381,9 @@ class UserVerificationViewModel @AssistedInject constructor( VerificationAction.SkipVerification, VerificationAction.VerifyFromPassphrase, VerificationAction.SecuredStorageHasBeenReset, - VerificationAction.FailedToGetKeysFrom4S -> { - // Not applicable for user verification - } - VerificationAction.RequestSelfVerification -> TODO() + VerificationAction.FailedToGetKeysFrom4S, + VerificationAction.RequestSelfVerification, + VerificationAction.SelfVerificationWasNotMe, VerificationAction.ForgotResetAll -> { // Not applicable for user verification }