From 457f7fffeecbcd403f817306264421a12fa2c4dd Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Aug 2022 09:50:43 +0100 Subject: [PATCH 1/5] promoting the accept certificate to an explict ViewEvent - allows a retryAction to be provided to the event to avoid mutatble state within the view model along with providing a clear path of execution --- .../android/sdk/api/failure/Extensions.kt | 2 + .../features/onboarding/OnboardingAction.kt | 2 +- .../onboarding/OnboardingViewEvents.kt | 2 + .../onboarding/OnboardingViewModel.kt | 42 +++++++++++-------- .../ftueauth/AbstractFtueAuthFragment.kt | 12 +++--- .../onboarding/ftueauth/FtueAuthVariant.kt | 1 + 6 files changed, 36 insertions(+), 25 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt index 68b931b33c..429d346a1b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt @@ -93,6 +93,8 @@ fun Throwable.isMissingEmailVerification() = this is Failure.ServerError && error.code == MatrixError.M_UNAUTHORIZED && error.message == "Unable to get validated threepid" +fun Throwable.isUnrecognisedCertificate() = this is Failure.UnrecognizedCertificateFailure + /** * Try to convert to a RegistrationFlowResponse. Return null in the cases it's not possible */ diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt index d07ac46b85..f1617b660b 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt @@ -82,7 +82,7 @@ sealed interface OnboardingAction : VectorViewModelAction { data class PostViewEvent(val viewEvent: OnboardingViewEvents) : OnboardingAction - data class UserAcceptCertificate(val fingerprint: Fingerprint) : OnboardingAction + data class UserAcceptCertificate(val fingerprint: Fingerprint, val retryAction: OnboardingAction) : OnboardingAction object PersonalizeProfile : OnboardingAction data class UpdateDisplayName(val displayName: String) : OnboardingAction diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt index bbbf13fba9..1441152128 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt @@ -21,6 +21,7 @@ import im.vector.app.core.platform.VectorViewEvents import im.vector.app.features.login.ServerType import im.vector.app.features.login.SignMode import org.matrix.android.sdk.api.auth.registration.Stage +import org.matrix.android.sdk.api.failure.Failure as SdkFailure /** * Transient events for Login. @@ -29,6 +30,7 @@ sealed class OnboardingViewEvents : VectorViewEvents { data class Loading(val message: CharSequence? = null) : OnboardingViewEvents() data class Failure(val throwable: Throwable) : OnboardingViewEvents() data class DeeplinkAuthenticationFailure(val retryAction: OnboardingAction) : OnboardingViewEvents() + data class UnrecognisedCertificateFailure(val retryAction: OnboardingAction, val cause: SdkFailure.UnrecognizedCertificateFailure) : OnboardingViewEvents() object DisplayRegistrationFallback : OnboardingViewEvents() data class DisplayRegistrationStage(val stage: Stage) : OnboardingViewEvents() diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 73288bd6d5..b0ba113d41 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -60,7 +60,9 @@ import org.matrix.android.sdk.api.auth.data.SsoIdentityProvider import org.matrix.android.sdk.api.auth.login.LoginWizard import org.matrix.android.sdk.api.auth.registration.RegistrationAvailability import org.matrix.android.sdk.api.auth.registration.RegistrationWizard +import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.failure.isHomeserverUnavailable +import org.matrix.android.sdk.api.failure.isUnrecognisedCertificate import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider import timber.log.Timber @@ -113,8 +115,6 @@ class OnboardingViewModel @AssistedInject constructor( } } - // Store the last action, to redo it after user has trusted the untrusted certificate - private var lastAction: OnboardingAction? = null private var currentHomeServerConnectionConfig: HomeServerConnectionConfig? = null private val matrixOrgUrl = stringProvider.getString(R.string.matrix_org_server_url).ensureTrailingSlash() @@ -146,9 +146,9 @@ class OnboardingViewModel @AssistedInject constructor( is OnboardingAction.UpdateServerType -> handleUpdateServerType(action) is OnboardingAction.UpdateSignMode -> handleUpdateSignMode(action) is OnboardingAction.InitWith -> handleInitWith(action) - is OnboardingAction.HomeServerChange -> withAction(action) { handleHomeserverChange(action) } + is OnboardingAction.HomeServerChange -> handleHomeserverChange(action) is OnboardingAction.UserNameEnteredAction -> handleUserNameEntered(action) - is AuthenticateAction -> withAction(action) { handleAuthenticateAction(action) } + is AuthenticateAction -> handleAuthenticateAction(action) is OnboardingAction.LoginWithToken -> handleLoginWithToken(action) is OnboardingAction.WebLoginSuccess -> handleWebLoginSuccess(action) is OnboardingAction.ResetPassword -> handleResetPassword(action) @@ -221,11 +221,6 @@ class OnboardingViewModel @AssistedInject constructor( ) } - private fun withAction(action: OnboardingAction, block: (OnboardingAction) -> Unit) { - lastAction = action - block(action) - } - private fun handleAuthenticateAction(action: AuthenticateAction) { when (action) { is AuthenticateAction.Register -> handleRegisterWith(action.username, action.password, action.initialDeviceName) @@ -276,15 +271,15 @@ class OnboardingViewModel @AssistedInject constructor( private fun handleUserAcceptCertificate(action: OnboardingAction.UserAcceptCertificate) { // It happens when we get the login flow, or during direct authentication. // So alter the homeserver config and retrieve again the login flow - when (val finalLastAction = lastAction) { - is OnboardingAction.HomeServerChange.SelectHomeServer -> { + when (action.retryAction) { + is OnboardingAction.HomeServerChange -> { currentHomeServerConnectionConfig ?.let { it.copy(allowedFingerprints = it.allowedFingerprints + action.fingerprint) } - ?.let { startAuthenticationFlow(finalLastAction, it, serverTypeOverride = null) } + ?.let { startAuthenticationFlow(action.retryAction, it, serverTypeOverride = null) } } is AuthenticateAction.LoginDirect -> handleDirectLogin( - finalLastAction, + action.retryAction, HomeServerConnectionConfig.Builder() // Will be replaced by the task .withHomeServerUri("https://dummy.org") @@ -589,9 +584,19 @@ class OnboardingViewModel @AssistedInject constructor( currentJob = viewModelScope.launch { directLoginUseCase.execute(action, homeServerConnectionConfig).fold( onSuccess = { onSessionCreated(it, authenticationDescription = AuthenticationDescription.Login) }, - onFailure = { + onFailure = { error -> setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(it)) + when { + error.isUnrecognisedCertificate() -> { + _viewEvents.post( + OnboardingViewEvents.UnrecognisedCertificateFailure( + retryAction = action, + cause = error as Failure.UnrecognizedCertificateFailure + ) + ) + } + else -> _viewEvents.post(OnboardingViewEvents.Failure(error)) + } } ) } @@ -723,9 +728,10 @@ class OnboardingViewModel @AssistedInject constructor( retryAction = (trigger as OnboardingAction.HomeServerChange.SelectHomeServer).resetToDefaultUrl() ) ) - else -> _viewEvents.post( - OnboardingViewEvents.Failure(error) - ) + error.isUnrecognisedCertificate() -> { + _viewEvents.post(OnboardingViewEvents.UnrecognisedCertificateFailure(trigger, error as Failure.UnrecognizedCertificateFailure)) + } + else -> _viewEvents.post(OnboardingViewEvents.Failure(error)) } } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/AbstractFtueAuthFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/AbstractFtueAuthFragment.kt index 072e94bc30..f3cb326221 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/AbstractFtueAuthFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/AbstractFtueAuthFragment.kt @@ -33,7 +33,6 @@ import im.vector.app.features.onboarding.OnboardingViewEvents import im.vector.app.features.onboarding.OnboardingViewModel import im.vector.app.features.onboarding.OnboardingViewState import kotlinx.coroutines.CancellationException -import org.matrix.android.sdk.api.failure.Failure /** * Parent Fragment for all the login/registration screens. @@ -68,6 +67,7 @@ abstract class AbstractFtueAuthFragment : VectorBaseFragment showFailure(viewEvents.throwable) + is OnboardingViewEvents.UnrecognisedCertificateFailure -> showUnrecognizedCertificateFailure(viewEvents) else -> // This is handled by the Activity Unit @@ -84,20 +84,20 @@ abstract class AbstractFtueAuthFragment : VectorBaseFragment /* Ignore this error, user has cancelled the action */ Unit - is Failure.UnrecognizedCertificateFailure -> showUnrecognizedCertificateFailure(throwable) else -> onError(throwable) } } - private fun showUnrecognizedCertificateFailure(failure: Failure.UnrecognizedCertificateFailure) { + private fun showUnrecognizedCertificateFailure(event: OnboardingViewEvents.UnrecognisedCertificateFailure) { // Ask the user to accept the certificate + val cause = event.cause unrecognizedCertificateDialog.show(requireActivity(), - failure.fingerprint, - failure.url, + cause.fingerprint, + cause.url, object : UnrecognizedCertificateDialog.Callback { override fun onAccept() { // User accept the certificate - viewModel.handle(OnboardingAction.UserAcceptCertificate(failure.fingerprint)) + viewModel.handle(OnboardingAction.UserAcceptCertificate(cause.fingerprint, event.retryAction)) } override fun onIgnore() { diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 150ab74ec2..e568b3d92b 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -202,6 +202,7 @@ class FtueAuthVariant( openMsisdnConfirmation(viewEvents.msisdn) } is OnboardingViewEvents.Failure, + is OnboardingViewEvents.UnrecognisedCertificateFailure, is OnboardingViewEvents.Loading -> // This is handled by the Fragments Unit From a6ff10cbafd57ab4ba72292b062084d9788649ff Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Aug 2022 10:02:47 +0100 Subject: [PATCH 2/5] allowing fingerprint to be passed to the config factory - which in turn allows the android Uri to be bypassed and a unit test around the direct local certificate case added --- .../HomeServerConnectionConfigFactory.kt | 10 ++++- .../onboarding/OnboardingViewModel.kt | 7 +--- .../onboarding/OnboardingViewModelTest.kt | 37 ++++++++++++++++--- .../FakeHomeServerConnectionConfigFactory.kt | 5 ++- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/login/HomeServerConnectionConfigFactory.kt b/vector/src/main/java/im/vector/app/features/login/HomeServerConnectionConfigFactory.kt index 955c3f7290..253c514e5a 100644 --- a/vector/src/main/java/im/vector/app/features/login/HomeServerConnectionConfigFactory.kt +++ b/vector/src/main/java/im/vector/app/features/login/HomeServerConnectionConfigFactory.kt @@ -17,12 +17,13 @@ package im.vector.app.features.login import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig +import org.matrix.android.sdk.api.network.ssl.Fingerprint import timber.log.Timber import javax.inject.Inject class HomeServerConnectionConfigFactory @Inject constructor() { - fun create(url: String?): HomeServerConnectionConfig? { + fun create(url: String?, fingerprint: Fingerprint? = null): HomeServerConnectionConfig? { if (url == null) { return null } @@ -30,6 +31,13 @@ class HomeServerConnectionConfigFactory @Inject constructor() { return try { HomeServerConnectionConfig.Builder() .withHomeServerUri(url) + .run { + if (fingerprint == null) { + this + } else { + withAllowedFingerPrints(listOf(fingerprint)) + } + } .build() } catch (t: Throwable) { Timber.e(t) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index b0ba113d41..0cd5437e5b 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -280,11 +280,8 @@ class OnboardingViewModel @AssistedInject constructor( is AuthenticateAction.LoginDirect -> handleDirectLogin( action.retryAction, - HomeServerConnectionConfig.Builder() - // Will be replaced by the task - .withHomeServerUri("https://dummy.org") - .withAllowedFingerPrints(listOf(action.fingerprint)) - .build() + // Will be replaced by the task + homeServerConnectionConfigFactory.create("https://dummy.org", action.fingerprint) ) else -> Unit } diff --git a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt index b505d05944..f802c3e8f6 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt @@ -58,6 +58,7 @@ import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig import org.matrix.android.sdk.api.auth.data.SsoIdentityProvider import org.matrix.android.sdk.api.auth.registration.Stage import org.matrix.android.sdk.api.failure.Failure +import org.matrix.android.sdk.api.network.ssl.Fingerprint import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilities @@ -69,6 +70,7 @@ private val A_LOADABLE_REGISTER_ACTION = RegisterAction.StartRegistration private val A_NON_LOADABLE_REGISTER_ACTION = RegisterAction.CheckIfEmailHasBeenValidated(delayMillis = -1L) private val A_RESULT_IGNORED_REGISTER_ACTION = RegisterAction.SendAgainThreePid private val A_HOMESERVER_CAPABILITIES = aHomeServerCapabilities(canChangeDisplayName = true, canChangeAvatar = true) +private val A_FINGERPRINT = Fingerprint(ByteArray(1), Fingerprint.HashType.SHA1) private val ANY_CONTINUING_REGISTRATION_RESULT = RegistrationActionHandler.Result.NextStage(Stage.Dummy(mandatory = true)) private val A_DIRECT_LOGIN = OnboardingAction.AuthenticateAction.LoginDirect("@a-user:id.org", "a-password", "a-device-name") private const val A_HOMESERVER_URL = "https://edited-homeserver.org" @@ -406,7 +408,7 @@ class OnboardingViewModelTest { @Test fun `given unavailable deeplink, when selecting homeserver, then emits failure with default homeserver as retry action`() = runTest { fakeContext.givenHasConnection() - fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, A_HOMESERVER_CONFIG) + fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, fingerprint = null, A_HOMESERVER_CONFIG) fakeStartAuthenticationFlowUseCase.givenHomeserverUnavailable(A_HOMESERVER_CONFIG) val test = viewModel.test() @@ -723,6 +725,25 @@ class OnboardingViewModelTest { .assertEvents(OnboardingViewEvents.OnPersonalizationComplete) .finish() } + + @Test + fun `given DirectLogin retry action, when accepting user certificate, then logs in directly`() = runTest { + fakeHomeServerConnectionConfigFactory.givenConfigFor("https://dummy.org", A_FINGERPRINT, A_HOMESERVER_CONFIG) + fakeDirectLoginUseCase.givenSuccessResult(A_DIRECT_LOGIN, config = A_HOMESERVER_CONFIG, result = fakeSession) + givenInitialisesSession(fakeSession) + val test = viewModel.test() + + viewModel.handle(OnboardingAction.UserAcceptCertificate(A_FINGERPRINT, A_DIRECT_LOGIN)) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(isLoading = false) } + ) + .assertEvents(OnboardingViewEvents.OnAccountSignedIn) + .finish() + } @Test fun `given can successfully start password reset, when resetting password, then emits confirmation email sent`() = runTest { @@ -991,15 +1012,19 @@ class OnboardingViewModelTest { fakeRegistrationActionHandler.givenResultsFor(results) } - private fun givenCanSuccessfullyUpdateHomeserver(homeserverUrl: String, resultingState: SelectedHomeserverState) { - fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, A_HOMESERVER_CONFIG) - fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) + private fun givenCanSuccessfullyUpdateHomeserver( + homeserverUrl: String, + resultingState: SelectedHomeserverState, + config: HomeServerConnectionConfig = A_HOMESERVER_CONFIG + ) { + fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, fingerprint = null, config) + fakeStartAuthenticationFlowUseCase.givenResult(config, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) givenRegistrationResultFor(RegisterAction.StartRegistration, RegistrationActionHandler.Result.StartRegistration) - fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) + fakeHomeServerHistoryService.expectUrlToBeAdded(config.homeServerUri.toString()) } private fun givenUpdatingHomeserverErrors(homeserverUrl: String, resultingState: SelectedHomeserverState, error: Throwable) { - fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, A_HOMESERVER_CONFIG) + fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, fingerprint = null, A_HOMESERVER_CONFIG) fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) givenRegistrationResultFor(RegisterAction.StartRegistration, RegistrationActionHandler.Result.Error(error)) fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerConnectionConfigFactory.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerConnectionConfigFactory.kt index 553a35ad8c..c0cfe5375b 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerConnectionConfigFactory.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeHomeServerConnectionConfigFactory.kt @@ -20,11 +20,12 @@ import im.vector.app.features.login.HomeServerConnectionConfigFactory import io.mockk.every import io.mockk.mockk import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig +import org.matrix.android.sdk.api.network.ssl.Fingerprint class FakeHomeServerConnectionConfigFactory { val instance: HomeServerConnectionConfigFactory = mockk() - fun givenConfigFor(url: String, config: HomeServerConnectionConfig) { - every { instance.create(url) } returns config + fun givenConfigFor(url: String, fingerprint: Fingerprint? = null, config: HomeServerConnectionConfig) { + every { instance.create(url, fingerprint) } returns config } } From e4a08d1be14c07b7795708a306f3ff25e03d4e78 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Aug 2022 10:59:02 +0100 Subject: [PATCH 3/5] recreating the homeserver config from the retry action when handle certificate accept action - adds unit tests around the edit/selection cases --- .../onboarding/OnboardingViewModel.kt | 20 +++---- .../onboarding/OnboardingViewModelTest.kt | 59 ++++++++++++++++++- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 0cd5437e5b..6228b95398 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -63,6 +63,7 @@ import org.matrix.android.sdk.api.auth.registration.RegistrationWizard import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.failure.isHomeserverUnavailable import org.matrix.android.sdk.api.failure.isUnrecognisedCertificate +import org.matrix.android.sdk.api.network.ssl.Fingerprint import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider import timber.log.Timber @@ -115,8 +116,6 @@ class OnboardingViewModel @AssistedInject constructor( } } - private var currentHomeServerConnectionConfig: HomeServerConnectionConfig? = null - private val matrixOrgUrl = stringProvider.getString(R.string.matrix_org_server_url).ensureTrailingSlash() private val defaultHomeserverUrl = matrixOrgUrl @@ -272,11 +271,7 @@ class OnboardingViewModel @AssistedInject constructor( // It happens when we get the login flow, or during direct authentication. // So alter the homeserver config and retrieve again the login flow when (action.retryAction) { - is OnboardingAction.HomeServerChange -> { - currentHomeServerConnectionConfig - ?.let { it.copy(allowedFingerprints = it.allowedFingerprints + action.fingerprint) } - ?.let { startAuthenticationFlow(action.retryAction, it, serverTypeOverride = null) } - } + is OnboardingAction.HomeServerChange -> handleHomeserverChange(action.retryAction, fingerprint = action.fingerprint) is AuthenticateAction.LoginDirect -> handleDirectLogin( action.retryAction, @@ -684,8 +679,13 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun handleHomeserverChange(action: OnboardingAction.HomeServerChange, serverTypeOverride: ServerType? = null, postAction: suspend () -> Unit = {}) { - val homeServerConnectionConfig = homeServerConnectionConfigFactory.create(action.homeServerUrl) + private fun handleHomeserverChange( + action: OnboardingAction.HomeServerChange, + serverTypeOverride: ServerType? = null, + fingerprint: Fingerprint? = null, + postAction: suspend () -> Unit = {}, + ) { + val homeServerConnectionConfig = homeServerConnectionConfigFactory.create(action.homeServerUrl, fingerprint) if (homeServerConnectionConfig == null) { // This is invalid _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Unable to create a HomeServerConnectionConfig"))) @@ -700,8 +700,6 @@ class OnboardingViewModel @AssistedInject constructor( serverTypeOverride: ServerType?, postAction: suspend () -> Unit = {}, ) { - currentHomeServerConnectionConfig = homeServerConnectionConfig - currentJob = viewModelScope.launch { setState { copy(isLoading = true) } runCatching { startAuthenticationFlowUseCase.execute(homeServerConnectionConfig) }.fold( diff --git a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt index f802c3e8f6..c612644576 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt @@ -725,7 +725,59 @@ class OnboardingViewModelTest { .assertEvents(OnboardingViewEvents.OnPersonalizationComplete) .finish() } - + + + @Test + fun `given in sign in mode, when accepting user certificate with SelectHomeserver retry action, then emits OnHomeserverEdited`() = runTest { + viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignIn)) + val test = viewModel.test() + fakeVectorFeatures.givenCombinedLoginEnabled() + givenCanSuccessfullyUpdateHomeserver( + A_HOMESERVER_URL, + SELECTED_HOMESERVER_STATE, + config = A_HOMESERVER_CONFIG.copy(allowedFingerprints = listOf(A_FINGERPRINT)), + fingerprint = A_FINGERPRINT, + ) + + viewModel.handle(OnboardingAction.UserAcceptCertificate(A_FINGERPRINT, OnboardingAction.HomeServerChange.SelectHomeServer(A_HOMESERVER_URL))) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(selectedHomeserver = SELECTED_HOMESERVER_STATE) }, + { copy(signMode = SignMode.SignIn) }, + { copy(isLoading = false) } + ) + .assertEvents(OnboardingViewEvents.OpenCombinedLogin) + .finish() + } + + @Test + fun `given in sign up mode, when accepting user certificate with EditHomeserver retry action, then emits OnHomeserverEdited`() = runTest { + viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignUp)) + givenCanSuccessfullyUpdateHomeserver( + A_HOMESERVER_URL, + SELECTED_HOMESERVER_STATE, + config = A_HOMESERVER_CONFIG.copy(allowedFingerprints = listOf(A_FINGERPRINT)), + fingerprint = A_FINGERPRINT, + ) + val test = viewModel.test() + + viewModel.handle(OnboardingAction.UserAcceptCertificate(A_FINGERPRINT, OnboardingAction.HomeServerChange.EditHomeServer(A_HOMESERVER_URL))) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(selectedHomeserver = SELECTED_HOMESERVER_STATE) }, + { copy(isLoading = false) } + + ) + .assertEvents(OnboardingViewEvents.OnHomeserverEdited) + .finish() + } + @Test fun `given DirectLogin retry action, when accepting user certificate, then logs in directly`() = runTest { fakeHomeServerConnectionConfigFactory.givenConfigFor("https://dummy.org", A_FINGERPRINT, A_HOMESERVER_CONFIG) @@ -1015,9 +1067,10 @@ class OnboardingViewModelTest { private fun givenCanSuccessfullyUpdateHomeserver( homeserverUrl: String, resultingState: SelectedHomeserverState, - config: HomeServerConnectionConfig = A_HOMESERVER_CONFIG + config: HomeServerConnectionConfig = A_HOMESERVER_CONFIG, + fingerprint: Fingerprint? = null, ) { - fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, fingerprint = null, config) + fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, fingerprint, config) fakeStartAuthenticationFlowUseCase.givenResult(config, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) givenRegistrationResultFor(RegisterAction.StartRegistration, RegistrationActionHandler.Result.StartRegistration) fakeHomeServerHistoryService.expectUrlToBeAdded(config.homeServerUri.toString()) From 91176eca2243b77984ab52dc544330b7bf480b44 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Aug 2022 11:22:38 +0100 Subject: [PATCH 4/5] adding test cases around certificate errors being thrown and mapped --- .../onboarding/OnboardingViewModelTest.kt | 60 ++++++++++++++++++- .../FakeStartAuthenticationFlowUseCase.kt | 4 ++ .../app/test/fixtures/FailureFixture.kt | 3 + 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt index c612644576..216cb76084 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt @@ -48,6 +48,7 @@ import im.vector.app.test.fakes.FakeVectorOverrides import im.vector.app.test.fakes.toTestString import im.vector.app.test.fixtures.a401ServerError import im.vector.app.test.fixtures.aHomeServerCapabilities +import im.vector.app.test.fixtures.anUnrecognisedCertificateError import im.vector.app.test.test import kotlinx.coroutines.test.runTest import org.amshove.kluent.shouldBeEqualTo @@ -66,6 +67,7 @@ private const val A_DISPLAY_NAME = "a display name" private const val A_PICTURE_FILENAME = "a-picture.png" private val A_SERVER_ERROR = a401ServerError() private val AN_ERROR = RuntimeException("an error!") +private val AN_UNRECOGNISED_CERTIFICATE_ERROR = anUnrecognisedCertificateError() private val A_LOADABLE_REGISTER_ACTION = RegisterAction.StartRegistration private val A_NON_LOADABLE_REGISTER_ACTION = RegisterAction.CheckIfEmailHasBeenValidated(delayMillis = -1L) private val A_RESULT_IGNORED_REGISTER_ACTION = RegisterAction.SendAgainThreePid @@ -322,6 +324,25 @@ class OnboardingViewModelTest { .finish() } + @Test + fun `given has sign in with matrix id sign mode, when handling login or register action fails with certificate error, then emits error`() = runTest { + viewModelWith(initialState.copy(signMode = SignMode.SignInWithMatrixId)) + fakeDirectLoginUseCase.givenFailureResult(A_DIRECT_LOGIN, config = null, cause = AN_UNRECOGNISED_CERTIFICATE_ERROR) + givenInitialisesSession(fakeSession) + val test = viewModel.test() + + viewModel.handle(A_DIRECT_LOGIN) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(isLoading = false) } + ) + .assertEvents(OnboardingViewEvents.UnrecognisedCertificateFailure(A_DIRECT_LOGIN, AN_UNRECOGNISED_CERTIFICATE_ERROR)) + .finish() + } + @Test fun `when handling SignUp then sets sign mode to sign up and starts registration`() = runTest { givenRegistrationResultFor(RegisterAction.StartRegistration, ANY_CONTINUING_REGISTRATION_RESULT) @@ -550,6 +571,44 @@ class OnboardingViewModelTest { .finish() } + @Test + fun `when editing homeserver errors with certificate error, then emits error`() = runTest { + fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, fingerprint = null, A_HOMESERVER_CONFIG) + fakeStartAuthenticationFlowUseCase.givenErrors(A_HOMESERVER_CONFIG, AN_UNRECOGNISED_CERTIFICATE_ERROR) + val editAction = OnboardingAction.HomeServerChange.EditHomeServer(A_HOMESERVER_URL) + val test = viewModel.test() + + viewModel.handle(editAction) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(isLoading = false) } + ) + .assertEvents(OnboardingViewEvents.UnrecognisedCertificateFailure(editAction, AN_UNRECOGNISED_CERTIFICATE_ERROR)) + .finish() + } + + @Test + fun `when selecting homeserver errors with certificate error, then emits error`() = runTest { + fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, fingerprint = null, A_HOMESERVER_CONFIG) + fakeStartAuthenticationFlowUseCase.givenErrors(A_HOMESERVER_CONFIG, AN_UNRECOGNISED_CERTIFICATE_ERROR) + val selectAction = OnboardingAction.HomeServerChange.SelectHomeServer(A_HOMESERVER_URL) + val test = viewModel.test() + + viewModel.handle(selectAction) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(isLoading = false) } + ) + .assertEvents(OnboardingViewEvents.UnrecognisedCertificateFailure(selectAction, AN_UNRECOGNISED_CERTIFICATE_ERROR)) + .finish() + } + @Test fun `given unavailable full matrix id, when a register username is entered, then emits availability error`() = runTest { viewModelWith(initialRegistrationState("ignored-url")) @@ -726,7 +785,6 @@ class OnboardingViewModelTest { .finish() } - @Test fun `given in sign in mode, when accepting user certificate with SelectHomeserver retry action, then emits OnHomeserverEdited`() = runTest { viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignIn)) diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeStartAuthenticationFlowUseCase.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeStartAuthenticationFlowUseCase.kt index bfbef9e565..4b2709facc 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeStartAuthenticationFlowUseCase.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeStartAuthenticationFlowUseCase.kt @@ -31,6 +31,10 @@ class FakeStartAuthenticationFlowUseCase { coEvery { instance.execute(config) } returns result } + fun givenErrors(config: HomeServerConnectionConfig, error: Throwable) { + coEvery { instance.execute(config) } throws error + } + fun givenHomeserverUnavailable(config: HomeServerConnectionConfig) { coEvery { instance.execute(config) } throws aHomeserverUnavailableError() } diff --git a/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt b/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt index 0f44976ab3..8437401294 100644 --- a/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt +++ b/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt @@ -18,6 +18,7 @@ package im.vector.app.test.fixtures import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.failure.MatrixError +import org.matrix.android.sdk.api.network.ssl.Fingerprint import java.net.UnknownHostException import javax.net.ssl.HttpsURLConnection @@ -38,3 +39,5 @@ fun aLoginEmailUnknownError() = Failure.ServerError( ) fun aHomeserverUnavailableError() = Failure.NetworkConnection(UnknownHostException()) + +fun anUnrecognisedCertificateError() = Failure.UnrecognizedCertificateFailure("a-url", Fingerprint(ByteArray(1), Fingerprint.HashType.SHA1)) From e948fe05ca8a55b59ceb619a1750477548347481 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Aug 2022 11:37:12 +0100 Subject: [PATCH 5/5] adding changelog entry --- changelog.d/6864.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6864.bugfix diff --git a/changelog.d/6864.bugfix b/changelog.d/6864.bugfix new file mode 100644 index 0000000000..6db3d7c074 --- /dev/null +++ b/changelog.d/6864.bugfix @@ -0,0 +1 @@ +Fixes server selection being unable to trust certificates