From b44f5d3b4a8fda4eb647336268c4c256254cf4f3 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Sun, 10 May 2020 08:54:54 +0200 Subject: [PATCH] Handle correctly the verification code error case --- .../identity/db/IdentityServiceStore.kt | 4 ++ .../session/profile/BindThreePidBody.kt | 4 +- .../session/profile/BindThreePidsTask.kt | 14 ++-- .../session/profile/UnbindThreePidBody.kt | 2 +- .../session/profile/UnbindThreePidsTask.kt | 5 +- .../discovery/DiscoverySettingsController.kt | 25 ++++++- .../discovery/DiscoverySettingsViewModel.kt | 56 +++++++++++---- ...ngsItemText.kt => SettingsItemEditText.kt} | 14 +++- .../res/layout/item_settings_edit_text.xml | 72 +++++++++++-------- vector/src/main/res/values/strings.xml | 1 + 10 files changed, 140 insertions(+), 57 deletions(-) rename vector/src/main/java/im/vector/riotx/features/discovery/{SettingsItemText.kt => SettingsItemEditText.kt} (84%) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/db/IdentityServiceStore.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/db/IdentityServiceStore.kt index 5763806f02..7c2638ccdf 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/db/IdentityServiceStore.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/identity/db/IdentityServiceStore.kt @@ -40,3 +40,7 @@ internal interface IdentityServiceStore { fun deletePendingBinding(threePid: ThreePid) } + +internal fun IdentityServiceStore.getIdentityServerUrlWithoutProtocol(): String? { + return getIdentityServerDetails()?.identityServerUrl?.substringAfter("://") +} diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidBody.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidBody.kt index c6e6351d7a..6ba3ddda4a 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidBody.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidBody.kt @@ -30,13 +30,13 @@ internal data class BindThreePidBody( * Required. The identity server to use. (without "https://") */ @Json(name = "id_server") - var idServer: String, + var identityServerUrlWithoutProtocol: String, /** * Required. An access token previously registered with the identity server. */ @Json(name = "id_access_token") - var idAccessToken: String, + var identityServerAccessToken: String, /** * Required. The session identifier given by the identity server. diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidsTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidsTask.kt index 37b90567f5..93daf8f8ed 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidsTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/BindThreePidsTask.kt @@ -18,8 +18,11 @@ package im.vector.matrix.android.internal.session.profile import im.vector.matrix.android.api.session.identity.IdentityServiceError import im.vector.matrix.android.api.session.identity.ThreePid +import im.vector.matrix.android.internal.di.AuthenticatedIdentity import im.vector.matrix.android.internal.network.executeRequest +import im.vector.matrix.android.internal.network.token.AccessTokenProvider import im.vector.matrix.android.internal.session.identity.db.IdentityServiceStore +import im.vector.matrix.android.internal.session.identity.db.getIdentityServerUrlWithoutProtocol import im.vector.matrix.android.internal.task.Task import org.greenrobot.eventbus.EventBus import javax.inject.Inject @@ -32,18 +35,21 @@ internal abstract class BindThreePidsTask : Task internal class DefaultBindThreePidsTask @Inject constructor(private val profileAPI: ProfileAPI, private val identityServiceStore: IdentityServiceStore, + @AuthenticatedIdentity + private val accessTokenProvider: AccessTokenProvider, private val eventBus: EventBus) : BindThreePidsTask() { override suspend fun execute(params: Params) { - val idServer = identityServiceStore.getIdentityServerDetails()?.identityServerUrl?.substringAfter("://") ?: throw IdentityServiceError.NoIdentityServerConfigured - val idToken = identityServiceStore.getIdentityServerDetails()?.token ?: throw IdentityServiceError.NoIdentityServerConfigured + val identityServerUrlWithoutProtocol = identityServiceStore.getIdentityServerUrlWithoutProtocol() + ?: throw IdentityServiceError.NoIdentityServerConfigured + val identityServerAccessToken = accessTokenProvider.getToken() ?: throw IdentityServiceError.NoIdentityServerConfigured val pendingThreePid = identityServiceStore.getPendingBinding(params.threePid) ?: throw IdentityServiceError.NoCurrentBindingError executeRequest(eventBus) { apiCall = profileAPI.bindThreePid( BindThreePidBody( clientSecret = pendingThreePid.clientSecret, - idServer = idServer, - idAccessToken = idToken, + identityServerUrlWithoutProtocol = identityServerUrlWithoutProtocol, + identityServerAccessToken = identityServerAccessToken, sid = pendingThreePid.sid )) } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidBody.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidBody.kt index 226fd111b7..705569ba87 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidBody.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidBody.kt @@ -25,7 +25,7 @@ internal data class UnbindThreePidBody( * If the homeserver does not know the original id_server, it MUST return a id_server_unbind_result of no-support. */ @Json(name = "id_server") - val idServer: String?, + val identityServerUrlWithoutProtocol: String?, /** * Required. The medium of the third party identifier being removed. One of: ["email", "msisdn"] diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidsTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidsTask.kt index f708c104b0..0f84eb9926 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidsTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/profile/UnbindThreePidsTask.kt @@ -21,6 +21,7 @@ import im.vector.matrix.android.api.session.identity.ThreePid import im.vector.matrix.android.api.session.identity.toMedium import im.vector.matrix.android.internal.network.executeRequest import im.vector.matrix.android.internal.session.identity.db.IdentityServiceStore +import im.vector.matrix.android.internal.session.identity.db.getIdentityServerUrlWithoutProtocol import im.vector.matrix.android.internal.task.Task import org.greenrobot.eventbus.EventBus import javax.inject.Inject @@ -35,12 +36,12 @@ internal class DefaultUnbindThreePidsTask @Inject constructor(private val profil private val identityServiceStore: IdentityServiceStore, private val eventBus: EventBus) : UnbindThreePidsTask() { override suspend fun execute(params: Params): Boolean { - val idServer = identityServiceStore.getIdentityServerDetails()?.identityServerUrl?.substringAfter("://") ?: throw IdentityServiceError.NoIdentityServerConfigured + val identityServerUrlWithoutProtocol = identityServiceStore.getIdentityServerUrlWithoutProtocol() ?: throw IdentityServiceError.NoIdentityServerConfigured return executeRequest(eventBus) { apiCall = profileAPI.unbindThreePid( UnbindThreePidBody( - idServer, + identityServerUrlWithoutProtocol, params.threePid.toMedium(), params.threePid.value )) diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsController.kt b/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsController.kt index c8a16b37a2..30c571ed08 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsController.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsController.kt @@ -22,18 +22,22 @@ import com.airbnb.mvrx.Incomplete import com.airbnb.mvrx.Loading import com.airbnb.mvrx.Success import com.google.i18n.phonenumbers.PhoneNumberUtil +import im.vector.matrix.android.api.failure.Failure import im.vector.matrix.android.api.session.identity.SharedState import im.vector.matrix.android.api.session.identity.ThreePid import im.vector.riotx.R import im.vector.riotx.core.epoxy.loadingItem +import im.vector.riotx.core.error.ErrorFormatter import im.vector.riotx.core.resources.ColorProvider import im.vector.riotx.core.resources.StringProvider import timber.log.Timber import javax.inject.Inject +import javax.net.ssl.HttpsURLConnection class DiscoverySettingsController @Inject constructor( private val colorProvider: ColorProvider, - private val stringProvider: StringProvider + private val stringProvider: StringProvider, + private val errorFormatter: ErrorFormatter ) : TypedEpoxyController() { var listener: Listener? = null @@ -139,10 +143,25 @@ class DiscoverySettingsController @Inject constructor( } when (piState.isShared()) { SharedState.BINDING_IN_PROGRESS -> { - settingsItemText { + val errorText = if (piState.isTokenSubmitted is Fail) { + val error = piState.isTokenSubmitted.error + // Deal with error 500 + //Ref: https://github.com/matrix-org/sydent/issues/292 + if (error is Failure.ServerError + && error.httpCode == HttpsURLConnection.HTTP_INTERNAL_ERROR /* 500 */) { + stringProvider.getString(R.string.settings_text_message_sent_wrong_code) + } else { + errorFormatter.toHumanReadable(error) + } + } else { + null + } + settingsItemEditText { id("tverif" + piState.threePid.value) descriptionText(stringProvider.getString(R.string.settings_text_message_sent, phoneNumber)) - interactionListener(object : SettingsItemText.Listener { + errorText(errorText) + inProgress(piState.isTokenSubmitted is Loading) + interactionListener(object : SettingsItemEditText.Listener { override fun onValidate(code: String) { if (piState.threePid is ThreePid.Msisdn) { listener?.sendMsisdnVerificationCode(piState.threePid, code) diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt b/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt index a680b8ce17..dbb54b6dce 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/DiscoverySettingsViewModel.kt @@ -42,7 +42,9 @@ data class PidInfo( // Retrieved from the homeserver val threePid: ThreePid, // Retrieved from IdentityServer, or transient state - val isShared: Async + val isShared: Async, + // Contains information about a current request to submit the token (for instance SMS code received by SMS) + val isTokenSubmitted: Async = Uninitialized ) data class DiscoverySettingsState( @@ -181,15 +183,16 @@ class DiscoverySettingsViewModel @AssistedInject constructor( setState { val currentMails = emailList() ?: emptyList() val phones = phoneNumbersList() ?: emptyList() - copy(emailList = Success( - currentMails.map { - if (it.threePid == threePid) { - it.copy(isShared = state) - } else { - it - } - } - ), + copy( + emailList = Success( + currentMails.map { + if (it.threePid == threePid) { + it.copy(isShared = state) + } else { + it + } + } + ), phoneNumbersList = Success( phones.map { if (it.threePid == threePid) { @@ -203,6 +206,33 @@ class DiscoverySettingsViewModel @AssistedInject constructor( } } + private fun changeThreePidSubmitState(threePid: ThreePid, submitState: Async) { + setState { + val currentMails = emailList() ?: emptyList() + val phones = phoneNumbersList() ?: emptyList() + copy( + emailList = Success( + currentMails.map { + if (it.threePid == threePid) { + it.copy(isTokenSubmitted = submitState) + } else { + it + } + } + ), + phoneNumbersList = Success( + phones.map { + if (it.threePid == threePid) { + it.copy(isTokenSubmitted = submitState) + } else { + it + } + } + ) + ) + } + } + private fun revokeThreePid(action: DiscoverySettingsAction.RevokeThreePid) { when (action.threePid) { is ThreePid.Email -> revokeEmail(action.threePid) @@ -312,16 +342,18 @@ class DiscoverySettingsViewModel @AssistedInject constructor( private fun submitMsisdnToken(action: DiscoverySettingsAction.SubmitMsisdnToken) = withState { state -> if (state.identityServer().isNullOrBlank()) return@withState + changeThreePidSubmitState(action.threePid, Loading()) + identityService.submitValidationToken(action.threePid, action.code, object : MatrixCallback { override fun onSuccess(data: Unit) { + changeThreePidSubmitState(action.threePid, Uninitialized) finalizeBind3pid(DiscoverySettingsAction.FinalizeBind3pid(action.threePid)) } override fun onFailure(failure: Throwable) { - _viewEvents.post(DiscoverySettingsViewEvents.Failure(failure)) - changeThreePidState(action.threePid, Fail(failure)) + changeThreePidSubmitState(action.threePid, Fail(failure)) } } ) diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/SettingsItemText.kt b/vector/src/main/java/im/vector/riotx/features/discovery/SettingsItemEditText.kt similarity index 84% rename from vector/src/main/java/im/vector/riotx/features/discovery/SettingsItemText.kt rename to vector/src/main/java/im/vector/riotx/features/discovery/SettingsItemEditText.kt index 67860535f5..16a4b3ebcc 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/SettingsItemText.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/SettingsItemEditText.kt @@ -15,10 +15,13 @@ */ package im.vector.riotx.features.discovery +import android.view.View import android.view.inputmethod.EditorInfo import android.widget.Button import android.widget.EditText import android.widget.TextView +import androidx.core.view.isInvisible +import androidx.core.view.isVisible import com.airbnb.epoxy.EpoxyAttribute import com.airbnb.epoxy.EpoxyModelClass import com.airbnb.epoxy.EpoxyModelWithHolder @@ -28,10 +31,11 @@ import im.vector.riotx.core.epoxy.VectorEpoxyHolder import im.vector.riotx.core.extensions.setTextOrHide @EpoxyModelClass(layout = R.layout.item_settings_edit_text) -abstract class SettingsItemText : EpoxyModelWithHolder() { +abstract class SettingsItemEditText : EpoxyModelWithHolder() { @EpoxyAttribute var descriptionText: String? = null @EpoxyAttribute var errorText: String? = null + @EpoxyAttribute var inProgress: Boolean = false @EpoxyAttribute var interactionListener: Listener? = null @@ -42,21 +46,24 @@ abstract class SettingsItemText : EpoxyModelWithHolder( holder.validateButton.setOnClickListener { val code = holder.editText.text.toString() - holder.editText.text.clear() interactionListener?.onValidate(code) } + holder.editText.isEnabled = !inProgress + if (errorText.isNullOrBlank()) { holder.textInputLayout.error = null } else { holder.textInputLayout.error = errorText } + holder.validateButton.isInvisible = inProgress + holder.progress.isVisible = inProgress + holder.editText.setOnEditorActionListener { tv, actionId, _ -> if (actionId == EditorInfo.IME_ACTION_DONE) { val code = tv.text.toString() interactionListener?.onValidate(code) - holder.editText.text.clear() return@setOnEditorActionListener true } return@setOnEditorActionListener false @@ -68,6 +75,7 @@ abstract class SettingsItemText : EpoxyModelWithHolder( val editText by bind(R.id.settings_item_edittext) val textInputLayout by bind(R.id.settings_item_enter_til) val validateButton by bind