From 1547045165478d577cf8357e5fd5d46ad7ee1097 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 12 Jun 2019 14:49:39 +0200 Subject: [PATCH 1/3] Request can now be canceled properly: it should fix the issue with live chunk being deleted. --- .../android/internal/network/Request.kt | 41 ++++++++++++------- .../session/group/DefaultGetGroupDataTask.kt | 31 +++++++------- .../room/relation/SendRelationWorker.kt | 5 ++- .../session/room/send/RedactEventWorker.kt | 5 ++- .../session/room/send/SendEventWorker.kt | 5 ++- 5 files changed, 49 insertions(+), 38 deletions(-) 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 139d62154c..bbd5859b21 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 @@ -28,34 +28,45 @@ 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 kotlinx.coroutines.suspendCancellableCoroutine import okhttp3.ResponseBody import retrofit2.Call import timber.log.Timber import java.io.IOException +import kotlin.coroutines.resume -internal inline fun executeRequest(block: Request.() -> Unit) = Request().apply(block).execute() +internal suspend inline fun executeRequest(block: Request.() -> Unit) = Request().apply(block).execute() internal class Request { private val moshi: Moshi = MoshiProvider.providesMoshi() lateinit var apiCall: Call - fun execute(): Try { - return Try { - val response = apiCall.runAsync(IO.async()).fix().unsafeRunSync() - if (response.isSuccessful) { - response.body() ?: throw IllegalStateException("The request returned a null body") - } else { - throw manageFailure(response.errorBody(), response.code()) + suspend fun execute(): Try { + return suspendCancellableCoroutine { continuation -> + continuation.invokeOnCancellation { + Timber.v("Request is canceled") + apiCall.cancel() } - }.recoverWith { - when (it) { - is IOException -> Failure.NetworkConnection(it) - is Failure.ServerError, - is Failure.OtherServerError -> it - else -> Failure.Unknown(it) - }.failure() + val result = Try { + val response = apiCall.runAsync(IO.async()).fix().unsafeRunSync() + if (response.isSuccessful) { + response.body() + ?: throw IllegalStateException("The request returned a null body") + } else { + throw manageFailure(response.errorBody(), response.code()) + } + }.recoverWith { + when (it) { + is IOException -> Failure.NetworkConnection(it) + is Failure.ServerError, + is Failure.OtherServerError -> it + else -> Failure.Unknown(it) + }.failure() + } + continuation.resume(result) } + } private fun manageFailure(errorBody: ResponseBody?, httpCode: Int): Throwable { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/group/DefaultGetGroupDataTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/group/DefaultGetGroupDataTask.kt index 0785c33472..3d547c5342 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/group/DefaultGetGroupDataTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/group/DefaultGetGroupDataTask.kt @@ -21,13 +21,13 @@ import arrow.core.fix import arrow.instances.`try`.monad.monad import arrow.typeclasses.binding import com.zhuinden.monarchy.Monarchy -import im.vector.matrix.android.internal.task.Task import im.vector.matrix.android.internal.database.model.GroupSummaryEntity import im.vector.matrix.android.internal.database.query.where import im.vector.matrix.android.internal.network.executeRequest import im.vector.matrix.android.internal.session.group.model.GroupRooms import im.vector.matrix.android.internal.session.group.model.GroupSummaryResponse import im.vector.matrix.android.internal.session.group.model.GroupUsers +import im.vector.matrix.android.internal.task.Task import im.vector.matrix.android.internal.util.tryTransactionSync import io.realm.kotlin.createObject @@ -45,21 +45,17 @@ internal class DefaultGetGroupDataTask( override suspend fun execute(params: GetGroupDataTask.Params): Try { val groupId = params.groupId + val groupSummary = executeRequest { + apiCall = groupAPI.getSummary(groupId) + } + val groupRooms = executeRequest { + apiCall = groupAPI.getRooms(groupId) + } + val groupUsers = executeRequest { + apiCall = groupAPI.getUsers(groupId) + } return Try.monad().binding { - - val groupSummary = executeRequest { - apiCall = groupAPI.getSummary(groupId) - }.bind() - - val groupRooms = executeRequest { - apiCall = groupAPI.getRooms(groupId) - }.bind() - - val groupUsers = executeRequest { - apiCall = groupAPI.getUsers(groupId) - }.bind() - - insertInDb(groupSummary, groupRooms, groupUsers, groupId).bind() + insertInDb(groupSummary.bind(), groupRooms.bind(), groupUsers.bind(), groupId).bind() }.fix() } @@ -71,12 +67,13 @@ internal class DefaultGetGroupDataTask( return monarchy .tryTransactionSync { realm -> val groupSummaryEntity = GroupSummaryEntity.where(realm, groupId).findFirst() - ?: realm.createObject(groupId) + ?: realm.createObject(groupId) groupSummaryEntity.avatarUrl = groupSummary.profile?.avatarUrl ?: "" val name = groupSummary.profile?.name groupSummaryEntity.displayName = if (name.isNullOrEmpty()) groupId else name - groupSummaryEntity.shortDescription = groupSummary.profile?.shortDescription ?: "" + groupSummaryEntity.shortDescription = groupSummary.profile?.shortDescription + ?: "" val roomIds = groupRooms.rooms.map { it.roomId } groupSummaryEntity.roomIds.clear() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/SendRelationWorker.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/SendRelationWorker.kt index 41184aaf14..824c85b8a4 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/SendRelationWorker.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/SendRelationWorker.kt @@ -16,6 +16,7 @@ package im.vector.matrix.android.internal.session.room.relation import android.content.Context +import androidx.work.CoroutineWorker import androidx.work.Worker import androidx.work.WorkerParameters import com.squareup.moshi.JsonClass @@ -32,7 +33,7 @@ import im.vector.matrix.android.internal.util.WorkerParamsFactory import org.koin.standalone.inject class SendRelationWorker(context: Context, params: WorkerParameters) - : Worker(context, params), MatrixKoinComponent { + : CoroutineWorker(context, params), MatrixKoinComponent { @JsonClass(generateAdapter = true) @@ -44,7 +45,7 @@ class SendRelationWorker(context: Context, params: WorkerParameters) private val roomAPI by inject() - override fun doWork(): Result { + override suspend fun doWork(): Result { val params = WorkerParamsFactory.fromData(inputData) ?: return Result.failure() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/RedactEventWorker.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/RedactEventWorker.kt index 5056a93b08..e51889f4fb 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/RedactEventWorker.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/RedactEventWorker.kt @@ -16,6 +16,7 @@ package im.vector.matrix.android.internal.session.room.send import android.content.Context +import androidx.work.CoroutineWorker import androidx.work.Worker import androidx.work.WorkerParameters import com.squareup.moshi.JsonClass @@ -27,7 +28,7 @@ import im.vector.matrix.android.internal.util.WorkerParamsFactory import org.koin.standalone.inject internal class RedactEventWorker(context: Context, params: WorkerParameters) - : Worker(context, params), MatrixKoinComponent { + : CoroutineWorker(context, params), MatrixKoinComponent { @JsonClass(generateAdapter = true) internal data class Params( @@ -39,7 +40,7 @@ internal class RedactEventWorker(context: Context, params: WorkerParameters) private val roomAPI by inject() - override fun doWork(): Result { + override suspend fun doWork(): Result { val params = WorkerParamsFactory.fromData(inputData) ?: return Result.failure() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/SendEventWorker.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/SendEventWorker.kt index 32239b53a8..3b2fd87b16 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/SendEventWorker.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/SendEventWorker.kt @@ -17,6 +17,7 @@ package im.vector.matrix.android.internal.session.room.send import android.content.Context +import androidx.work.CoroutineWorker import androidx.work.Worker import androidx.work.WorkerParameters import com.squareup.moshi.JsonClass @@ -30,7 +31,7 @@ import im.vector.matrix.android.internal.util.WorkerParamsFactory import org.koin.standalone.inject internal class SendEventWorker(context: Context, params: WorkerParameters) - : Worker(context, params), MatrixKoinComponent { + : CoroutineWorker(context, params), MatrixKoinComponent { @JsonClass(generateAdapter = true) @@ -42,7 +43,7 @@ internal class SendEventWorker(context: Context, params: WorkerParameters) private val roomAPI by inject() private val localEchoUpdater by inject() - override fun doWork(): Result { + override suspend fun doWork(): Result { val params = WorkerParamsFactory.fromData(inputData) ?: return Result.success() From 9649e190efcba57195b70ca19316cf31664f4ea4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 17 Jun 2019 16:28:27 +0200 Subject: [PATCH 2/3] Fix compilation issue after rebase --- .../internal/crypto/tasks/DeleteDeviceTask.kt | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) 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 2b7649fdd4..2222c470cc 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 @@ -29,7 +29,10 @@ 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 kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.launch import timber.log.Timber +import java.util.concurrent.CountDownLatch internal interface DeleteDeviceTask : Task { data class Params( @@ -50,9 +53,10 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, // Replay the request with passing the credentials // Parse to get a RegistrationFlowResponse - val registrationFlowResponseAdapter = MoshiProvider.providesMoshi().adapter(RegistrationFlowResponse::class.java) val registrationFlowResponse = try { - registrationFlowResponseAdapter.fromJson(throwable.errorBody) + MoshiProvider.providesMoshi() + .adapter(RegistrationFlowResponse::class.java) + .fromJson(throwable.errorBody) } catch (e: Exception) { null } @@ -68,7 +72,16 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, Timber.v("## deleteDevice() : supported stages $stages") - deleteDeviceRecursive(registrationFlowResponse.session, params, stages) + val latch = CountDownLatch(1) + var deleteDeviceRecursiveResult: Try = Try.just(Unit) + + GlobalScope.launch { + deleteDeviceRecursiveResult = deleteDeviceRecursive(registrationFlowResponse.session, params, stages) + latch.countDown() + } + + latch.await() + deleteDeviceRecursiveResult } else { throwable.failure() } @@ -80,9 +93,9 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, } } - private fun deleteDeviceRecursive(authSession: String?, - params: DeleteDeviceTask.Params, - remainingStages: MutableList): Try { + private suspend fun deleteDeviceRecursive(authSession: String?, + params: DeleteDeviceTask.Params, + remainingStages: MutableList): Try { // Pick the first stage val stage = remainingStages.first() @@ -107,7 +120,16 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, // Try next stage val otherStages = remainingStages.subList(1, remainingStages.size) - deleteDeviceRecursive(authSession, params, otherStages) + val latch = CountDownLatch(1) + var deleteDeviceRecursiveResult: Try = Try.just(Unit) + + GlobalScope.launch { + deleteDeviceRecursiveResult = deleteDeviceRecursive(authSession, params, otherStages) + latch.countDown() + } + + latch.await() + deleteDeviceRecursiveResult } else { // No more stage remaining throwable.failure() From 6266f9e6a13d4ac37749f75c937b5b4b092dd909 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 17 Jun 2019 17:32:35 +0200 Subject: [PATCH 3/3] Handle device deletion the proper way --- .../matrix/android/api/failure/Failure.kt | 3 + .../api/session/crypto/CryptoService.kt | 4 +- .../data/InteractiveAuthenticationFlow.kt | 2 +- .../internal/auth/data/LoginFlowTypes.kt | 2 +- .../registration/RegistrationFlowResponse.kt | 2 +- .../android/internal/crypto/CryptoManager.kt | 13 +- .../android/internal/crypto/CryptoModule.kt | 6 +- .../crypto/model/rest/DeleteDeviceAuth.kt | 2 +- .../crypto/model/rest/DeleteDeviceParams.kt | 2 +- .../internal/crypto/tasks/DeleteDeviceTask.kt | 85 +----------- .../tasks/DeleteDeviceWithUserPasswordTask.kt | 54 ++++++++ .../internal/session/DefaultSession.kt | 8 +- .../VectorSettingsPreferencesFragment.kt | 128 +++++++++++------- 13 files changed, 172 insertions(+), 139 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceWithUserPasswordTask.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 232a13d834..593c33b933 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 @@ -17,6 +17,7 @@ package im.vector.matrix.android.api.failure import im.vector.matrix.android.api.session.crypto.MXCryptoError +import im.vector.matrix.android.internal.auth.registration.RegistrationFlowResponse import java.io.IOException /** @@ -35,6 +36,8 @@ sealed class Failure(cause: Throwable? = null) : Throwable(cause = cause) { // 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 RegistrationFlowError(val registrationFlowResponse: RegistrationFlowResponse) : Failure(RuntimeException(registrationFlowResponse.toString())) + 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/api/session/crypto/CryptoService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/CryptoService.kt index e7c3c445ee..0f97841362 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/CryptoService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/CryptoService.kt @@ -36,7 +36,9 @@ interface CryptoService { fun setDeviceName(deviceId: String, deviceName: String, callback: MatrixCallback) - fun deleteDevice(deviceId: String, accountPassword: String, callback: MatrixCallback) + fun deleteDevice(deviceId: String, callback: MatrixCallback) + + fun deleteDeviceWithUserPassword(deviceId: String, authSession: String?, password: String, callback: MatrixCallback) fun getCryptoVersion(context: Context, longFormat: Boolean): String diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/InteractiveAuthenticationFlow.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/InteractiveAuthenticationFlow.kt index c5d8dcc53a..ae75b2737d 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/InteractiveAuthenticationFlow.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/InteractiveAuthenticationFlow.kt @@ -23,7 +23,7 @@ import com.squareup.moshi.JsonClass * An interactive authentication flow. */ @JsonClass(generateAdapter = true) -internal data class InteractiveAuthenticationFlow( +data class InteractiveAuthenticationFlow( @Json(name = "type") val type: String? = null, diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowTypes.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowTypes.kt index 2cf450d2cb..1e129f1b45 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowTypes.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/auth/data/LoginFlowTypes.kt @@ -16,7 +16,7 @@ package im.vector.matrix.android.internal.auth.data -internal object LoginFlowTypes { +object LoginFlowTypes { const val PASSWORD = "m.login.password" const val OAUTH2 = "m.login.oauth2" const val EMAIL_CODE = "m.login.email.code" 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 747db64c41..0eb7b05bf8 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 @@ -22,7 +22,7 @@ import im.vector.matrix.android.api.util.JsonDict import im.vector.matrix.android.internal.auth.data.InteractiveAuthenticationFlow @JsonClass(generateAdapter = true) -internal data class RegistrationFlowResponse( +data class RegistrationFlowResponse( /** * The list of flows. diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoManager.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoManager.kt index 8b29287d70..5e422571b5 100755 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoManager.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/CryptoManager.kt @@ -55,6 +55,7 @@ import im.vector.matrix.android.internal.crypto.model.rest.KeysUploadResponse import im.vector.matrix.android.internal.crypto.model.rest.RoomKeyRequestBody import im.vector.matrix.android.internal.crypto.repository.WarnOnUnknownDeviceRepository import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore +import im.vector.matrix.android.internal.crypto.tasks.* import im.vector.matrix.android.internal.crypto.tasks.DeleteDeviceTask import im.vector.matrix.android.internal.crypto.tasks.GetDevicesTask import im.vector.matrix.android.internal.crypto.tasks.SetDeviceNameTask @@ -124,6 +125,7 @@ internal class CryptoManager( private val megolmEncryptionFactory: MXMegolmEncryptionFactory, private val olmEncryptionFactory: MXOlmEncryptionFactory, private val deleteDeviceTask: DeleteDeviceTask, + private val deleteDeviceWithUserPasswordTask: DeleteDeviceWithUserPasswordTask, // Tasks private val getDevicesTask: GetDevicesTask, private val setDeviceNameTask: SetDeviceNameTask, @@ -163,9 +165,16 @@ internal class CryptoManager( .executeBy(taskExecutor) } - override fun deleteDevice(deviceId: String, accountPassword: String, callback: MatrixCallback) { + override fun deleteDevice(deviceId: String, callback: MatrixCallback) { deleteDeviceTask - .configureWith(DeleteDeviceTask.Params(deviceId, accountPassword)) + .configureWith(DeleteDeviceTask.Params(deviceId)) + .dispatchTo(callback) + .executeBy(taskExecutor) + } + + override fun deleteDeviceWithUserPassword(deviceId: String, authSession: String?, password: String, callback: MatrixCallback) { + deleteDeviceWithUserPasswordTask + .configureWith(DeleteDeviceWithUserPasswordTask.Params(deviceId, authSession, password)) .dispatchTo(callback) .executeBy(taskExecutor) } 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 6c7862095d..761318ace6 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 @@ -193,6 +193,7 @@ internal class CryptoModule { megolmEncryptionFactory = get(), olmEncryptionFactory = get(), deleteDeviceTask = get(), + deleteDeviceWithUserPasswordTask = get(), // Tasks getDevicesTask = get(), setDeviceNameTask = get(), @@ -227,7 +228,10 @@ internal class CryptoModule { DefaultClaimOneTimeKeysForUsersDevice(get()) as ClaimOneTimeKeysForUsersDeviceTask } scope(DefaultSession.SCOPE) { - DefaultDeleteDeviceTask(get(), get()) as DeleteDeviceTask + DefaultDeleteDeviceTask(get()) as DeleteDeviceTask + } + scope(DefaultSession.SCOPE) { + DefaultDeleteDeviceWithUserPasswordTask(get(), get()) as DeleteDeviceWithUserPasswordTask } 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 6a03d260e7..53ba4179eb 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 @@ -22,7 +22,7 @@ import com.squareup.moshi.JsonClass * This class provides the authentication data to delete a device */ @JsonClass(generateAdapter = true) -data class DeleteDeviceAuth( +internal data class DeleteDeviceAuth( // device device session id @Json(name = "session") 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 c7e9836237..f823de2eb3 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 @@ -22,7 +22,7 @@ import com.squareup.moshi.JsonClass * This class provides the parameter to delete a device */ @JsonClass(generateAdapter = true) -data class DeleteDeviceParams( +internal data class DeleteDeviceParams( @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 2222c470cc..6bdb72d8d2 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 @@ -19,30 +19,21 @@ 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 kotlinx.coroutines.GlobalScope -import kotlinx.coroutines.launch -import timber.log.Timber -import java.util.concurrent.CountDownLatch internal interface DeleteDeviceTask : Task { data class Params( - val deviceId: String, - val accountPassword: String + val deviceId: String ) } -internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, - private val credentials: Credentials) +internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi) : DeleteDeviceTask { override suspend fun execute(params: DeleteDeviceTask.Params): Try { @@ -50,8 +41,6 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, apiCall = cryptoApi.deleteDevice(params.deviceId, DeleteDeviceParams()) }.recoverWith { throwable -> if (throwable is Failure.OtherServerError && throwable.httpCode == 401) { - // Replay the request with passing the credentials - // Parse to get a RegistrationFlowResponse val registrationFlowResponse = try { MoshiProvider.providesMoshi() @@ -62,26 +51,8 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, } // check if the server response can be casted - if (registrationFlowResponse?.flows?.isNotEmpty() == true) { - val stages = ArrayList() - - // Get all stages - registrationFlowResponse.flows?.forEach { - stages.addAll(it.stages ?: emptyList()) - } - - Timber.v("## deleteDevice() : supported stages $stages") - - val latch = CountDownLatch(1) - var deleteDeviceRecursiveResult: Try = Try.just(Unit) - - GlobalScope.launch { - deleteDeviceRecursiveResult = deleteDeviceRecursive(registrationFlowResponse.session, params, stages) - latch.countDown() - } - - latch.await() - deleteDeviceRecursiveResult + if (registrationFlowResponse != null) { + Failure.RegistrationFlowError(registrationFlowResponse).failure() } else { throwable.failure() } @@ -92,52 +63,4 @@ internal class DefaultDeleteDeviceTask(private val cryptoApi: CryptoApi, } } } - - private suspend 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) - - val latch = CountDownLatch(1) - var deleteDeviceRecursiveResult: Try = Try.just(Unit) - - GlobalScope.launch { - deleteDeviceRecursiveResult = deleteDeviceRecursive(authSession, params, otherStages) - latch.countDown() - } - - latch.await() - deleteDeviceRecursiveResult - } 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/crypto/tasks/DeleteDeviceWithUserPasswordTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceWithUserPasswordTask.kt new file mode 100644 index 0000000000..7b6d1dfe13 --- /dev/null +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/DeleteDeviceWithUserPasswordTask.kt @@ -0,0 +1,54 @@ +/* + * 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.crypto.tasks + +import arrow.core.Try +import im.vector.matrix.android.api.auth.data.Credentials +import im.vector.matrix.android.internal.auth.data.LoginFlowTypes +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.network.executeRequest +import im.vector.matrix.android.internal.task.Task + +internal interface DeleteDeviceWithUserPasswordTask : Task { + data class Params( + val deviceId: String, + val authSession: String?, + val password: String + ) +} + +internal class DefaultDeleteDeviceWithUserPasswordTask(private val cryptoApi: CryptoApi, + private val credentials: Credentials) + : DeleteDeviceWithUserPasswordTask { + + override suspend fun execute(params: DeleteDeviceWithUserPasswordTask.Params): Try { + return executeRequest { + apiCall = cryptoApi.deleteDevice(params.deviceId, DeleteDeviceParams() + .apply { + deleteDeviceAuth = DeleteDeviceAuth() + .apply { + type = LoginFlowTypes.PASSWORD + session = params.authSession + user = credentials.userId + password = params.password + } + }) + } + } +} diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/DefaultSession.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/DefaultSession.kt index 747ee9e4e6..ecbdd15603 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/DefaultSession.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/DefaultSession.kt @@ -290,8 +290,12 @@ internal class DefaultSession(override val sessionParams: SessionParams) : Sessi cryptoService.setDeviceName(deviceId, deviceName, callback) } - override fun deleteDevice(deviceId: String, accountPassword: String, callback: MatrixCallback) { - cryptoService.deleteDevice(deviceId, accountPassword, callback) + override fun deleteDevice(deviceId: String, callback: MatrixCallback) { + cryptoService.deleteDevice(deviceId, callback) + } + + override fun deleteDeviceWithUserPassword(deviceId: String, authSession: String?, password: String, callback: MatrixCallback) { + cryptoService.deleteDeviceWithUserPassword(deviceId, authSession, password, callback) } override fun getCryptoVersion(context: Context, longFormat: Boolean): String { 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 fb5c3a58fa..4b8bb592df 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 @@ -46,7 +46,9 @@ import com.google.android.material.textfield.TextInputLayout import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.extensions.getFingerprintHumanReadable import im.vector.matrix.android.api.extensions.sortByLastSeen +import im.vector.matrix.android.api.failure.Failure import im.vector.matrix.android.api.session.Session +import im.vector.matrix.android.internal.auth.data.LoginFlowTypes import im.vector.matrix.android.internal.crypto.model.rest.DeviceInfo import im.vector.matrix.android.internal.crypto.model.rest.DevicesListResponse import im.vector.riotredesign.R @@ -2413,7 +2415,7 @@ class VectorSettingsPreferencesFragment : VectorPreferenceFragment(), SharedPref // disable the deletion for our own device if (!TextUtils.equals(mSession.getMyDevice()?.deviceId, aDeviceInfo.deviceId)) { - builder.setNegativeButton(R.string.delete) { _, _ -> displayDeviceDeletionDialog(aDeviceInfo) } + builder.setNegativeButton(R.string.delete) { _, _ -> deleteDevice(aDeviceInfo) } } builder.setNeutralButton(R.string.cancel, null) @@ -2486,11 +2488,17 @@ class VectorSettingsPreferencesFragment : VectorPreferenceFragment(), SharedPref /** * Try to delete a device. * - * @param deviceId the device id + * @param deviceInfo the device to delete */ - private fun deleteDevice(deviceId: String) { + private fun deleteDevice(deviceInfo: DeviceInfo) { + val deviceId = deviceInfo.deviceId + if (deviceId == null) { + Timber.e("## displayDeviceDeletionDialog(): sanity check failure") + return + } + displayLoadingView() - mSession.deleteDevice(deviceId, mAccountPassword, object : MatrixCallback { + mSession.deleteDevice(deviceId, object : MatrixCallback { override fun onSuccess(data: Unit) { hideLoadingView() // force settings update @@ -2498,59 +2506,85 @@ class VectorSettingsPreferencesFragment : VectorPreferenceFragment(), SharedPref } override fun onFailure(failure: Throwable) { - mAccountPassword = "" - onCommonDone(failure.localizedMessage) + var isPasswordRequestFound = false + + if (failure is Failure.RegistrationFlowError) { + // We only support LoginFlowTypes.PASSWORD + // Check if we can provide the user password + failure.registrationFlowResponse.flows?.forEach { interactiveAuthenticationFlow -> + isPasswordRequestFound = isPasswordRequestFound || interactiveAuthenticationFlow.stages?.any { it == LoginFlowTypes.PASSWORD } == true + } + + if (isPasswordRequestFound) { + maybeShowDeleteDeviceWithPasswordDialog(deviceId, failure.registrationFlowResponse.session) + } + + } + + if (!isPasswordRequestFound) { + // LoginFlowTypes.PASSWORD not supported, and this is the only one RiotX supports so far... + onCommonDone(failure.localizedMessage) + } } }) } /** - * Display a delete confirmation dialog to remove a device.

- * The user is invited to enter his password to confirm the deletion. - * - * @param aDeviceInfoToDelete device info + * Show a dialog to ask for user password, or use a previously entered password. */ - private fun displayDeviceDeletionDialog(aDeviceInfoToDelete: DeviceInfo) { - if (aDeviceInfoToDelete.deviceId != null) { - if (!TextUtils.isEmpty(mAccountPassword)) { - deleteDevice(aDeviceInfoToDelete.deviceId!!) - } else { - activity?.let { - val inflater = it.layoutInflater - val layout = inflater.inflate(R.layout.dialog_device_delete, null) - val passwordEditText = layout.findViewById(R.id.delete_password) - - AlertDialog.Builder(it) - .setIcon(android.R.drawable.ic_dialog_alert) - .setTitle(R.string.devices_delete_dialog_title) - .setView(layout) - .setPositiveButton(R.string.devices_delete_submit_button_label, DialogInterface.OnClickListener { _, _ -> - if (TextUtils.isEmpty(passwordEditText.toString())) { - it.toast(R.string.error_empty_field_your_password) - return@OnClickListener - } - mAccountPassword = passwordEditText.text.toString() - deleteDevice(aDeviceInfoToDelete.deviceId!!) - }) - .setNegativeButton(R.string.cancel) { _, _ -> - hideLoadingView() - } - .setOnKeyListener(DialogInterface.OnKeyListener { dialog, keyCode, event -> - if (event.action == KeyEvent.ACTION_UP && keyCode == KeyEvent.KEYCODE_BACK) { - dialog.cancel() - hideLoadingView() - return@OnKeyListener true - } - false - }) - .show() - } - } + private fun maybeShowDeleteDeviceWithPasswordDialog(deviceId: String, authSession: String?) { + if (!TextUtils.isEmpty(mAccountPassword)) { + deleteDeviceWithPassword(deviceId, authSession, mAccountPassword) } else { - Timber.e("## displayDeviceDeletionDialog(): sanity check failure") + activity?.let { + val inflater = it.layoutInflater + val layout = inflater.inflate(R.layout.dialog_device_delete, null) + val passwordEditText = layout.findViewById(R.id.delete_password) + + AlertDialog.Builder(it) + .setIcon(android.R.drawable.ic_dialog_alert) + .setTitle(R.string.devices_delete_dialog_title) + .setView(layout) + .setPositiveButton(R.string.devices_delete_submit_button_label, DialogInterface.OnClickListener { _, _ -> + if (TextUtils.isEmpty(passwordEditText.toString())) { + it.toast(R.string.error_empty_field_your_password) + return@OnClickListener + } + mAccountPassword = passwordEditText.text.toString() + deleteDeviceWithPassword(deviceId, authSession, mAccountPassword) + }) + .setNegativeButton(R.string.cancel) { _, _ -> + hideLoadingView() + } + .setOnKeyListener(DialogInterface.OnKeyListener { dialog, keyCode, event -> + if (event.action == KeyEvent.ACTION_UP && keyCode == KeyEvent.KEYCODE_BACK) { + dialog.cancel() + hideLoadingView() + return@OnKeyListener true + } + false + }) + .show() + } } } + private fun deleteDeviceWithPassword(deviceId: String, authSession: String?, accountPassword: String) { + mSession.deleteDeviceWithUserPassword(deviceId, authSession, accountPassword, object : MatrixCallback { + override fun onSuccess(data: Unit) { + hideLoadingView() + // force settings update + refreshDevicesList() + } + + override fun onFailure(failure: Throwable) { + // Password is maybe not good + onCommonDone(failure.localizedMessage) + mAccountPassword = "" + } + }) + } + /** * Manage the e2e keys export. */