From d353e9314b906541a77ede7437997dce47f52a96 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Jun 2019 16:39:35 +0200 Subject: [PATCH 1/2] Crypto: Delete device --- .../matrix/android/api/failure/Failure.kt | 5 +- .../android/internal/auth/data/LoginFlow.kt | 14 ++- .../registration/RegistrationFlowResponse.kt | 53 +++++++++++ .../android/internal/crypto/CryptoModule.kt | 2 +- .../crypto/model/rest/DeleteDeviceAuth.kt | 5 +- .../crypto/model/rest/DeleteDeviceParams.kt | 4 +- .../internal/crypto/tasks/DeleteDeviceTask.kt | 90 +++++++++++++++++-- .../android/internal/network/Request.kt | 39 +++++--- .../VectorSettingsPreferencesFragment.kt | 10 +-- 9 files changed, 192 insertions(+), 30 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/Failure.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/Failure.kt index 8431239ac2..232a13d834 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/Failure.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/failure/Failure.kt @@ -31,7 +31,10 @@ import java.io.IOException sealed class Failure(cause: Throwable? = null) : Throwable(cause = cause) { data class Unknown(val throwable: Throwable? = null) : Failure(throwable) data class NetworkConnection(val ioException: IOException? = null) : Failure(ioException) - data class ServerError(val error: MatrixError) : Failure(RuntimeException(error.toString())) + data class ServerError(val error: MatrixError, val httpCode: Int) : Failure(RuntimeException(error.toString())) + // When server send an error, but it cannot be interpreted as a MatrixError + data class OtherServerError(val errorBody: String, val httpCode: Int) : Failure(RuntimeException(errorBody)) + data class CryptoError(val error: MXCryptoError) : Failure(RuntimeException(error.toString())) abstract class FeatureFailure : Failure() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlow.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlow.kt index fba7980bcc..a0b56d7aeb 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlow.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlow.kt @@ -16,8 +16,18 @@ package im.vector.matrix.android.internal.auth.data +import com.squareup.moshi.Json import com.squareup.moshi.JsonClass +/** + * A Login flow. + */ @JsonClass(generateAdapter = true) -internal data class LoginFlow(val type: String, - val stages: List) \ No newline at end of file +internal data class LoginFlow( + + @Json(name = "type") + val type: String? = null, + + @Json(name = "stages") + val stages: List +) \ No newline at end of file diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt new file mode 100644 index 0000000000..ac23fe1518 --- /dev/null +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt @@ -0,0 +1,53 @@ +/* + * Copyright 2019 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.matrix.android.internal.auth.registration + +import com.squareup.moshi.Json +import com.squareup.moshi.JsonClass +import im.vector.matrix.android.api.util.JsonDict +import im.vector.matrix.android.internal.auth.data.LoginFlow + +@JsonClass(generateAdapter = true) +internal data class RegistrationFlowResponse( + + /** + * The list of stages the client has completed successfully. + */ + @Json(name = "flows") + var loginFlows: List? = null, + + /** + * The list of stages the client has completed successfully. + */ + @Json(name = "completed") + var completedStages: List? = null, + + /** + * The session identifier that the client must pass back to the home server, if one is provided, + * in subsequent attempts to authenticate in the same API call. + */ + @Json(name = "session") + var session: String? = null, + + /** + * The information that the client will need to know in order to use a given type of authentication. + * For each login stage type presented, that type may be present as a key in this dictionary. + * For example, the public key of reCAPTCHA stage could be given here. + */ + @Json(name = "params") + var params: JsonDict? = null +) \ No newline at end of file diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoModule.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoModule.kt index 84029ba64b..6c7862095d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoModule.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoModule.kt @@ -227,7 +227,7 @@ internal class CryptoModule { DefaultClaimOneTimeKeysForUsersDevice(get()) as ClaimOneTimeKeysForUsersDeviceTask } scope(DefaultSession.SCOPE) { - DefaultDeleteDeviceTask(get()) as DeleteDeviceTask + DefaultDeleteDeviceTask(get(), get()) as DeleteDeviceTask } scope(DefaultSession.SCOPE) { DefaultDownloadKeysForUsers(get()) as DownloadKeysForUsersTask diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceAuth.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceAuth.kt index f50fbaba99..6a03d260e7 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceAuth.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceAuth.kt @@ -19,7 +19,7 @@ import com.squareup.moshi.Json import com.squareup.moshi.JsonClass /** - * This class provides the + * This class provides the authentication data to delete a device */ @JsonClass(generateAdapter = true) data class DeleteDeviceAuth( @@ -32,6 +32,9 @@ data class DeleteDeviceAuth( @Json(name = "type") var type: String? = null, + @Json(name = "user") var user: String? = null, + + @Json(name = "password") var password: String? = null ) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceParams.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceParams.kt index ba3ed9037c..c7e9836237 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceParams.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/model/rest/DeleteDeviceParams.kt @@ -15,6 +15,7 @@ */ package im.vector.matrix.android.internal.crypto.model.rest +import com.squareup.moshi.Json import com.squareup.moshi.JsonClass /** @@ -22,5 +23,6 @@ import com.squareup.moshi.JsonClass */ @JsonClass(generateAdapter = true) data class DeleteDeviceParams( - var auth: DeleteDeviceAuth? = null + @Json(name = "auth") + var deleteDeviceAuth: DeleteDeviceAuth? = null ) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt index 0fb9ad0f15..bbd2d440e4 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt @@ -17,10 +17,19 @@ package im.vector.matrix.android.internal.crypto.tasks import arrow.core.Try +import arrow.core.failure +import arrow.core.recoverWith +import im.vector.matrix.android.api.auth.data.Credentials +import im.vector.matrix.android.api.failure.Failure +import im.vector.matrix.android.api.failure.MatrixError +import im.vector.matrix.android.internal.auth.registration.RegistrationFlowResponse import im.vector.matrix.android.internal.crypto.api.CryptoApi +import im.vector.matrix.android.internal.crypto.model.rest.DeleteDeviceAuth import im.vector.matrix.android.internal.crypto.model.rest.DeleteDeviceParams +import im.vector.matrix.android.internal.di.MoshiProvider import im.vector.matrix.android.internal.network.executeRequest import im.vector.matrix.android.internal.task.Task +import timber.log.Timber internal interface DeleteDeviceTask : Task { data class Params( @@ -29,15 +38,84 @@ internal interface DeleteDeviceTask : Task { ) } -internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi) +internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, + private val credentials: Credentials) : DeleteDeviceTask { override suspend fun execute(params: DeleteDeviceTask.Params): Try { - return executeRequest { - apiCall = cryptoApi.deleteDevice(params.deviceId, - DeleteDeviceParams()) - } + return executeRequest { + apiCall = cryptoApi.deleteDevice(params.deviceId, DeleteDeviceParams()) + }.recoverWith { throwable -> + if (throwable is Failure.OtherServerError && throwable.httpCode == 401) { + // Replay the request with passing the credentials - // TODO Recover error, see legacy code MXSession.deleteDevice() + // Parse to get a RegistrationFlowResponse + val registrationFlowResponseAdapter = MoshiProvider.providesMoshi().adapter(RegistrationFlowResponse::class.java) + val registrationFlowResponse = try { + registrationFlowResponseAdapter.fromJson(throwable.errorBody) + } catch (e: Exception) { + null + } + + // check if the server response can be casted + if (registrationFlowResponse?.loginFlows?.isNotEmpty() == true) { + val stages = ArrayList() + + // Get all stages + registrationFlowResponse.loginFlows?.forEach { + stages.addAll(it.stages) + } + + Timber.v("## deleteDevice() : supported stages $stages") + + deleteDeviceRecursive(registrationFlowResponse.session, params, stages) + } else { + throwable.failure() + } + + } else { + // Other error + throwable.failure() + } + } + } + + private fun deleteDeviceRecursive(authSession: String?, + params: DeleteDeviceTask.Params, + remainingStages: MutableList): Try { + // Pick the first stage + val stage = remainingStages.first() + + val newParams = DeleteDeviceParams() + .apply { + deleteDeviceAuth = DeleteDeviceAuth() + .apply { + type = stage + session = authSession + user = credentials.userId + password = params.accountPassword + } + } + + return executeRequest { + apiCall = cryptoApi.deleteDevice(params.deviceId, newParams) + }.recoverWith { throwable -> + if (throwable is Failure.ServerError + && throwable.httpCode == 401 + && (throwable.error.code == MatrixError.FORBIDDEN || throwable.error.code == MatrixError.UNKNOWN)) { + if (remainingStages.size > 1) { + // Try next stage + val otherStages = remainingStages.subList(1, remainingStages.size) + + deleteDeviceRecursive(authSession, params, otherStages) + } else { + // No more stage remaining + throwable.failure() + } + } else { + // Other error + throwable.failure() + } + } } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/Request.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/Request.kt index f8a6bc5799..139d62154c 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/Request.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/Request.kt @@ -23,12 +23,14 @@ import arrow.effects.IO import arrow.effects.fix import arrow.effects.instances.io.async.async import arrow.integrations.retrofit.adapter.runAsync +import com.squareup.moshi.JsonDataException import com.squareup.moshi.Moshi import im.vector.matrix.android.api.failure.Failure import im.vector.matrix.android.api.failure.MatrixError import im.vector.matrix.android.internal.di.MoshiProvider import okhttp3.ResponseBody import retrofit2.Call +import timber.log.Timber import java.io.IOException internal inline fun executeRequest(block: Request.() -> Unit) = Request().apply(block).execute() @@ -44,23 +46,38 @@ internal class Request { if (response.isSuccessful) { response.body() ?: throw IllegalStateException("The request returned a null body") } else { - throw manageFailure(response.errorBody()) + throw manageFailure(response.errorBody(), response.code()) } }.recoverWith { when (it) { - is IOException -> Failure.NetworkConnection(it) - is Failure.ServerError -> it - else -> Failure.Unknown(it) + is IOException -> Failure.NetworkConnection(it) + is Failure.ServerError, + is Failure.OtherServerError -> it + else -> Failure.Unknown(it) }.failure() } } - private fun manageFailure(errorBody: ResponseBody?): Throwable { - val matrixError = errorBody?.let { - val matrixErrorAdapter = moshi.adapter(MatrixError::class.java) - matrixErrorAdapter.fromJson(errorBody.source()) - } ?: return RuntimeException("Matrix error should not be null") - return Failure.ServerError(matrixError) - } + private fun manageFailure(errorBody: ResponseBody?, httpCode: Int): Throwable { + if (errorBody == null) { + return RuntimeException("Error body should not be null") + } + val errorBodyStr = errorBody.string() + + val matrixErrorAdapter = moshi.adapter(MatrixError::class.java) + + try { + val matrixError = matrixErrorAdapter.fromJson(errorBodyStr) + + if (matrixError != null) { + return Failure.ServerError(matrixError, httpCode) + } + } catch (ex: JsonDataException) { + // This is not a MatrixError + Timber.w("The error returned by the server is not a MatrixError") + } + + return Failure.OtherServerError(errorBodyStr, httpCode) + } } \ No newline at end of file diff --git a/vector/src/main/java/im/vector/riotredesign/features/settings/VectorSettingsPreferencesFragment.kt b/vector/src/main/java/im/vector/riotredesign/features/settings/VectorSettingsPreferencesFragment.kt index a55b0f6e81..41f8ba4500 100755 --- a/vector/src/main/java/im/vector/riotredesign/features/settings/VectorSettingsPreferencesFragment.kt +++ b/vector/src/main/java/im/vector/riotredesign/features/settings/VectorSettingsPreferencesFragment.kt @@ -2493,15 +2493,12 @@ class VectorSettingsPreferencesFragment : VectorPreferenceFragment(), SharedPref * @param deviceId the device id */ private fun deleteDevice(deviceId: String) { - notImplemented() - - // We have to manage registration flow first, to handle what is necessary to delete a devive - /* displayLoadingView() - session.deleteDevice(deviceId, mAccountPassword, object : MatrixCallback { + mSession.deleteDevice(deviceId, mAccountPassword, object : MatrixCallback { override fun onSuccess(data: Unit) { hideLoadingView() - refreshDevicesList() // force settings update + // force settings update + refreshDevicesList() } override fun onFailure(failure: Throwable) { @@ -2509,7 +2506,6 @@ class VectorSettingsPreferencesFragment : VectorPreferenceFragment(), SharedPref onCommonDone(failure.localizedMessage) } }) - */ } /** From 8f6f72ca487e6f430b3e33a9edcd8bc6079d4e8f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Jun 2019 16:06:07 +0200 Subject: [PATCH 2/2] Review: Added optional to stage and renamed to "InteractiveAuthenticationFlow" --- .../data/{LoginFlow.kt => InteractiveAuthenticationFlow.kt} | 6 +++--- .../matrix/android/internal/auth/data/LoginFlowResponse.kt | 6 +++++- .../internal/auth/registration/RegistrationFlowResponse.kt | 6 +++--- .../android/internal/crypto/tasks/DeleteDeviceTask.kt | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-) rename matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/{LoginFlow.kt => InteractiveAuthenticationFlow.kt} (86%) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlow.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/InteractiveAuthenticationFlow.kt similarity index 86% rename from matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlow.kt rename to matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/InteractiveAuthenticationFlow.kt index a0b56d7aeb..c5d8dcc53a 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlow.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/InteractiveAuthenticationFlow.kt @@ -20,14 +20,14 @@ import com.squareup.moshi.Json import com.squareup.moshi.JsonClass /** - * A Login flow. + * An interactive authentication flow. */ @JsonClass(generateAdapter = true) -internal data class LoginFlow( +internal data class InteractiveAuthenticationFlow( @Json(name = "type") val type: String? = null, @Json(name = "stages") - val stages: List + val stages: List? = null ) \ No newline at end of file diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowResponse.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowResponse.kt index a5062181f4..834b0aee16 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowResponse.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowResponse.kt @@ -16,7 +16,11 @@ package im.vector.matrix.android.internal.auth.data +import com.squareup.moshi.Json import com.squareup.moshi.JsonClass @JsonClass(generateAdapter = true) -internal data class LoginFlowResponse(val flows: List) \ No newline at end of file +internal data class LoginFlowResponse( + @Json(name = "flows") + val flows: List +) \ No newline at end of file diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt index ac23fe1518..747db64c41 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/registration/RegistrationFlowResponse.kt @@ -19,16 +19,16 @@ package im.vector.matrix.android.internal.auth.registration import com.squareup.moshi.Json import com.squareup.moshi.JsonClass import im.vector.matrix.android.api.util.JsonDict -import im.vector.matrix.android.internal.auth.data.LoginFlow +import im.vector.matrix.android.internal.auth.data.InteractiveAuthenticationFlow @JsonClass(generateAdapter = true) internal data class RegistrationFlowResponse( /** - * The list of stages the client has completed successfully. + * The list of flows. */ @Json(name = "flows") - var loginFlows: List? = null, + var flows: List? = null, /** * The list of stages the client has completed successfully. diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt index bbd2d440e4..2b7649fdd4 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceTask.kt @@ -58,12 +58,12 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, } // check if the server response can be casted - if (registrationFlowResponse?.loginFlows?.isNotEmpty() == true) { + if (registrationFlowResponse?.flows?.isNotEmpty() == true) { val stages = ArrayList() // Get all stages - registrationFlowResponse.loginFlows?.forEach { - stages.addAll(it.stages) + registrationFlowResponse.flows?.forEach { + stages.addAll(it.stages ?: emptyList()) } Timber.v("## deleteDevice() : supported stages $stages")