Merge pull request #6171 from vector-im/feature/adm/sdk-new-password-on-confirmation

[SDK] Allow passwords to be set at the point of reset confirmation

changelog.d/6169.sdk

@@ -0,0 +1 @@
+Allows new passwords to be passed at the point of confirmation when resetting a password

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt

@@ -65,16 +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
      */
-    suspend fun resetPassword(
+    suspend fun resetPassword(email: String)
-            email: String,
-            newPassword: 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
      */
-    suspend fun resetPasswordMailConfirmed()
+    suspend fun resetPasswordMailConfirmed(newPassword: String)
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt

@@ -103,7 +103,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,
@@ -117,18 +117,16 @@ 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() {
+    override suspend fun resetPasswordMailConfirmed(newPassword: String) {
-        val safeResetPasswordData = pendingSessionData.resetPasswordData
+        val resetPasswordData = pendingSessionData.resetPasswordData ?: throw IllegalStateException("Developer error - Must call resetPassword first")
-                ?: throw IllegalStateException("developer error, no reset password in progress")
-
         val param = ResetPasswordMailConfirmed.create(
                 pendingSessionData.clientSecret,
-                safeResetPasswordData.addThreePidRegistrationResponse.sid,
+                resetPasswordData.addThreePidRegistrationResponse.sid,
-                safeResetPasswordData.newPassword
+                newPassword
         )
 
         executeRequest(null) {

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
 )

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
                     )
                 }
 
@@ -529,8 +531,18 @@ class LoginViewModel @AssistedInject constructor(
             }
 
             currentJob = viewModelScope.launch {
+                val state = awaitState()
+
+                if (state.resetPasswordNewPassword == null) {
+                    setState {
+                        copy(
+                                asyncResetPassword = Uninitialized,
+                                asyncResetMailConfirmed = Fail(Throwable("Developer error - New password not set"))
+                        )
+                    }
+                } else {
                     try {
-                    safeLoginWizard.resetPasswordMailConfirmed()
+                        safeLoginWizard.resetPasswordMailConfirmed(state.resetPasswordNewPassword)
                     } catch (failure: Throwable) {
                         setState {
                             copy(
@@ -542,14 +554,15 @@ class LoginViewModel @AssistedInject constructor(
                     setState {
                         copy(
                                 asyncResetMailConfirmed = Success(Unit),
-                            resetPasswordEmail = null
+                                resetPasswordEmail = null,
+                                resetPasswordNewPassword = null
                         )
                     }
-
                     _viewEvents.post(LoginViewEvents.OnResetPasswordMailConfirmationSuccess)
                 }
             }
         }
+    }
 
     private fun handleLoginOrRegister(action: LoginAction.LoginOrRegister) = withState { state ->
         when (state.signMode) {

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

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
                     )
                 }
 

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

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
@@ -246,11 +246,6 @@ class OnboardingViewModel @AssistedInject constructor(
 
     private fun handleLoginWithToken(action: OnboardingAction.LoginWithToken) {
         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 {
@@ -263,7 +258,6 @@ class OnboardingViewModel @AssistedInject constructor(
             }
         }
     }
-    }
 
     private fun handleRegisterAction(action: RegisterAction, onNextRegistrationStepAction: (FlowResult) -> Unit) {
         val job = viewModelScope.launch {
@@ -377,7 +371,7 @@ class OnboardingViewModel @AssistedInject constructor(
                 setState {
                     copy(
                             isLoading = false,
-                            resetPasswordEmail = null
+                            resetState = ResetState()
                     )
                 }
             }
@@ -447,59 +441,52 @@ 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 {
+            runCatching { safeLoginWizard.resetPassword(action.email) }.fold(
-                    safeLoginWizard.resetPassword(action.email, action.newPassword)
+                    onSuccess = {
-                } catch (failure: Throwable) {
-                    setState { copy(isLoading = false) }
-                    _viewEvents.post(OnboardingViewEvents.Failure(failure))
-                    return@launch
-                }
-
                         setState {
                             copy(
                                     isLoading = false,
-                            resetPasswordEmail = action.email
+                                    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
+        setState { copy(isLoading = true) }
-
-        if (safeLoginWizard == null) {
-            setState { copy(isLoading = false) }
-            _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Bad configuration")))
-        } else {
-            setState { copy(isLoading = false) }
-
         currentJob = viewModelScope.launch {
-                try {
+            val resetState = awaitState().resetState
-                    safeLoginWizard.resetPasswordMailConfirmed()
+            when (val newPassword = resetState.newPassword) {
-                } catch (failure: Throwable) {
+                null -> {
                     setState { copy(isLoading = false) }
-                    _viewEvents.post(OnboardingViewEvents.Failure(failure))
+                    _viewEvents.post(OnboardingViewEvents.Failure(IllegalStateException("Developer error - No new password has been set")))
-                    return@launch
                 }
+                else -> {
+                    runCatching { loginWizard.resetPasswordMailConfirmed(newPassword) }.fold(
+                            onSuccess = {
                                 setState {
                                     copy(
                                             isLoading = false,
-                            resetPasswordEmail = null
+                                            resetState = ResetState()
                                     )
                                 }
-
                                 _viewEvents.post(OnboardingViewEvents.OnResetPasswordMailConfirmationSuccess)
+                            },
+                            onFailure = {
+                                setState { copy(isLoading = false) }
+                                _viewEvents.post(OnboardingViewEvents.Failure(it))
+                            }
+                    )
+                }
             }
         }
     }
@@ -519,11 +506,6 @@ 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 {
@@ -540,7 +522,6 @@ class OnboardingViewModel @AssistedInject constructor(
             }
         }
     }
-    }
 
     private fun startAuthenticationFlow() {
         // Ensure Wizard is ready

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,12 @@ data class PersonalizationState(
     fun supportsPersonalization() = supportsChangingDisplayName || supportsChangingProfilePicture
 }
 
+@Parcelize
+data class ResetState(
+        val email: String? = null,
+        val newPassword: String? = null,
+) : Parcelable
+
 @Parcelize
 data class SelectedAuthenticationState(
         val description: AuthenticationDescription? = null,

vector/src/main/java/im/vector/app/features/onboarding/ftueauth/AbstractFtueAuthFragment.kt

@@ -153,7 +153,7 @@ abstract class AbstractFtueAuthFragment<VB : ViewBinding> : VectorBaseFragment<V
 
     final override fun invalidate() = withState(viewModel) { state ->
         // True when email is sent with success to the homeserver
-        isResetPasswordStarted = state.resetPasswordEmail.isNullOrBlank().not()
+        isResetPasswordStarted = state.resetState.email.isNullOrBlank().not()
 
         updateWithState(state)
     }

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() {

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,

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
     }

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) }
+    }
+}