From 341967bf3c31e0f247e6081f5ba0f6e5b32ec523 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 1 Dec 2022 15:25:54 +0100 Subject: [PATCH 1/4] Fix crash when invalid url is entered #7672 --- .../onboarding/OnboardingViewModel.kt | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 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 022fea5ed1..7fe73f8087 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 @@ -118,7 +118,7 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun checkQrCodeLoginCapability(homeServerUrl: String) { + private suspend fun checkQrCodeLoginCapability(config: HomeServerConnectionConfig) { if (!vectorFeatures.isQrCodeLoginEnabled()) { setState { copy( @@ -133,16 +133,12 @@ class OnboardingViewModel @AssistedInject constructor( ) } } else { - viewModelScope.launch { - // check if selected server supports MSC3882 first - homeServerConnectionConfigFactory.create(homeServerUrl)?.let { - val canLoginWithQrCode = authenticationService.isQrLoginSupported(it) - setState { - copy( - canLoginWithQrCode = canLoginWithQrCode - ) - } - } + // check if selected server supports MSC3882 first + val canLoginWithQrCode = authenticationService.isQrLoginSupported(config) + setState { + copy( + canLoginWithQrCode = canLoginWithQrCode + ) } } } @@ -710,7 +706,6 @@ class OnboardingViewModel @AssistedInject constructor( _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Unable to create a HomeServerConnectionConfig"))) } else { startAuthenticationFlow(action, homeServerConnectionConfig, serverTypeOverride, postAction) - checkQrCodeLoginCapability(homeServerConnectionConfig.homeServerUri.toString()) } } @@ -769,6 +764,8 @@ class OnboardingViewModel @AssistedInject constructor( _viewEvents.post(OnboardingViewEvents.OutdatedHomeserver) } + checkQrCodeLoginCapability(config) + when (trigger) { is OnboardingAction.HomeServerChange.SelectHomeServer -> { onHomeServerSelected(config, serverTypeOverride, authResult) From d96ff6e5277b619a637bc2403889a285390bb191 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 1 Dec 2022 15:38:31 +0100 Subject: [PATCH 2/4] Changelog --- changelog.d/7684.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7684.bugfix diff --git a/changelog.d/7684.bugfix b/changelog.d/7684.bugfix new file mode 100644 index 0000000000..4a9af884a1 --- /dev/null +++ b/changelog.d/7684.bugfix @@ -0,0 +1 @@ + Fix crash when invalid homeserver url is entered. From 381103383ec983b3232770214946fb3ce0e95671 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 1 Dec 2022 17:44:12 +0100 Subject: [PATCH 3/4] Fix unit tests. --- .../vector/app/features/onboarding/OnboardingViewModelTest.kt | 3 +++ .../im/vector/app/test/fakes/FakeAuthenticationService.kt | 4 ++++ 2 files changed, 7 insertions(+) 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 718f1ec7a9..1666491afb 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 @@ -1152,11 +1152,13 @@ class OnboardingViewModelTest { resultingState: SelectedHomeserverState, config: HomeServerConnectionConfig = A_HOMESERVER_CONFIG, fingerprint: Fingerprint? = null, + canLoginWithQrCode: Boolean = false, ) { fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, fingerprint, config) fakeStartAuthenticationFlowUseCase.givenResult(config, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) givenRegistrationResultFor(RegisterAction.StartRegistration, RegistrationActionHandler.Result.StartRegistration) fakeHomeServerHistoryService.expectUrlToBeAdded(config.homeServerUri.toString()) + fakeAuthenticationService.givenIsQrLoginSupported(config, canLoginWithQrCode) } private fun givenUpdatingHomeserverErrors(homeserverUrl: String, resultingState: SelectedHomeserverState, error: Throwable) { @@ -1164,6 +1166,7 @@ class OnboardingViewModelTest { fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) givenRegistrationResultFor(RegisterAction.StartRegistration, RegistrationActionHandler.Result.Error(error)) fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) + fakeAuthenticationService.givenIsQrLoginSupported(A_HOMESERVER_CONFIG, false) } private fun givenUserNameIsAvailable(userName: String) { diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeAuthenticationService.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeAuthenticationService.kt index af53913169..5d0e317c57 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeAuthenticationService.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeAuthenticationService.kt @@ -58,6 +58,10 @@ class FakeAuthenticationService : AuthenticationService by mockk() { coEvery { getWellKnownData(matrixId, config) } returns result } + fun givenIsQrLoginSupported(config: HomeServerConnectionConfig, result: Boolean) { + coEvery { isQrLoginSupported(config) } returns result + } + fun givenWellKnownThrows(matrixId: String, config: HomeServerConnectionConfig?, cause: Throwable) { coEvery { getWellKnownData(matrixId, config) } throws cause } From b6aae0c7c10162ba1f502bc4f84b0cc8de196ebb Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 1 Dec 2022 17:51:44 +0100 Subject: [PATCH 4/4] Add unit test for canLoginWithQrCode = true --- .../onboarding/OnboardingViewModelTest.kt | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) 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 1666491afb..92083eb50b 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 @@ -160,6 +160,28 @@ class OnboardingViewModelTest { .finish() } + @Test + fun `given combined login enabled, when handling sign in splash action, then emits OpenCombinedLogin with default homeserver qrCode supported`() = runTest { + val test = viewModel.test() + fakeVectorFeatures.givenCombinedLoginEnabled() + givenCanSuccessfullyUpdateHomeserver(A_DEFAULT_HOMESERVER_URL, DEFAULT_SELECTED_HOMESERVER_STATE, canLoginWithQrCode = true) + + viewModel.handle(OnboardingAction.SplashAction.OnIAlreadyHaveAnAccount(OnboardingFlow.SignIn)) + + test + .assertStatesChanges( + initialState, + { copy(onboardingFlow = OnboardingFlow.SignIn) }, + { copy(isLoading = true) }, + { copy(canLoginWithQrCode = true) }, + { copy(selectedHomeserver = DEFAULT_SELECTED_HOMESERVER_STATE) }, + { copy(signMode = SignMode.SignIn) }, + { copy(isLoading = false) } + ) + .assertEvents(OnboardingViewEvents.OpenCombinedLogin) + .finish() + } + @Test fun `given can successfully login in with token, when logging in with token, then emits AccountSignedIn`() = runTest { val test = viewModel.test()