From e3d46cfd151e7f496a3c0e7e515cceab5f049a89 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 26 May 2022 17:25:55 +0100 Subject: [PATCH 1/7] introducing a reset state for holding onto the reset inputs --- .../vector/app/features/onboarding/OnboardingViewModel.kt | 6 +++--- .../vector/app/features/onboarding/OnboardingViewState.kt | 7 ++++++- .../onboarding/ftueauth/AbstractFtueAuthFragment.kt | 2 +- .../FtueAuthResetPasswordMailConfirmationFragment.kt | 2 +- 4 files changed, 11 insertions(+), 6 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 8c0eeb7b4f..39c5b9f3c1 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 @@ -377,7 +377,7 @@ class OnboardingViewModel @AssistedInject constructor( setState { copy( isLoading = false, - resetPasswordEmail = null + resetState = ResetState() ) } } @@ -466,7 +466,7 @@ class OnboardingViewModel @AssistedInject constructor( setState { copy( isLoading = false, - resetPasswordEmail = action.email + resetState = ResetState(email = action.email) ) } @@ -495,7 +495,7 @@ class OnboardingViewModel @AssistedInject constructor( setState { copy( isLoading = false, - resetPasswordEmail = null + resetState = ResetState() ) } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt index e91fee4d21..761d9a4472 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt @@ -39,7 +39,7 @@ data class OnboardingViewState( @PersistState val signMode: SignMode = SignMode.Unknown, @PersistState - val resetPasswordEmail: String? = null, + val resetState: ResetState = ResetState(), // For SSO session recovery @PersistState @@ -84,6 +84,11 @@ data class PersonalizationState( fun supportsPersonalization() = supportsChangingDisplayName || supportsChangingProfilePicture } +@Parcelize +data class ResetState( + val email: String? = null +) : Parcelable + @Parcelize data class SelectedAuthenticationState( val description: AuthenticationDescription? = null, 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 4fa94541e5..a851e13e36 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 @@ -153,7 +153,7 @@ abstract class AbstractFtueAuthFragment : VectorBaseFragment // True when email is sent with success to the homeserver - isResetPasswordStarted = state.resetPasswordEmail.isNullOrBlank().not() + isResetPasswordStarted = state.resetState.email.isNullOrBlank().not() updateWithState(state) } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordMailConfirmationFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordMailConfirmationFragment.kt index fd7f14b1cc..76fbae6f40 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordMailConfirmationFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordMailConfirmationFragment.kt @@ -44,7 +44,7 @@ class FtueAuthResetPasswordMailConfirmationFragment @Inject constructor() : Abst } private fun setupUi(state: OnboardingViewState) { - views.resetPasswordMailConfirmationNotice.text = getString(R.string.login_reset_password_mail_confirmation_notice, state.resetPasswordEmail) + views.resetPasswordMailConfirmationNotice.text = getString(R.string.login_reset_password_mail_confirmation_notice, state.resetState.email) } private fun submit() { From 35163f77ba1335b0893a10d9807a2393eeca60d7 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 27 May 2022 15:38:33 +0100 Subject: [PATCH 2/7] allow passing the new password when resetting passwords either upfront or as part of the confirmation step --- changelog.d/6169.sdk | 1 + .../android/sdk/api/auth/login/LoginWizard.kt | 8 +++-- .../internal/auth/login/DefaultLoginWizard.kt | 30 ++++++++++++------- .../internal/auth/login/ResetPasswordData.kt | 2 +- 4 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 changelog.d/6169.sdk diff --git a/changelog.d/6169.sdk b/changelog.d/6169.sdk new file mode 100644 index 0000000000..dfee158c3f --- /dev/null +++ b/changelog.d/6169.sdk @@ -0,0 +1 @@ +Allows new passwords to be passed at the point of confirmation when resetting a password diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt index f5670875c2..cf810de733 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt @@ -65,16 +65,18 @@ interface LoginWizard { * [resetPasswordMailConfirmed] is successfully called. * * @param email an email previously associated to the account the user wants the password to be reset. - * @param newPassword the desired new password + * @param newPassword the desired new password, can be optionally set here or as part of [resetPasswordMailConfirmed] */ suspend fun resetPassword( email: String, - newPassword: String + newPassword: String? = null ) /** * Confirm the new password, once the user has checked their email * When this method succeed, tha account password will be effectively modified. + * + * @param newPassword the desired new password, required if a password was not supplied to [resetPassword] */ - suspend fun resetPasswordMailConfirmed() + suspend fun resetPasswordMailConfirmed(newPassword: String? = null) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt index 0a189f86e6..f949e793df 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt @@ -30,6 +30,7 @@ import org.matrix.android.sdk.internal.auth.data.ThreePidMedium import org.matrix.android.sdk.internal.auth.data.TokenLoginParams import org.matrix.android.sdk.internal.auth.db.PendingSessionData import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistrationParams +import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistrationResponse import org.matrix.android.sdk.internal.auth.registration.RegisterAddThreePidTask import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.session.content.DefaultContentUrlResolver @@ -103,7 +104,7 @@ internal class DefaultLoginWizard( return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig) } - override suspend fun resetPassword(email: String, newPassword: String) { + override suspend fun resetPassword(email: String, newPassword: String?) { val param = RegisterAddThreePidTask.Params( RegisterThreePid.Email(email), pendingSessionData.clientSecret, @@ -121,15 +122,14 @@ internal class DefaultLoginWizard( .also { pendingSessionStore.savePendingSessionData(it) } } - override suspend fun resetPasswordMailConfirmed() { - val safeResetPasswordData = pendingSessionData.resetPasswordData - ?: throw IllegalStateException("developer error, no reset password in progress") - - val param = ResetPasswordMailConfirmed.create( - pendingSessionData.clientSecret, - safeResetPasswordData.addThreePidRegistrationResponse.sid, - safeResetPasswordData.newPassword - ) + override suspend fun resetPasswordMailConfirmed(newPassword: String?) { + val param = pendingSessionData.readResetPasswordDataOrThrow(newPassword).let { (response, password) -> + ResetPasswordMailConfirmed.create( + pendingSessionData.clientSecret, + response.sid, + password + ) + } executeRequest(null) { authAPI.resetPasswordMailConfirmed(param) @@ -138,4 +138,14 @@ internal class DefaultLoginWizard( // Set to null? // resetPasswordData = null } + + private fun PendingSessionData.readResetPasswordDataOrThrow(newPassword: String?): Pair { + return when (resetPasswordData) { + null -> throw IllegalStateException("developer error, no reset password in progress") + else -> { + val password = newPassword ?: resetPasswordData.newPassword ?: throw IllegalStateException("developer error, no new password set") + resetPasswordData.addThreePidRegistrationResponse to password + } + } + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt index a65ec38d6d..0e31d2bf13 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt @@ -24,6 +24,6 @@ import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistration */ @JsonClass(generateAdapter = true) internal data class ResetPasswordData( - val newPassword: String, + val newPassword: String?, val addThreePidRegistrationResponse: AddThreePidRegistrationResponse ) From cc8f17b78682f452991fc37fafafcb631aef66cf Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 30 May 2022 11:12:03 +0100 Subject: [PATCH 3/7] moving the reset password new password to the reset confirmation step - the new password is moved to the in memory view model state --- .../android/sdk/api/auth/login/LoginWizard.kt | 10 ++----- .../internal/auth/login/DefaultLoginWizard.kt | 29 ++++++------------- .../internal/auth/login/ResetPasswordData.kt | 1 - .../app/features/login/LoginViewModel.kt | 14 +++++---- .../app/features/login/LoginViewState.kt | 2 ++ .../app/features/login2/LoginViewModel2.kt | 14 +++++---- .../app/features/login2/LoginViewState2.kt | 2 ++ .../onboarding/OnboardingViewModel.kt | 7 +++-- .../onboarding/OnboardingViewState.kt | 3 +- 9 files changed, 40 insertions(+), 42 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt index cf810de733..5b8d2328c7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt @@ -65,18 +65,14 @@ interface LoginWizard { * [resetPasswordMailConfirmed] is successfully called. * * @param email an email previously associated to the account the user wants the password to be reset. - * @param newPassword the desired new password, can be optionally set here or as part of [resetPasswordMailConfirmed] */ - suspend fun resetPassword( - email: String, - newPassword: String? = null - ) + suspend fun resetPassword(email: String) /** * Confirm the new password, once the user has checked their email * When this method succeed, tha account password will be effectively modified. * - * @param newPassword the desired new password, required if a password was not supplied to [resetPassword] + * @param newPassword the desired new password */ - suspend fun resetPasswordMailConfirmed(newPassword: String? = null) + suspend fun resetPasswordMailConfirmed(newPassword: String) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt index f949e793df..f947d76b95 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt @@ -104,7 +104,7 @@ internal class DefaultLoginWizard( return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig) } - override suspend fun resetPassword(email: String, newPassword: String?) { + override suspend fun resetPassword(email: String) { val param = RegisterAddThreePidTask.Params( RegisterThreePid.Email(email), pendingSessionData.clientSecret, @@ -118,18 +118,17 @@ internal class DefaultLoginWizard( authAPI.resetPassword(AddThreePidRegistrationParams.from(param)) } - pendingSessionData = pendingSessionData.copy(resetPasswordData = ResetPasswordData(newPassword, result)) + pendingSessionData = pendingSessionData.copy(resetPasswordData = ResetPasswordData(result)) .also { pendingSessionStore.savePendingSessionData(it) } } - override suspend fun resetPasswordMailConfirmed(newPassword: String?) { - val param = pendingSessionData.readResetPasswordDataOrThrow(newPassword).let { (response, password) -> - ResetPasswordMailConfirmed.create( - pendingSessionData.clientSecret, - response.sid, - password - ) - } + override suspend fun resetPasswordMailConfirmed(newPassword: String) { + val resetPasswordData = pendingSessionData.resetPasswordData ?: throw IllegalStateException("Developer error - Must call resetPassword first") + val param = ResetPasswordMailConfirmed.create( + pendingSessionData.clientSecret, + resetPasswordData.addThreePidRegistrationResponse.sid, + newPassword + ) executeRequest(null) { authAPI.resetPasswordMailConfirmed(param) @@ -138,14 +137,4 @@ internal class DefaultLoginWizard( // Set to null? // resetPasswordData = null } - - private fun PendingSessionData.readResetPasswordDataOrThrow(newPassword: String?): Pair { - return when (resetPasswordData) { - null -> throw IllegalStateException("developer error, no reset password in progress") - else -> { - val password = newPassword ?: resetPasswordData.newPassword ?: throw IllegalStateException("developer error, no new password set") - resetPasswordData.addThreePidRegistrationResponse to password - } - } - } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt index 0e31d2bf13..87a7b346dc 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/ResetPasswordData.kt @@ -24,6 +24,5 @@ import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistration */ @JsonClass(generateAdapter = true) internal data class ResetPasswordData( - val newPassword: String?, val addThreePidRegistrationResponse: AddThreePidRegistrationResponse ) diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt index 5e9a95083e..c13372cbe4 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt @@ -413,7 +413,8 @@ class LoginViewModel @AssistedInject constructor( copy( asyncResetPassword = Uninitialized, asyncResetMailConfirmed = Uninitialized, - resetPasswordEmail = null + resetPasswordEmail = null, + resetPasswordNewPassword = null ) } } @@ -488,7 +489,7 @@ class LoginViewModel @AssistedInject constructor( currentJob = viewModelScope.launch { try { - safeLoginWizard.resetPassword(action.email, action.newPassword) + safeLoginWizard.resetPassword(action.email) } catch (failure: Throwable) { setState { copy( @@ -501,7 +502,8 @@ class LoginViewModel @AssistedInject constructor( setState { copy( asyncResetPassword = Success(Unit), - resetPasswordEmail = action.email + resetPasswordEmail = action.email, + resetPasswordNewPassword = action.newPassword ) } @@ -530,7 +532,8 @@ class LoginViewModel @AssistedInject constructor( currentJob = viewModelScope.launch { try { - safeLoginWizard.resetPasswordMailConfirmed() + val state = awaitState() + safeLoginWizard.resetPasswordMailConfirmed(state.resetPasswordNewPassword!!) } catch (failure: Throwable) { setState { copy( @@ -542,7 +545,8 @@ class LoginViewModel @AssistedInject constructor( setState { copy( asyncResetMailConfirmed = Success(Unit), - resetPasswordEmail = null + resetPasswordEmail = null, + resetPasswordNewPassword = null ) } diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewState.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewState.kt index 23689225b5..83540d6205 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewState.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewState.kt @@ -38,6 +38,8 @@ data class LoginViewState( @PersistState val resetPasswordEmail: String? = null, @PersistState + val resetPasswordNewPassword: String? = null, + @PersistState val homeServerUrlFromUser: String? = null, // Can be modified after a Wellknown request diff --git a/vector/src/main/java/im/vector/app/features/login2/LoginViewModel2.kt b/vector/src/main/java/im/vector/app/features/login2/LoginViewModel2.kt index 81601e6b59..2120d2997c 100644 --- a/vector/src/main/java/im/vector/app/features/login2/LoginViewModel2.kt +++ b/vector/src/main/java/im/vector/app/features/login2/LoginViewModel2.kt @@ -392,7 +392,8 @@ class LoginViewModel2 @AssistedInject constructor( LoginAction2.ResetResetPassword -> { setState { copy( - resetPasswordEmail = null + resetPasswordEmail = null, + resetPasswordNewPassword = null ) } } @@ -443,7 +444,7 @@ class LoginViewModel2 @AssistedInject constructor( currentJob = viewModelScope.launch { try { - safeLoginWizard.resetPassword(action.email, action.newPassword) + safeLoginWizard.resetPassword(action.email) } catch (failure: Throwable) { _viewEvents.post(LoginViewEvents2.Failure(failure)) setState { copy(isLoading = false) } @@ -453,7 +454,8 @@ class LoginViewModel2 @AssistedInject constructor( setState { copy( isLoading = false, - resetPasswordEmail = action.email + resetPasswordEmail = action.email, + resetPasswordNewPassword = action.newPassword ) } @@ -472,7 +474,8 @@ class LoginViewModel2 @AssistedInject constructor( currentJob = viewModelScope.launch { try { - safeLoginWizard.resetPasswordMailConfirmed() + val state = awaitState() + safeLoginWizard.resetPasswordMailConfirmed(state.resetPasswordNewPassword!!) } catch (failure: Throwable) { _viewEvents.post(LoginViewEvents2.Failure(failure)) setState { copy(isLoading = false) } @@ -481,7 +484,8 @@ class LoginViewModel2 @AssistedInject constructor( setState { copy( isLoading = false, - resetPasswordEmail = null + resetPasswordEmail = null, + resetPasswordNewPassword = null ) } diff --git a/vector/src/main/java/im/vector/app/features/login2/LoginViewState2.kt b/vector/src/main/java/im/vector/app/features/login2/LoginViewState2.kt index 276d1111bb..8405381c4f 100644 --- a/vector/src/main/java/im/vector/app/features/login2/LoginViewState2.kt +++ b/vector/src/main/java/im/vector/app/features/login2/LoginViewState2.kt @@ -36,6 +36,8 @@ data class LoginViewState2( @PersistState val resetPasswordEmail: String? = null, @PersistState + val resetPasswordNewPassword: String? = null, + @PersistState val homeServerUrlFromUser: String? = null, // Can be modified after a Wellknown request 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 39c5b9f3c1..71093a6600 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 @@ -456,7 +456,7 @@ class OnboardingViewModel @AssistedInject constructor( currentJob = viewModelScope.launch { try { - safeLoginWizard.resetPassword(action.email, action.newPassword) + safeLoginWizard.resetPassword(action.email) } catch (failure: Throwable) { setState { copy(isLoading = false) } _viewEvents.post(OnboardingViewEvents.Failure(failure)) @@ -466,7 +466,7 @@ class OnboardingViewModel @AssistedInject constructor( setState { copy( isLoading = false, - resetState = ResetState(email = action.email) + resetState = ResetState(email = action.email, newPassword = action.newPassword) ) } @@ -486,7 +486,8 @@ class OnboardingViewModel @AssistedInject constructor( currentJob = viewModelScope.launch { try { - safeLoginWizard.resetPasswordMailConfirmed() + val state = awaitState() + safeLoginWizard.resetPasswordMailConfirmed(state.resetState.newPassword!!) } catch (failure: Throwable) { setState { copy(isLoading = false) } _viewEvents.post(OnboardingViewEvents.Failure(failure)) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt index 761d9a4472..268b1e7d49 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewState.kt @@ -86,7 +86,8 @@ data class PersonalizationState( @Parcelize data class ResetState( - val email: String? = null + val email: String? = null, + val newPassword: String? = null, ) : Parcelable @Parcelize From 93a247e0ce728b1edaf52cadd7aa495f6a0ffe8e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 30 May 2022 11:15:23 +0100 Subject: [PATCH 4/7] converting if/else and try/catch to when and runCatching --- .../onboarding/OnboardingViewModel.kt | 117 ++++++++---------- 1 file changed, 51 insertions(+), 66 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 71093a6600..0321f97532 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 @@ -128,7 +128,7 @@ class OnboardingViewModel @AssistedInject constructor( val isRegistrationStarted: Boolean get() = authenticationService.isRegistrationStarted() - private val loginWizard: LoginWizard? + private val loginWizard: LoginWizard get() = authenticationService.getLoginWizard() private var loginConfig: LoginConfig? = null @@ -447,60 +447,51 @@ class OnboardingViewModel @AssistedInject constructor( private fun handleResetPassword(action: OnboardingAction.ResetPassword) { val safeLoginWizard = loginWizard - - if (safeLoginWizard == null) { - setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Bad configuration"))) - } else { - setState { copy(isLoading = true) } - - currentJob = viewModelScope.launch { - try { - safeLoginWizard.resetPassword(action.email) - } catch (failure: Throwable) { - setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(failure)) - return@launch - } - - setState { - copy( - isLoading = false, - resetState = ResetState(email = action.email, newPassword = action.newPassword) - ) - } - - _viewEvents.post(OnboardingViewEvents.OnResetPasswordSendThreePidDone) - } + setState { copy(isLoading = true) } + currentJob = viewModelScope.launch { + runCatching { safeLoginWizard.resetPassword(action.email) }.fold( + onSuccess = { + setState { + copy( + isLoading = false, + resetState = ResetState(email = action.email, newPassword = action.newPassword) + ) + } + _viewEvents.post(OnboardingViewEvents.OnResetPasswordSendThreePidDone) + }, + onFailure = { + setState { copy(isLoading = false) } + _viewEvents.post(OnboardingViewEvents.Failure(it)) + } + ) } } private fun handleResetPasswordMailConfirmed() { - val safeLoginWizard = loginWizard - - if (safeLoginWizard == null) { - setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Bad configuration"))) - } else { - setState { copy(isLoading = false) } - - currentJob = viewModelScope.launch { - try { - val state = awaitState() - safeLoginWizard.resetPasswordMailConfirmed(state.resetState.newPassword!!) - } catch (failure: Throwable) { + currentJob = viewModelScope.launch { + val resetState = awaitState().resetState + when (val newPassword = resetState.newPassword) { + null -> { setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(failure)) - return@launch + _viewEvents.post(OnboardingViewEvents.Failure(IllegalStateException("Developer error - No new password has been set"))) } - setState { - copy( - isLoading = false, - resetState = ResetState() + else -> { + runCatching { loginWizard.resetPasswordMailConfirmed(newPassword) }.fold( + onSuccess = { + setState { + copy( + isLoading = false, + resetState = ResetState() + ) + } + _viewEvents.post(OnboardingViewEvents.OnResetPasswordMailConfirmationSuccess) + }, + onFailure = { + setState { copy(isLoading = false) } + _viewEvents.post(OnboardingViewEvents.Failure(it)) + } ) } - - _viewEvents.post(OnboardingViewEvents.OnResetPasswordMailConfirmationSuccess) } } } @@ -520,25 +511,19 @@ class OnboardingViewModel @AssistedInject constructor( private fun handleLogin(action: AuthenticateAction.Login) { val safeLoginWizard = loginWizard - - if (safeLoginWizard == null) { - setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Bad configuration"))) - } else { - setState { copy(isLoading = true) } - currentJob = viewModelScope.launch { - try { - val result = safeLoginWizard.login( - action.username, - action.password, - action.initialDeviceName - ) - reAuthHelper.data = action.password - onSessionCreated(result, authenticationDescription = AuthenticationDescription.Login) - } catch (failure: Throwable) { - setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(failure)) - } + setState { copy(isLoading = true) } + currentJob = viewModelScope.launch { + try { + val result = safeLoginWizard.login( + action.username, + action.password, + action.initialDeviceName + ) + reAuthHelper.data = action.password + onSessionCreated(result, authenticationDescription = AuthenticationDescription.Login) + } catch (failure: Throwable) { + setState { copy(isLoading = false) } + _viewEvents.post(OnboardingViewEvents.Failure(failure)) } } } From 32389a9b33d025257e65c0596beb88c51807d8c8 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 30 May 2022 13:22:44 +0100 Subject: [PATCH 5/7] removing impossible case --- .../onboarding/OnboardingViewModel.kt | 22 +++++++------------ 1 file changed, 8 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 0321f97532..23bab0c62d 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 @@ -246,21 +246,15 @@ class OnboardingViewModel @AssistedInject constructor( private fun handleLoginWithToken(action: OnboardingAction.LoginWithToken) { val safeLoginWizard = loginWizard + setState { copy(isLoading = true) } - if (safeLoginWizard == null) { - setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Bad configuration"))) - } else { - setState { copy(isLoading = true) } - - currentJob = viewModelScope.launch { - try { - val result = safeLoginWizard.loginWithToken(action.loginToken) - onSessionCreated(result, authenticationDescription = AuthenticationDescription.Login) - } catch (failure: Throwable) { - setState { copy(isLoading = false) } - _viewEvents.post(OnboardingViewEvents.Failure(failure)) - } + currentJob = viewModelScope.launch { + try { + val result = safeLoginWizard.loginWithToken(action.loginToken) + onSessionCreated(result, authenticationDescription = AuthenticationDescription.Login) + } catch (failure: Throwable) { + setState { copy(isLoading = false) } + _viewEvents.post(OnboardingViewEvents.Failure(failure)) } } } From fa5b7c66ca3368af9e458276bd3d66056d7b8a4e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 30 May 2022 13:35:06 +0100 Subject: [PATCH 6/7] adding dedicated fail event in the legacy onboarding flow when the reset password new password is not set --- .../app/features/login/LoginViewModel.kt | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt index c13372cbe4..5a1e8b107f 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt @@ -531,26 +531,34 @@ class LoginViewModel @AssistedInject constructor( } currentJob = viewModelScope.launch { - try { - val state = awaitState() - safeLoginWizard.resetPasswordMailConfirmed(state.resetPasswordNewPassword!!) - } catch (failure: Throwable) { + val state = awaitState() + + if (state.resetPasswordNewPassword == null) setState { copy( - asyncResetMailConfirmed = Fail(failure) + asyncResetPassword = Uninitialized, + asyncResetMailConfirmed = Fail(Throwable("Developer error - New password not set")) + ) + } else { + try { + safeLoginWizard.resetPasswordMailConfirmed(state.resetPasswordNewPassword) + } catch (failure: Throwable) { + setState { + copy( + asyncResetMailConfirmed = Fail(failure) + ) + } + return@launch + } + setState { + copy( + asyncResetMailConfirmed = Success(Unit), + resetPasswordEmail = null, + resetPasswordNewPassword = null ) } - return@launch + _viewEvents.post(LoginViewEvents.OnResetPasswordMailConfirmationSuccess) } - setState { - copy( - asyncResetMailConfirmed = Success(Unit), - resetPasswordEmail = null, - resetPasswordNewPassword = null - ) - } - - _viewEvents.post(LoginViewEvents.OnResetPasswordMailConfirmationSuccess) } } } From edfabb0f260aaafa9a5333c0c22b68eaf291c2f2 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 30 May 2022 13:51:33 +0100 Subject: [PATCH 7/7] adding missing loading state when confirming password reset - adds reset test cases to the onboarding view model --- .../internal/auth/login/DefaultLoginWizard.kt | 1 - .../app/features/login/LoginViewModel.kt | 5 ++- .../onboarding/OnboardingViewModel.kt | 1 + .../onboarding/OnboardingViewModelTest.kt | 41 +++++++++++++++++++ .../test/fakes/FakeAuthenticationService.kt | 5 +++ .../vector/app/test/fakes/FakeLoginWizard.kt | 32 +++++++++++++++ 6 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeLoginWizard.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt index f947d76b95..20b056f1c7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt @@ -30,7 +30,6 @@ import org.matrix.android.sdk.internal.auth.data.ThreePidMedium import org.matrix.android.sdk.internal.auth.data.TokenLoginParams import org.matrix.android.sdk.internal.auth.db.PendingSessionData import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistrationParams -import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistrationResponse import org.matrix.android.sdk.internal.auth.registration.RegisterAddThreePidTask import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.session.content.DefaultContentUrlResolver diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt index 5a1e8b107f..116b035c8e 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt @@ -533,13 +533,14 @@ class LoginViewModel @AssistedInject constructor( currentJob = viewModelScope.launch { val state = awaitState() - if (state.resetPasswordNewPassword == null) + if (state.resetPasswordNewPassword == null) { setState { copy( asyncResetPassword = Uninitialized, asyncResetMailConfirmed = Fail(Throwable("Developer error - New password not set")) ) - } else { + } + } else { try { safeLoginWizard.resetPasswordMailConfirmed(state.resetPasswordNewPassword) } catch (failure: Throwable) { 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 23bab0c62d..c24d3dbb48 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 @@ -462,6 +462,7 @@ class OnboardingViewModel @AssistedInject constructor( } private fun handleResetPasswordMailConfirmed() { + setState { copy(isLoading = true) } currentJob = viewModelScope.launch { val resetState = awaitState().resetState when (val newPassword = resetState.newPassword) { 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 1abfa7e9a8..77539da232 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 @@ -31,6 +31,7 @@ import im.vector.app.test.fakes.FakeContext import im.vector.app.test.fakes.FakeDirectLoginUseCase import im.vector.app.test.fakes.FakeHomeServerConnectionConfigFactory import im.vector.app.test.fakes.FakeHomeServerHistoryService +import im.vector.app.test.fakes.FakeLoginWizard import im.vector.app.test.fakes.FakeRegisterActionHandler import im.vector.app.test.fakes.FakeRegistrationWizard import im.vector.app.test.fakes.FakeSession @@ -67,6 +68,8 @@ private val A_DIRECT_LOGIN = OnboardingAction.AuthenticateAction.LoginDirect("@a private const val A_HOMESERVER_URL = "https://edited-homeserver.org" private val A_HOMESERVER_CONFIG = HomeServerConnectionConfig(FakeUri().instance) private val SELECTED_HOMESERVER_STATE = SelectedHomeserverState(preferredLoginMode = LoginMode.Password) +private const val AN_EMAIL = "hello@example.com" +private const val A_PASSWORD = "a-password" class OnboardingViewModelTest { @@ -85,6 +88,7 @@ class OnboardingViewModelTest { private val fakeHomeServerConnectionConfigFactory = FakeHomeServerConnectionConfigFactory() private val fakeStartAuthenticationFlowUseCase = FakeStartAuthenticationFlowUseCase() private val fakeHomeServerHistoryService = FakeHomeServerHistoryService() + private val fakeLoginWizard = FakeLoginWizard() private var initialState = OnboardingViewState() private lateinit var viewModel: OnboardingViewModel @@ -466,6 +470,43 @@ class OnboardingViewModelTest { .finish() } + @Test + fun `given can successfully reset password, when resetting password, then emits reset done event`() = runTest { + val test = viewModel.test() + fakeLoginWizard.givenResetPasswordSuccess(AN_EMAIL) + fakeAuthenticationService.givenLoginWizard(fakeLoginWizard) + + viewModel.handle(OnboardingAction.ResetPassword(email = AN_EMAIL, newPassword = A_PASSWORD)) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(isLoading = false, resetState = ResetState(AN_EMAIL, A_PASSWORD)) } + ) + .assertEvents(OnboardingViewEvents.OnResetPasswordSendThreePidDone) + .finish() + } + + @Test + fun `given can successfully confirm reset password, when confirm reset password, then emits reset success`() = runTest { + viewModelWith(initialState.copy(resetState = ResetState(AN_EMAIL, A_PASSWORD))) + val test = viewModel.test() + fakeLoginWizard.givenConfirmResetPasswordSuccess(A_PASSWORD) + fakeAuthenticationService.givenLoginWizard(fakeLoginWizard) + + viewModel.handle(OnboardingAction.ResetPasswordMailConfirmed) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(isLoading = false, resetState = ResetState()) } + ) + .assertEvents(OnboardingViewEvents.OnResetPasswordMailConfirmationSuccess) + .finish() + } + private fun viewModelWith(state: OnboardingViewState) { OnboardingViewModel( state, 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 0456bbd474..cc606497f5 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 @@ -23,6 +23,7 @@ import io.mockk.mockk import org.matrix.android.sdk.api.auth.AuthenticationService import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig import org.matrix.android.sdk.api.auth.data.LoginFlowResult +import org.matrix.android.sdk.api.auth.login.LoginWizard import org.matrix.android.sdk.api.auth.registration.RegistrationWizard import org.matrix.android.sdk.api.auth.wellknown.WellknownResult @@ -36,6 +37,10 @@ class FakeAuthenticationService : AuthenticationService by mockk() { every { isRegistrationStarted() } returns started } + fun givenLoginWizard(loginWizard: LoginWizard) { + every { getLoginWizard() } returns loginWizard + } + fun givenLoginFlow(config: HomeServerConnectionConfig, result: LoginFlowResult) { coEvery { getLoginFlow(config) } returns result } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeLoginWizard.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeLoginWizard.kt new file mode 100644 index 0000000000..38bb75087c --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeLoginWizard.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2022 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.test.fakes + +import io.mockk.coJustRun +import io.mockk.mockk +import org.matrix.android.sdk.api.auth.login.LoginWizard + +class FakeLoginWizard : LoginWizard by mockk() { + + fun givenResetPasswordSuccess(email: String) { + coJustRun { resetPassword(email) } + } + + fun givenConfirmResetPasswordSuccess(password: String) { + coJustRun { resetPasswordMailConfirmed(password) } + } +}