First commit to cleanup ReAuthHelper and it's usage

Also add some comment and do some other cleanup

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSigningKeysTask.kt

@@ -16,6 +16,7 @@
 
 package im.vector.matrix.android.internal.crypto.tasks
 
+import im.vector.matrix.android.api.auth.data.LoginFlowTypes
 import im.vector.matrix.android.api.failure.Failure
 import im.vector.matrix.android.api.failure.toRegistrationFlowResponse
 import im.vector.matrix.android.internal.crypto.api.CryptoApi
@@ -37,6 +38,15 @@ internal interface UploadSigningKeysTask : Task<UploadSigningKeysTask.Params, Un
             val userKey: CryptoCrossSigningKey,
             // the explicit device_id to use for upload (default is to use the same as that used during auth).
             val selfSignedKey: CryptoCrossSigningKey,
+            /**
+             * - If null:
+             *    - no retry will be performed
+             * - If not null, it may or may not contain a sessionId:
+             *    - If sessionId is null:
+             *       - password should not be null: the task will perform a first request to get a sessionId, and then a second one
+             *    - If sessionId is not null:
+             *       - password should not be null as well, and no retry will be performed
+             */
             val userPasswordAuth: UserPasswordAuth?
     )
 }
@@ -47,42 +57,41 @@ internal class DefaultUploadSigningKeysTask @Inject constructor(
         private val cryptoApi: CryptoApi,
         private val eventBus: EventBus
 ) : UploadSigningKeysTask {
+
     override suspend fun execute(params: UploadSigningKeysTask.Params) {
+        val paramsHaveSessionId = params.userPasswordAuth?.session != null
+
         val uploadQuery = UploadSigningKeysBody(
                 masterKey = params.masterKey.toRest(),
                 userSigningKey = params.userKey.toRest(),
                 selfSigningKey = params.selfSignedKey.toRest(),
-                auth = params.userPasswordAuth.takeIf { params.userPasswordAuth?.session != null }
+                // If sessionId is provided, use the userPasswordAuth
+                auth = params.userPasswordAuth.takeIf { paramsHaveSessionId }
         )
         try {
-            // Make a first request to start user-interactive authentication
+            doRequest(uploadQuery)
-            val request = executeRequest<KeysQueryResponse>(eventBus) {
-                apiCall = cryptoApi.uploadSigningKeys(uploadQuery)
-            }
-            if (request.failures?.isNotEmpty() == true) {
-                throw UploadSigningKeys(request.failures)
-            }
-            return
         } catch (throwable: Throwable) {
             val registrationFlowResponse = throwable.toRegistrationFlowResponse()
             if (registrationFlowResponse != null
-                    && params.userPasswordAuth != null
+                    && registrationFlowResponse.flows.orEmpty().any { it.stages?.contains(LoginFlowTypes.PASSWORD) == true }
-                    /* Avoid infinite loop */
+                    && params.userPasswordAuth?.password != null
-                    && params.userPasswordAuth.session.isNullOrEmpty()
+                    && !paramsHaveSessionId
             ) {
                 // Retry with authentication
-                val req = executeRequest<KeysQueryResponse>(eventBus) {
+                doRequest(uploadQuery.copy(auth = params.userPasswordAuth.copy(session = registrationFlowResponse.session)))
-                    apiCall = cryptoApi.uploadSigningKeys(
-                            uploadQuery.copy(auth = params.userPasswordAuth.copy(session = registrationFlowResponse.session))
-                    )
-                }
-                if (req.failures?.isNotEmpty() == true) {
-                    throw UploadSigningKeys(req.failures)
-                }
             } else {
                 // Other error
                 throw throwable
             }
         }
     }
+
+    private suspend fun doRequest(uploadQuery: UploadSigningKeysBody) {
+        val keysQueryResponse = executeRequest<KeysQueryResponse>(eventBus) {
+            apiCall = cryptoApi.uploadSigningKeys(uploadQuery)
+        }
+        if (keysQueryResponse.failures?.isNotEmpty() == true) {
+            throw UploadSigningKeys(keysQueryResponse.failures)
+        }
+    }
 }

vector/src/androidTest/java/im/vector/riotx/core/utils/TemporaryStoreTest.kt

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2020 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.riotx.core.utils
+
+import org.amshove.kluent.shouldBe
+import org.junit.Test
+import java.lang.Thread.sleep
+
+class TemporaryStoreTest {
+
+    @Test
+    fun testTemporaryStore() {
+        // Keep the data 30 millis
+        val store = TemporaryStore<String>(30)
+
+        store.data = "test"
+        store.data shouldBe "test"
+        sleep(15)
+        store.data shouldBe "test"
+        sleep(20)
+        store.data shouldBe null
+    }
+}

vector/src/main/java/im/vector/riotx/core/utils/TemporaryStore.kt

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2020 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.riotx.core.utils
+
+import java.util.Timer
+import java.util.TimerTask
+
+const val THREE_MINUTES = 3 * 60_000L
+
+/**
+ * Store an object T for a specific period of time
+ */
+open class TemporaryStore<T>(private val delay: Long = THREE_MINUTES) {
+
+    private var timer: Timer? = null
+
+    var data: T? = null
+        set(value) {
+            field = value
+            timer?.cancel()
+            timer = Timer().also {
+                it.schedule(object : TimerTask() {
+                    override fun run() {
+                        field = null
+                    }
+                }, delay)
+            }
+        }
+}

vector/src/main/java/im/vector/riotx/features/crypto/recover/BackupToQuadSMigrationTask.kt

@@ -97,7 +97,7 @@ class BackupToQuadSMigrationTask @Inject constructor(
                     when {
                         params.passphrase?.isNotEmpty() == true -> {
                             reportProgress(params, R.string.bootstrap_progress_generating_ssss)
-                            awaitCallback {
+                            awaitCallback<SsssKeyCreationInfo> {
                                 quadS.generateKeyWithPassphrase(
                                         UUID.randomUUID().toString(),
                                         "ssss_key",

vector/src/main/java/im/vector/riotx/features/crypto/recover/BootstrapActions.kt

@@ -16,7 +16,6 @@
 
 package im.vector.riotx.features.crypto.recover
 
-import im.vector.matrix.android.internal.crypto.model.rest.UserPasswordAuth
 import im.vector.riotx.core.platform.VectorViewModelAction
 import java.io.OutputStream
 
@@ -29,8 +28,8 @@ sealed class BootstrapActions : VectorViewModelAction {
     object GoToCompleted : BootstrapActions()
     object GoToEnterAccountPassword : BootstrapActions()
 
-    data class DoInitialize(val passphrase: String, val auth: UserPasswordAuth? = null) : BootstrapActions()
+    data class DoInitialize(val passphrase: String) : BootstrapActions()
-    data class DoInitializeGeneratedKey(val auth: UserPasswordAuth? = null) : BootstrapActions()
+    object DoInitializeGeneratedKey : BootstrapActions()
     object TogglePasswordVisibility : BootstrapActions()
     data class UpdateCandidatePassphrase(val pass: String) : BootstrapActions()
     data class UpdateConfirmCandidatePassphrase(val pass: String) : BootstrapActions()

vector/src/main/java/im/vector/riotx/features/crypto/recover/BootstrapBottomSheet.kt

@@ -90,7 +90,7 @@ class BootstrapBottomSheet : VectorBaseBottomSheetDialogFragment() {
                 .apply {
                     if (genKeyOption) {
                         setNeutralButton(R.string.generate_message_key) { _, _ ->
-                            viewModel.handle(BootstrapActions.DoInitializeGeneratedKey())
+                            viewModel.handle(BootstrapActions.DoInitializeGeneratedKey)
                         }
                     }
                 }

vector/src/main/java/im/vector/riotx/features/crypto/recover/BootstrapCrossSigningTask.kt

@@ -58,7 +58,7 @@ sealed class BootstrapResult {
     class FailedToStorePrivateKeyInSSSS(failure: Throwable) : Failure(failure.localizedMessage)
     object MissingPrivateKey : Failure(null)
 
-    data class PasswordAuthFlowMissing(val sessionId: String, val userId: String) : Failure(null)
+    data class PasswordAuthFlowMissing(val sessionId: String) : Failure(null)
 }
 
 interface BootstrapProgressListener {
@@ -232,9 +232,11 @@ class BootstrapCrossSigningTask @Inject constructor(
         } else {
             val registrationFlowResponse = failure.toRegistrationFlowResponse()
             if (registrationFlowResponse != null) {
-                if (registrationFlowResponse.flows?.any { it.stages?.contains(LoginFlowTypes.PASSWORD) == true } != true) {
+                return if (registrationFlowResponse.flows.orEmpty().any { it.stages?.contains(LoginFlowTypes.PASSWORD) == true }) {
+                    BootstrapResult.PasswordAuthFlowMissing(registrationFlowResponse.session ?: "")
+                } else {
                     // can't do this from here
-                    return BootstrapResult.UnsupportedAuthFlow()
+                    BootstrapResult.UnsupportedAuthFlow()
                 }
             }
         }

vector/src/main/java/im/vector/riotx/features/crypto/recover/BootstrapSharedViewModel.kt

@@ -30,6 +30,7 @@ import com.nulabinc.zxcvbn.Strength
 import com.nulabinc.zxcvbn.Zxcvbn
 import com.squareup.inject.assisted.Assisted
 import com.squareup.inject.assisted.AssistedInject
+import im.vector.matrix.android.api.failure.Failure
 import im.vector.matrix.android.api.session.Session
 import im.vector.matrix.android.api.session.securestorage.RawBytesKeySpec
 import im.vector.matrix.android.api.session.securestorage.SsssKeyCreationInfo
@@ -57,7 +58,6 @@ data class BootstrapViewState(
         val passphraseConfirmMatch: Async<Unit> = Uninitialized,
         val recoveryKeyCreationInfo: SsssKeyCreationInfo? = null,
         val initializationWaitingViewData: WaitingViewData? = null,
-        val currentReAuth: UserPasswordAuth? = null,
         val recoverySaveFileProcess: Async<Unit> = Uninitialized
 ) : MvRxState
 
@@ -78,6 +78,8 @@ class BootstrapSharedViewModel @AssistedInject constructor(
         fun create(initialState: BootstrapViewState, args: BootstrapBottomSheet.Args): BootstrapSharedViewModel
     }
 
+    private var _pendingSession: String? = null
+
     init {
         // need to check if user have an existing keybackup
         if (args.isNewAccount) {
@@ -182,15 +184,15 @@ class BootstrapSharedViewModel @AssistedInject constructor(
             }
             is BootstrapActions.DoInitialize                     -> {
                 if (state.passphrase == state.passphraseRepeat) {
-                    val auth = action.auth ?: reAuthHelper.rememberedAuth()
+                    val userPassword = reAuthHelper.data
-                    if (auth == null) {
+                    if (userPassword == null) {
                         setState {
                             copy(
                                     step = BootstrapStep.AccountPassword(false)
                             )
                         }
                     } else {
-                        startInitializeFlow(action.auth)
+                        startInitializeFlow(userPassword)
                     }
                 } else {
                     setState {
@@ -201,8 +203,8 @@ class BootstrapSharedViewModel @AssistedInject constructor(
                 }
             }
             is BootstrapActions.DoInitializeGeneratedKey         -> {
-                val auth = action.auth ?: reAuthHelper.rememberedAuth()
+                val userPassword = reAuthHelper.data
-                if (auth == null) {
+                if (userPassword == null) {
                     setState {
                         copy(
                                 passphrase = null,
@@ -217,7 +219,7 @@ class BootstrapSharedViewModel @AssistedInject constructor(
                                 passphraseRepeat = null
                         )
                     }
-                    startInitializeFlow(action.auth)
+                    startInitializeFlow(userPassword)
                 }
             }
             BootstrapActions.RecoveryKeySaved                    -> {
@@ -260,10 +262,7 @@ class BootstrapSharedViewModel @AssistedInject constructor(
                 } else return@withState
             }
             is BootstrapActions.ReAuth                           -> {
-                startInitializeFlow(
+                startInitializeFlow(action.pass)
-                        state.currentReAuth?.copy(password = action.pass)
-                                ?: UserPasswordAuth(user = session.myUserId, password = action.pass)
-                )
             }
             is BootstrapActions.DoMigrateWithPassphrase          -> {
                 startMigrationFlow(state.step, action.passphrase, null)
@@ -322,15 +321,15 @@ class BootstrapSharedViewModel @AssistedInject constructor(
                                 migrationRecoveryKey = recoveryKey
                         )
                     }
-                    val auth = reAuthHelper.rememberedAuth()
+                    val userPassword = reAuthHelper.data
-                    if (auth == null) {
+                    if (userPassword == null) {
                         setState {
                             copy(
                                     step = BootstrapStep.AccountPassword(false)
                             )
                         }
                     } else {
-                        startInitializeFlow(auth)
+                        startInitializeFlow(userPassword)
                     }
                 } else {
                     _viewEvents.post(
@@ -350,7 +349,7 @@ class BootstrapSharedViewModel @AssistedInject constructor(
         }
     }
 
-    private fun startInitializeFlow(auth: UserPasswordAuth?) {
+    private fun startInitializeFlow(userPassword: String?) {
         setState {
             copy(step = BootstrapStep.Initializing)
         }
@@ -367,25 +366,37 @@ class BootstrapSharedViewModel @AssistedInject constructor(
 
         withState { state ->
             viewModelScope.launch(Dispatchers.IO) {
-                bootstrapTask.invoke(this, Params(
+                val userPasswordAuth = userPassword?.let {
-                        userPasswordAuth = auth ?: reAuthHelper.rememberedAuth(),
+                    UserPasswordAuth(
-                        progressListener = progressListener,
+                            // Note that _pendingSession may or may not be null, this is OK, it will be managed by the task
-                        passphrase = state.passphrase,
+                            session = _pendingSession,
-                        keySpec = state.migrationRecoveryKey?.let { extractCurveKeyFromRecoveryKey(it)?.let { RawBytesKeySpec(it) } }
+                            user = session.myUserId,
-                )) {
+                            password = it
-                    when (it) {
+                    )
+                }
+
+                bootstrapTask.invoke(this,
+                        Params(
+                                userPasswordAuth = userPasswordAuth,
+                                progressListener = progressListener,
+                                passphrase = state.passphrase,
+                                keySpec = state.migrationRecoveryKey?.let { extractCurveKeyFromRecoveryKey(it)?.let { RawBytesKeySpec(it) } }
+                        )
+                ) { bootstrapResult ->
+                    when (bootstrapResult) {
                         is BootstrapResult.Success                 -> {
                             setState {
                                 copy(
-                                        recoveryKeyCreationInfo = it.keyInfo,
+                                        recoveryKeyCreationInfo = bootstrapResult.keyInfo,
                                         step = BootstrapStep.SaveRecoveryKey(false)
                                 )
                             }
                         }
                         is BootstrapResult.PasswordAuthFlowMissing -> {
+                            // Ask the password to the user
+                            _pendingSession = bootstrapResult.sessionId
                             setState {
                                 copy(
-                                        currentReAuth = UserPasswordAuth(session = it.sessionId, user = it.userId),
                                         step = BootstrapStep.AccountPassword(false)
                                 )
                             }
@@ -396,20 +407,20 @@ class BootstrapSharedViewModel @AssistedInject constructor(
                         }
                         is BootstrapResult.InvalidPasswordError    -> {
                             // it's a bad password
+                            // We clear the auth session, to avoid 'Requested operation has changed during the UI authentication session' error
+                            _pendingSession = null
                             setState {
                                 copy(
-                                        // We clear the auth session, to avoid 'Requested operation has changed during the UI authentication session' error
-                                        currentReAuth = UserPasswordAuth(session = null, user = session.myUserId),
                                         step = BootstrapStep.AccountPassword(false, stringProvider.getString(R.string.auth_invalid_login_param))
                                 )
                             }
                         }
                         is BootstrapResult.Failure                 -> {
-                            if (it is BootstrapResult.GenericError
+                            if (bootstrapResult is BootstrapResult.GenericError
-                                    && it.failure is im.vector.matrix.android.api.failure.Failure.OtherServerError
+                                    && bootstrapResult.failure is Failure.OtherServerError
-                                    && it.failure.httpCode == 401) {
+                                    && bootstrapResult.failure.httpCode == 401) {
                             } else {
-                                _viewEvents.post(BootstrapViewEvents.ModalError(it.error ?: stringProvider.getString(R.string.matrix_error)))
+                                _viewEvents.post(BootstrapViewEvents.ModalError(bootstrapResult.error ?: stringProvider.getString(R.string.matrix_error)))
                                 setState {
                                     copy(
                                             step = BootstrapStep.ConfirmPassphrase(false)

vector/src/main/java/im/vector/riotx/features/login/LoginViewModel.kt

@@ -41,7 +41,6 @@ import im.vector.matrix.android.api.auth.registration.Stage
 import im.vector.matrix.android.api.auth.wellknown.WellknownResult
 import im.vector.matrix.android.api.session.Session
 import im.vector.matrix.android.api.util.Cancelable
-import im.vector.matrix.android.internal.crypto.model.rest.UserPasswordAuth
 import im.vector.riotx.R
 import im.vector.riotx.core.di.ActiveSessionHolder
 import im.vector.riotx.core.extensions.configureAndStart
@@ -289,7 +288,7 @@ class LoginViewModel @AssistedInject constructor(
 
     private fun handleRegisterWith(action: LoginAction.LoginOrRegister) {
         setState { copy(asyncRegistration = Loading()) }
-        reAuthHelper.rememberAuth(UserPasswordAuth(user = action.username, password = action.password))
+        reAuthHelper.data = action.password
         currentTask = registrationWizard?.createAccount(
                 action.username,
                 action.password,

vector/src/main/java/im/vector/riotx/features/login/ReAuthHelper.kt

@@ -16,33 +16,12 @@
 
 package im.vector.riotx.features.login
 
-import im.vector.matrix.android.internal.crypto.model.rest.UserPasswordAuth
+import im.vector.riotx.core.utils.TemporaryStore
-import java.util.Timer
-import java.util.TimerTask
 import javax.inject.Inject
 import javax.inject.Singleton
 
-const val THREE_MINUTES = 3 * 60_000L
+/**
-
+ * Will store the account password for 3 minutes
+ */
 @Singleton
-class ReAuthHelper @Inject constructor() {
+class ReAuthHelper @Inject constructor() : TemporaryStore<String>()
-
-    private var timer: Timer? = null
-
-    private var rememberedInfo: UserPasswordAuth? = null
-
-    fun rememberAuth(password: UserPasswordAuth?) {
-        timer?.cancel()
-        timer = null
-        rememberedInfo = password
-        timer = Timer().apply {
-            schedule(object : TimerTask() {
-                override fun run() {
-                    rememberedInfo = null
-                }
-            }, THREE_MINUTES)
-        }
-    }
-
-    fun rememberedAuth() = rememberedInfo
-}