Handle correctly the verification code error case

This commit is contained in:
Benoit Marty 2020-05-10 08:54:54 +02:00
parent 03f8b66993
commit b44f5d3b4a
10 changed files with 140 additions and 57 deletions

View file

@ -40,3 +40,7 @@ internal interface IdentityServiceStore {
fun deletePendingBinding(threePid: ThreePid) fun deletePendingBinding(threePid: ThreePid)
} }
internal fun IdentityServiceStore.getIdentityServerUrlWithoutProtocol(): String? {
return getIdentityServerDetails()?.identityServerUrl?.substringAfter("://")
}

View file

@ -30,13 +30,13 @@ internal data class BindThreePidBody(
* Required. The identity server to use. (without "https://") * Required. The identity server to use. (without "https://")
*/ */
@Json(name = "id_server") @Json(name = "id_server")
var idServer: String, var identityServerUrlWithoutProtocol: String,
/** /**
* Required. An access token previously registered with the identity server. * Required. An access token previously registered with the identity server.
*/ */
@Json(name = "id_access_token") @Json(name = "id_access_token")
var idAccessToken: String, var identityServerAccessToken: String,
/** /**
* Required. The session identifier given by the identity server. * Required. The session identifier given by the identity server.

View file

@ -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.IdentityServiceError
import im.vector.matrix.android.api.session.identity.ThreePid 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.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.IdentityServiceStore
import im.vector.matrix.android.internal.session.identity.db.getIdentityServerUrlWithoutProtocol
import im.vector.matrix.android.internal.task.Task import im.vector.matrix.android.internal.task.Task
import org.greenrobot.eventbus.EventBus import org.greenrobot.eventbus.EventBus
import javax.inject.Inject import javax.inject.Inject
@ -32,18 +35,21 @@ internal abstract class BindThreePidsTask : Task<BindThreePidsTask.Params, Unit>
internal class DefaultBindThreePidsTask @Inject constructor(private val profileAPI: ProfileAPI, internal class DefaultBindThreePidsTask @Inject constructor(private val profileAPI: ProfileAPI,
private val identityServiceStore: IdentityServiceStore, private val identityServiceStore: IdentityServiceStore,
@AuthenticatedIdentity
private val accessTokenProvider: AccessTokenProvider,
private val eventBus: EventBus) : BindThreePidsTask() { private val eventBus: EventBus) : BindThreePidsTask() {
override suspend fun execute(params: Params) { override suspend fun execute(params: Params) {
val idServer = identityServiceStore.getIdentityServerDetails()?.identityServerUrl?.substringAfter("://") ?: throw IdentityServiceError.NoIdentityServerConfigured val identityServerUrlWithoutProtocol = identityServiceStore.getIdentityServerUrlWithoutProtocol()
val idToken = identityServiceStore.getIdentityServerDetails()?.token ?: throw IdentityServiceError.NoIdentityServerConfigured ?: throw IdentityServiceError.NoIdentityServerConfigured
val identityServerAccessToken = accessTokenProvider.getToken() ?: throw IdentityServiceError.NoIdentityServerConfigured
val pendingThreePid = identityServiceStore.getPendingBinding(params.threePid) ?: throw IdentityServiceError.NoCurrentBindingError val pendingThreePid = identityServiceStore.getPendingBinding(params.threePid) ?: throw IdentityServiceError.NoCurrentBindingError
executeRequest<Unit>(eventBus) { executeRequest<Unit>(eventBus) {
apiCall = profileAPI.bindThreePid( apiCall = profileAPI.bindThreePid(
BindThreePidBody( BindThreePidBody(
clientSecret = pendingThreePid.clientSecret, clientSecret = pendingThreePid.clientSecret,
idServer = idServer, identityServerUrlWithoutProtocol = identityServerUrlWithoutProtocol,
idAccessToken = idToken, identityServerAccessToken = identityServerAccessToken,
sid = pendingThreePid.sid sid = pendingThreePid.sid
)) ))
} }

View file

@ -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. * 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") @Json(name = "id_server")
val idServer: String?, val identityServerUrlWithoutProtocol: String?,
/** /**
* Required. The medium of the third party identifier being removed. One of: ["email", "msisdn"] * Required. The medium of the third party identifier being removed. One of: ["email", "msisdn"]

View file

@ -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.api.session.identity.toMedium
import im.vector.matrix.android.internal.network.executeRequest 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.IdentityServiceStore
import im.vector.matrix.android.internal.session.identity.db.getIdentityServerUrlWithoutProtocol
import im.vector.matrix.android.internal.task.Task import im.vector.matrix.android.internal.task.Task
import org.greenrobot.eventbus.EventBus import org.greenrobot.eventbus.EventBus
import javax.inject.Inject import javax.inject.Inject
@ -35,12 +36,12 @@ internal class DefaultUnbindThreePidsTask @Inject constructor(private val profil
private val identityServiceStore: IdentityServiceStore, private val identityServiceStore: IdentityServiceStore,
private val eventBus: EventBus) : UnbindThreePidsTask() { private val eventBus: EventBus) : UnbindThreePidsTask() {
override suspend fun execute(params: Params): Boolean { 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<UnbindThreePidResponse>(eventBus) { return executeRequest<UnbindThreePidResponse>(eventBus) {
apiCall = profileAPI.unbindThreePid( apiCall = profileAPI.unbindThreePid(
UnbindThreePidBody( UnbindThreePidBody(
idServer, identityServerUrlWithoutProtocol,
params.threePid.toMedium(), params.threePid.toMedium(),
params.threePid.value params.threePid.value
)) ))

View file

@ -22,18 +22,22 @@ import com.airbnb.mvrx.Incomplete
import com.airbnb.mvrx.Loading import com.airbnb.mvrx.Loading
import com.airbnb.mvrx.Success import com.airbnb.mvrx.Success
import com.google.i18n.phonenumbers.PhoneNumberUtil 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.SharedState
import im.vector.matrix.android.api.session.identity.ThreePid import im.vector.matrix.android.api.session.identity.ThreePid
import im.vector.riotx.R import im.vector.riotx.R
import im.vector.riotx.core.epoxy.loadingItem 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.ColorProvider
import im.vector.riotx.core.resources.StringProvider import im.vector.riotx.core.resources.StringProvider
import timber.log.Timber import timber.log.Timber
import javax.inject.Inject import javax.inject.Inject
import javax.net.ssl.HttpsURLConnection
class DiscoverySettingsController @Inject constructor( class DiscoverySettingsController @Inject constructor(
private val colorProvider: ColorProvider, private val colorProvider: ColorProvider,
private val stringProvider: StringProvider private val stringProvider: StringProvider,
private val errorFormatter: ErrorFormatter
) : TypedEpoxyController<DiscoverySettingsState>() { ) : TypedEpoxyController<DiscoverySettingsState>() {
var listener: Listener? = null var listener: Listener? = null
@ -139,10 +143,25 @@ class DiscoverySettingsController @Inject constructor(
} }
when (piState.isShared()) { when (piState.isShared()) {
SharedState.BINDING_IN_PROGRESS -> { 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) id("tverif" + piState.threePid.value)
descriptionText(stringProvider.getString(R.string.settings_text_message_sent, phoneNumber)) 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) { override fun onValidate(code: String) {
if (piState.threePid is ThreePid.Msisdn) { if (piState.threePid is ThreePid.Msisdn) {
listener?.sendMsisdnVerificationCode(piState.threePid, code) listener?.sendMsisdnVerificationCode(piState.threePid, code)

View file

@ -42,7 +42,9 @@ data class PidInfo(
// Retrieved from the homeserver // Retrieved from the homeserver
val threePid: ThreePid, val threePid: ThreePid,
// Retrieved from IdentityServer, or transient state // Retrieved from IdentityServer, or transient state
val isShared: Async<SharedState> val isShared: Async<SharedState>,
// Contains information about a current request to submit the token (for instance SMS code received by SMS)
val isTokenSubmitted: Async<Unit> = Uninitialized
) )
data class DiscoverySettingsState( data class DiscoverySettingsState(
@ -181,15 +183,16 @@ class DiscoverySettingsViewModel @AssistedInject constructor(
setState { setState {
val currentMails = emailList() ?: emptyList() val currentMails = emailList() ?: emptyList()
val phones = phoneNumbersList() ?: emptyList() val phones = phoneNumbersList() ?: emptyList()
copy(emailList = Success( copy(
currentMails.map { emailList = Success(
if (it.threePid == threePid) { currentMails.map {
it.copy(isShared = state) if (it.threePid == threePid) {
} else { it.copy(isShared = state)
it } else {
} it
} }
), }
),
phoneNumbersList = Success( phoneNumbersList = Success(
phones.map { phones.map {
if (it.threePid == threePid) { if (it.threePid == threePid) {
@ -203,6 +206,33 @@ class DiscoverySettingsViewModel @AssistedInject constructor(
} }
} }
private fun changeThreePidSubmitState(threePid: ThreePid, submitState: Async<Unit>) {
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) { private fun revokeThreePid(action: DiscoverySettingsAction.RevokeThreePid) {
when (action.threePid) { when (action.threePid) {
is ThreePid.Email -> revokeEmail(action.threePid) is ThreePid.Email -> revokeEmail(action.threePid)
@ -312,16 +342,18 @@ class DiscoverySettingsViewModel @AssistedInject constructor(
private fun submitMsisdnToken(action: DiscoverySettingsAction.SubmitMsisdnToken) = withState { state -> private fun submitMsisdnToken(action: DiscoverySettingsAction.SubmitMsisdnToken) = withState { state ->
if (state.identityServer().isNullOrBlank()) return@withState if (state.identityServer().isNullOrBlank()) return@withState
changeThreePidSubmitState(action.threePid, Loading())
identityService.submitValidationToken(action.threePid, identityService.submitValidationToken(action.threePid,
action.code, action.code,
object : MatrixCallback<Unit> { object : MatrixCallback<Unit> {
override fun onSuccess(data: Unit) { override fun onSuccess(data: Unit) {
changeThreePidSubmitState(action.threePid, Uninitialized)
finalizeBind3pid(DiscoverySettingsAction.FinalizeBind3pid(action.threePid)) finalizeBind3pid(DiscoverySettingsAction.FinalizeBind3pid(action.threePid))
} }
override fun onFailure(failure: Throwable) { override fun onFailure(failure: Throwable) {
_viewEvents.post(DiscoverySettingsViewEvents.Failure(failure)) changeThreePidSubmitState(action.threePid, Fail(failure))
changeThreePidState(action.threePid, Fail(failure))
} }
} }
) )

View file

@ -15,10 +15,13 @@
*/ */
package im.vector.riotx.features.discovery package im.vector.riotx.features.discovery
import android.view.View
import android.view.inputmethod.EditorInfo import android.view.inputmethod.EditorInfo
import android.widget.Button import android.widget.Button
import android.widget.EditText import android.widget.EditText
import android.widget.TextView import android.widget.TextView
import androidx.core.view.isInvisible
import androidx.core.view.isVisible
import com.airbnb.epoxy.EpoxyAttribute import com.airbnb.epoxy.EpoxyAttribute
import com.airbnb.epoxy.EpoxyModelClass import com.airbnb.epoxy.EpoxyModelClass
import com.airbnb.epoxy.EpoxyModelWithHolder import com.airbnb.epoxy.EpoxyModelWithHolder
@ -28,10 +31,11 @@ import im.vector.riotx.core.epoxy.VectorEpoxyHolder
import im.vector.riotx.core.extensions.setTextOrHide import im.vector.riotx.core.extensions.setTextOrHide
@EpoxyModelClass(layout = R.layout.item_settings_edit_text) @EpoxyModelClass(layout = R.layout.item_settings_edit_text)
abstract class SettingsItemText : EpoxyModelWithHolder<SettingsItemText.Holder>() { abstract class SettingsItemEditText : EpoxyModelWithHolder<SettingsItemEditText.Holder>() {
@EpoxyAttribute var descriptionText: String? = null @EpoxyAttribute var descriptionText: String? = null
@EpoxyAttribute var errorText: String? = null @EpoxyAttribute var errorText: String? = null
@EpoxyAttribute var inProgress: Boolean = false
@EpoxyAttribute @EpoxyAttribute
var interactionListener: Listener? = null var interactionListener: Listener? = null
@ -42,21 +46,24 @@ abstract class SettingsItemText : EpoxyModelWithHolder<SettingsItemText.Holder>(
holder.validateButton.setOnClickListener { holder.validateButton.setOnClickListener {
val code = holder.editText.text.toString() val code = holder.editText.text.toString()
holder.editText.text.clear()
interactionListener?.onValidate(code) interactionListener?.onValidate(code)
} }
holder.editText.isEnabled = !inProgress
if (errorText.isNullOrBlank()) { if (errorText.isNullOrBlank()) {
holder.textInputLayout.error = null holder.textInputLayout.error = null
} else { } else {
holder.textInputLayout.error = errorText holder.textInputLayout.error = errorText
} }
holder.validateButton.isInvisible = inProgress
holder.progress.isVisible = inProgress
holder.editText.setOnEditorActionListener { tv, actionId, _ -> holder.editText.setOnEditorActionListener { tv, actionId, _ ->
if (actionId == EditorInfo.IME_ACTION_DONE) { if (actionId == EditorInfo.IME_ACTION_DONE) {
val code = tv.text.toString() val code = tv.text.toString()
interactionListener?.onValidate(code) interactionListener?.onValidate(code)
holder.editText.text.clear()
return@setOnEditorActionListener true return@setOnEditorActionListener true
} }
return@setOnEditorActionListener false return@setOnEditorActionListener false
@ -68,6 +75,7 @@ abstract class SettingsItemText : EpoxyModelWithHolder<SettingsItemText.Holder>(
val editText by bind<EditText>(R.id.settings_item_edittext) val editText by bind<EditText>(R.id.settings_item_edittext)
val textInputLayout by bind<TextInputLayout>(R.id.settings_item_enter_til) val textInputLayout by bind<TextInputLayout>(R.id.settings_item_enter_til)
val validateButton by bind<Button>(R.id.settings_item_enter_button) val validateButton by bind<Button>(R.id.settings_item_enter_button)
val progress by bind<View>(R.id.settings_item_enter_progress)
} }
interface Listener { interface Listener {

View file

@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?> <?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" <androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools" xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent" android:layout_width="match_parent"
@ -10,39 +10,39 @@
android:paddingEnd="@dimen/layout_horizontal_margin" android:paddingEnd="@dimen/layout_horizontal_margin"
android:paddingBottom="@dimen/layout_vertical_margin"> android:paddingBottom="@dimen/layout_vertical_margin">
<LinearLayout <TextView
android:id="@+id/settings_item_description"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:orientation="vertical"
android:textColor="?android:textColorSecondary"
android:textSize="15sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
tools:text="@string/settings_text_message_sent" />
<com.google.android.material.textfield.TextInputLayout
android:id="@+id/settings_item_enter_til"
style="@style/VectorTextInputLayout"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:orientation="vertical"> app:errorEnabled="true"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/settings_item_description">
<TextView <com.google.android.material.textfield.TextInputEditText
android:id="@+id/settings_item_description" android:id="@+id/settings_item_edittext"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:orientation="vertical" android:imeOptions="actionDone|flagNoPersonalizedLearning"
android:textColor="?android:textColorSecondary" android:inputType="numberDecimal"
android:textSize="15sp" android:maxLines="1"
tools:text="@string/settings_text_message_sent" /> android:textColor="?android:textColorPrimary"
tools:text="1234" />
<com.google.android.material.textfield.TextInputLayout </com.google.android.material.textfield.TextInputLayout>
android:id="@+id/settings_item_enter_til"
style="@style/VectorTextInputLayout"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:errorEnabled="true">
<com.google.android.material.textfield.TextInputEditText
android:id="@+id/settings_item_edittext"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:imeOptions="actionDone|flagNoPersonalizedLearning"
android:inputType="numberDecimal"
android:maxLines="1"
android:textColor="?android:textColorPrimary"
tools:text="1234" />
</com.google.android.material.textfield.TextInputLayout>
</LinearLayout>
<Button <Button
android:id="@+id/settings_item_enter_button" android:id="@+id/settings_item_enter_button"
@ -50,6 +50,18 @@
android:layout_width="wrap_content" android:layout_width="wrap_content"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_gravity="end" android:layout_gravity="end"
android:text="@string/_continue" /> android:text="@string/_continue"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toBottomOf="@+id/settings_item_enter_til" />
</LinearLayout> <ProgressBar
android:id="@+id/settings_item_enter_progress"
style="?android:attr/progressBarStyle"
android:layout_width="20dp"
android:layout_height="20dp"
app:layout_constraintBottom_toBottomOf="@+id/settings_item_enter_button"
app:layout_constraintEnd_toEndOf="@+id/settings_item_enter_button"
app:layout_constraintStart_toStartOf="@+id/settings_item_enter_button"
app:layout_constraintTop_toTopOf="@+id/settings_item_enter_button" />
</androidx.constraintlayout.widget.ConstraintLayout>

View file

@ -1741,6 +1741,7 @@ Not all features in Riot are implemented in RiotX yet. Main missing (and coming
<string name="settings_discovery_no_terms_title">Identity server has no terms of services</string> <string name="settings_discovery_no_terms_title">Identity server has no terms of services</string>
<string name="settings_discovery_no_terms">The identity server you have chosen does not have any terms of services. Only continue if you trust the owner of the service</string> <string name="settings_discovery_no_terms">The identity server you have chosen does not have any terms of services. Only continue if you trust the owner of the service</string>
<string name="settings_text_message_sent">A text message has been sent to %s. Please enter the verification code it contains.</string> <string name="settings_text_message_sent">A text message has been sent to %s. Please enter the verification code it contains.</string>
<string name="settings_text_message_sent_wrong_code">The verification code is not correct.</string>
<string name="settings_discovery_disconnect_with_bound_pid">You are currently sharing email addresses or phone numbers on the identity server %1$s. You will need to reconnect to %2$s to stop sharing them.</string> <string name="settings_discovery_disconnect_with_bound_pid">You are currently sharing email addresses or phone numbers on the identity server %1$s. You will need to reconnect to %2$s to stop sharing them.</string>
<string name="settings_agree_to_terms">Agree to the identity server (%s) Terms of Service to allow yourself to be discoverable by email address or phone number.</string> <string name="settings_agree_to_terms">Agree to the identity server (%s) Terms of Service to allow yourself to be discoverable by email address or phone number.</string>