From 6aa5dc992d436a41c2d41da2985789bec6418cd1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 17 Feb 2021 17:56:13 +0100 Subject: [PATCH 1/8] Migrate AuthenticationService API to coroutines (#2449) --- CHANGES.md | 2 +- .../sdk/api/auth/AuthenticationService.kt | 29 +- .../android/sdk/api/auth/login/LoginWizard.kt | 20 +- .../auth/registration/RegistrationWizard.kt | 21 +- .../auth/DefaultAuthenticationService.kt | 195 +++---- .../internal/auth/login/DefaultLoginWizard.kt | 79 +-- .../registration/DefaultRegistrationWizard.kt | 122 ++-- .../app/features/login/LoginViewModel.kt | 527 +++++++++--------- .../signout/soft/SoftLogoutViewModel.kt | 69 +-- 9 files changed, 462 insertions(+), 602 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 13f2ec231c..14bde7c5d6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,7 +19,7 @@ Translations 🗣: - SDK API changes ⚠️: - - + - Migrate AuthenticationService API to coroutines (#2449) Build 🧱: - diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt index bf21941e0c..09ac140db6 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt @@ -16,7 +16,6 @@ package org.matrix.android.sdk.api.auth -import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig import org.matrix.android.sdk.api.auth.data.LoginFlowResult @@ -24,7 +23,6 @@ import org.matrix.android.sdk.api.auth.login.LoginWizard import org.matrix.android.sdk.api.auth.registration.RegistrationWizard import org.matrix.android.sdk.api.auth.wellknown.WellknownResult import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.util.Cancelable /** * This interface defines methods to authenticate or to create an account to a matrix server. @@ -34,12 +32,12 @@ interface AuthenticationService { * Request the supported login flows for this homeserver. * This is the first method to call to be able to get a wizard to login or the create an account */ - fun getLoginFlow(homeServerConnectionConfig: HomeServerConnectionConfig, callback: MatrixCallback): Cancelable + suspend fun getLoginFlow(homeServerConnectionConfig: HomeServerConnectionConfig): LoginFlowResult /** * Request the supported login flows for the corresponding sessionId. */ - fun getLoginFlowOfSession(sessionId: String, callback: MatrixCallback): Cancelable + suspend fun getLoginFlowOfSession(sessionId: String): LoginFlowResult /** * Get a SSO url @@ -69,12 +67,12 @@ interface AuthenticationService { /** * Cancel pending login or pending registration */ - fun cancelPendingLoginOrRegistration() + suspend fun cancelPendingLoginOrRegistration() /** * Reset all pending settings, including current HomeServerConnectionConfig */ - fun reset() + suspend fun reset() /** * Check if there is an authenticated [Session]. @@ -91,24 +89,21 @@ interface AuthenticationService { /** * Create a session after a SSO successful login */ - fun createSessionFromSso(homeServerConnectionConfig: HomeServerConnectionConfig, - credentials: Credentials, - callback: MatrixCallback): Cancelable + suspend fun createSessionFromSso(homeServerConnectionConfig: HomeServerConnectionConfig, + credentials: Credentials): Session /** * Perform a wellknown request, using the domain from the matrixId */ - fun getWellKnownData(matrixId: String, - homeServerConnectionConfig: HomeServerConnectionConfig?, - callback: MatrixCallback): Cancelable + suspend fun getWellKnownData(matrixId: String, + homeServerConnectionConfig: HomeServerConnectionConfig?): WellknownResult /** * Authenticate with a matrixId and a password * Usually call this after a successful call to getWellKnownData() */ - fun directAuthentication(homeServerConnectionConfig: HomeServerConnectionConfig, - matrixId: String, - password: String, - initialDeviceName: String, - callback: MatrixCallback): Cancelable + suspend fun directAuthentication(homeServerConnectionConfig: HomeServerConnectionConfig, + matrixId: String, + password: String, + initialDeviceName: String): Session } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt index 48705ee7b7..9c96cba40c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/login/LoginWizard.kt @@ -16,7 +16,6 @@ package org.matrix.android.sdk.api.auth.login -import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.util.Cancelable @@ -29,26 +28,23 @@ interface LoginWizard { * @param callback the matrix callback on which you'll receive the result of authentication. * @return a [Cancelable] */ - fun login(login: String, - password: String, - deviceName: String, - callback: MatrixCallback): Cancelable + suspend fun login(login: String, + password: String, + deviceName: String): Session /** * Exchange a login token to an access token */ - fun loginWithToken(loginToken: String, - callback: MatrixCallback): Cancelable + suspend fun loginWithToken(loginToken: String): Session /** * Reset user password */ - fun resetPassword(email: String, - newPassword: String, - callback: MatrixCallback): Cancelable + suspend fun resetPassword(email: String, + newPassword: String) /** - * Confirm the new password, once the user has checked his email + * Confirm the new password, once the user has checked their email */ - fun resetPasswordMailConfirmed(callback: MatrixCallback): Cancelable + suspend fun resetPasswordMailConfirmed() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/registration/RegistrationWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/registration/RegistrationWizard.kt index ed7b249f1e..d00c9a0c82 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/registration/RegistrationWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/registration/RegistrationWizard.kt @@ -16,28 +16,25 @@ package org.matrix.android.sdk.api.auth.registration -import org.matrix.android.sdk.api.MatrixCallback -import org.matrix.android.sdk.api.util.Cancelable - interface RegistrationWizard { - fun getRegistrationFlow(callback: MatrixCallback): Cancelable + suspend fun getRegistrationFlow(): RegistrationResult - fun createAccount(userName: String, password: String, initialDeviceDisplayName: String?, callback: MatrixCallback): Cancelable + suspend fun createAccount(userName: String, password: String, initialDeviceDisplayName: String?): RegistrationResult - fun performReCaptcha(response: String, callback: MatrixCallback): Cancelable + suspend fun performReCaptcha(response: String): RegistrationResult - fun acceptTerms(callback: MatrixCallback): Cancelable + suspend fun acceptTerms(): RegistrationResult - fun dummy(callback: MatrixCallback): Cancelable + suspend fun dummy(): RegistrationResult - fun addThreePid(threePid: RegisterThreePid, callback: MatrixCallback): Cancelable + suspend fun addThreePid(threePid: RegisterThreePid): RegistrationResult - fun sendAgainThreePid(callback: MatrixCallback): Cancelable + suspend fun sendAgainThreePid(): RegistrationResult - fun handleValidateThreePid(code: String, callback: MatrixCallback): Cancelable + suspend fun handleValidateThreePid(code: String): RegistrationResult - fun checkIfEmailHasBeenValidated(delayMillis: Long, callback: MatrixCallback): Cancelable + suspend fun checkIfEmailHasBeenValidated(delayMillis: Long): RegistrationResult val currentThreePid: String? diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt index c99e9bd81c..4344f74b54 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt @@ -18,10 +18,7 @@ package org.matrix.android.sdk.internal.auth import android.net.Uri import dagger.Lazy -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import okhttp3.OkHttpClient -import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.auth.AuthenticationService import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig @@ -32,8 +29,6 @@ import org.matrix.android.sdk.api.auth.registration.RegistrationWizard import org.matrix.android.sdk.api.auth.wellknown.WellknownResult import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.util.Cancelable -import org.matrix.android.sdk.api.util.NoOpCancellable import org.matrix.android.sdk.api.util.appendParamToUrl import org.matrix.android.sdk.internal.SessionManager import org.matrix.android.sdk.internal.auth.data.LoginFlowResponse @@ -50,11 +45,6 @@ import org.matrix.android.sdk.internal.network.RetrofitFactory import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.network.httpclient.addSocketFactory import org.matrix.android.sdk.internal.network.ssl.UnrecognizedCertificateException -import org.matrix.android.sdk.internal.task.TaskExecutor -import org.matrix.android.sdk.internal.task.configureWith -import org.matrix.android.sdk.internal.task.launchToCallback -import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers -import org.matrix.android.sdk.internal.util.toCancelable import org.matrix.android.sdk.internal.wellknown.GetWellknownTask import javax.inject.Inject import javax.net.ssl.HttpsURLConnection @@ -63,14 +53,12 @@ internal class DefaultAuthenticationService @Inject constructor( @Unauthenticated private val okHttpClient: Lazy, private val retrofitFactory: RetrofitFactory, - private val coroutineDispatchers: MatrixCoroutineDispatchers, private val sessionParamsStore: SessionParamsStore, private val sessionManager: SessionManager, private val sessionCreator: SessionCreator, private val pendingSessionStore: PendingSessionStore, private val getWellknownTask: GetWellknownTask, - private val directLoginTask: DirectLoginTask, - private val taskExecutor: TaskExecutor + private val directLoginTask: DirectLoginTask ) : AuthenticationService { private var pendingSessionData: PendingSessionData? = pendingSessionStore.getPendingSessionData() @@ -89,15 +77,11 @@ internal class DefaultAuthenticationService @Inject constructor( } } - override fun getLoginFlowOfSession(sessionId: String, callback: MatrixCallback): Cancelable { + override suspend fun getLoginFlowOfSession(sessionId: String): LoginFlowResult { val homeServerConnectionConfig = sessionParamsStore.get(sessionId)?.homeServerConnectionConfig + ?: throw IllegalStateException("Session not found") - return if (homeServerConnectionConfig == null) { - callback.onFailure(IllegalStateException("Session not found")) - NoOpCancellable - } else { - getLoginFlow(homeServerConnectionConfig, callback) - } + return getLoginFlow(homeServerConnectionConfig) } override fun getSsoUrl(redirectUrl: String, deviceId: String?, providerId: String?): String? { @@ -146,70 +130,65 @@ internal class DefaultAuthenticationService @Inject constructor( ?.trim { it == '/' } } - override fun getLoginFlow(homeServerConnectionConfig: HomeServerConnectionConfig, callback: MatrixCallback): Cancelable { + override suspend fun getLoginFlow(homeServerConnectionConfig: HomeServerConnectionConfig): LoginFlowResult { pendingSessionData = null - return taskExecutor.executorScope.launch(coroutineDispatchers.main) { - pendingSessionStore.delete() + pendingSessionStore.delete() - val result = runCatching { - getLoginFlowInternal(homeServerConnectionConfig) - } - result.fold( - { - if (it is LoginFlowResult.Success) { - // The homeserver exists and up to date, keep the config - // Homeserver url may have been changed, if it was a Riot url - val alteredHomeServerConnectionConfig = homeServerConnectionConfig.copy( - homeServerUri = Uri.parse(it.homeServerUrl) - ) - - pendingSessionData = PendingSessionData(alteredHomeServerConnectionConfig) - .also { data -> pendingSessionStore.savePendingSessionData(data) } - } - callback.onSuccess(it) - }, - { - if (it is UnrecognizedCertificateException) { - callback.onFailure(Failure.UnrecognizedCertificateFailure(homeServerConnectionConfig.homeServerUri.toString(), it.fingerprint)) - } else { - callback.onFailure(it) - } - } - ) + val result = runCatching { + getLoginFlowInternal(homeServerConnectionConfig) } - .toCancelable() + return result.fold( + { + if (it is LoginFlowResult.Success) { + // The homeserver exists and up to date, keep the config + // Homeserver url may have been changed, if it was a Riot url + val alteredHomeServerConnectionConfig = homeServerConnectionConfig.copy( + homeServerUri = Uri.parse(it.homeServerUrl) + ) + + pendingSessionData = PendingSessionData(alteredHomeServerConnectionConfig) + .also { data -> pendingSessionStore.savePendingSessionData(data) } + } + it + }, + { + if (it is UnrecognizedCertificateException) { + throw Failure.UnrecognizedCertificateFailure(homeServerConnectionConfig.homeServerUri.toString(), it.fingerprint) + } else { + throw it + } + } + ) } private suspend fun getLoginFlowInternal(homeServerConnectionConfig: HomeServerConnectionConfig): LoginFlowResult { - return withContext(coroutineDispatchers.io) { - val authAPI = buildAuthAPI(homeServerConnectionConfig) + val authAPI = buildAuthAPI(homeServerConnectionConfig) - // First check the homeserver version - runCatching { - executeRequest(null) { - apiCall = authAPI.versions() - } + // First check the homeserver version + return runCatching { + executeRequest(null) { + apiCall = authAPI.versions() } - .map { versions -> - // Ok, it seems that the homeserver url is valid - getLoginFlowResult(authAPI, versions, homeServerConnectionConfig.homeServerUri.toString()) - } - .fold( - { - it - }, - { - if (it is Failure.OtherServerError - && it.httpCode == HttpsURLConnection.HTTP_NOT_FOUND /* 404 */) { - // It's maybe a Riot url? - getRiotDomainLoginFlowInternal(homeServerConnectionConfig) - } else { - throw it - } - } - ) } + .map { versions -> + // Ok, it seems that the homeserver url is valid + getLoginFlowResult(authAPI, versions, homeServerConnectionConfig.homeServerUri.toString()) + } + .fold( + { + it + }, + { + if (it is Failure.OtherServerError + && it.httpCode == HttpsURLConnection.HTTP_NOT_FOUND /* 404 */) { + // It's maybe a Riot url? + getRiotDomainLoginFlowInternal(homeServerConnectionConfig) + } else { + throw it + } + } + ) } private suspend fun getRiotDomainLoginFlowInternal(homeServerConnectionConfig: HomeServerConnectionConfig): LoginFlowResult { @@ -340,10 +319,8 @@ internal class DefaultAuthenticationService @Inject constructor( DefaultRegistrationWizard( buildClient(it), retrofitFactory, - coroutineDispatchers, sessionCreator, - pendingSessionStore, - taskExecutor.executorScope + pendingSessionStore ).also { currentRegistrationWizard = it } @@ -361,10 +338,8 @@ internal class DefaultAuthenticationService @Inject constructor( DefaultLoginWizard( buildClient(it), retrofitFactory, - coroutineDispatchers, sessionCreator, - pendingSessionStore, - taskExecutor.executorScope + pendingSessionStore ).also { currentLoginWizard = it } @@ -372,7 +347,7 @@ internal class DefaultAuthenticationService @Inject constructor( } } - override fun cancelPendingLoginOrRegistration() { + override suspend fun cancelPendingLoginOrRegistration() { currentLoginWizard = null currentRegistrationWizard = null @@ -381,61 +356,39 @@ internal class DefaultAuthenticationService @Inject constructor( pendingSessionData = pendingSessionData?.homeServerConnectionConfig ?.let { PendingSessionData(it) } .also { - taskExecutor.executorScope.launch(coroutineDispatchers.main) { - if (it == null) { - // Should not happen - pendingSessionStore.delete() - } else { - pendingSessionStore.savePendingSessionData(it) - } + if (it == null) { + // Should not happen + pendingSessionStore.delete() + } else { + pendingSessionStore.savePendingSessionData(it) } } } - override fun reset() { + override suspend fun reset() { currentLoginWizard = null currentRegistrationWizard = null pendingSessionData = null - taskExecutor.executorScope.launch(coroutineDispatchers.main) { - pendingSessionStore.delete() - } + pendingSessionStore.delete() } - override fun createSessionFromSso(homeServerConnectionConfig: HomeServerConnectionConfig, - credentials: Credentials, - callback: MatrixCallback): Cancelable { - return taskExecutor.executorScope.launchToCallback(coroutineDispatchers.main, callback) { - createSessionFromSso(credentials, homeServerConnectionConfig) - } + override suspend fun createSessionFromSso(homeServerConnectionConfig: HomeServerConnectionConfig, + credentials: Credentials): Session { + return sessionCreator.createSession(credentials, homeServerConnectionConfig) } - override fun getWellKnownData(matrixId: String, - homeServerConnectionConfig: HomeServerConnectionConfig?, - callback: MatrixCallback): Cancelable { - return getWellknownTask - .configureWith(GetWellknownTask.Params(matrixId, homeServerConnectionConfig)) { - this.callback = callback - } - .executeBy(taskExecutor) + override suspend fun getWellKnownData(matrixId: String, + homeServerConnectionConfig: HomeServerConnectionConfig?): WellknownResult { + return getWellknownTask.execute(GetWellknownTask.Params(matrixId, homeServerConnectionConfig)) } - override fun directAuthentication(homeServerConnectionConfig: HomeServerConnectionConfig, - matrixId: String, - password: String, - initialDeviceName: String, - callback: MatrixCallback): Cancelable { - return directLoginTask - .configureWith(DirectLoginTask.Params(homeServerConnectionConfig, matrixId, password, initialDeviceName)) { - this.callback = callback - } - .executeBy(taskExecutor) - } - - private suspend fun createSessionFromSso(credentials: Credentials, - homeServerConnectionConfig: HomeServerConnectionConfig): Session = withContext(coroutineDispatchers.computation) { - sessionCreator.createSession(credentials, homeServerConnectionConfig) + override suspend fun directAuthentication(homeServerConnectionConfig: HomeServerConnectionConfig, + matrixId: String, + password: String, + initialDeviceName: String): Session { + return directLoginTask.execute(DirectLoginTask.Params(homeServerConnectionConfig, matrixId, password, initialDeviceName)) } private fun buildAuthAPI(homeServerConnectionConfig: HomeServerConnectionConfig): AuthAPI { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt index 108d0d4a42..ec0cc169bd 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt @@ -17,13 +17,11 @@ package org.matrix.android.sdk.internal.auth.login import android.util.Patterns -import org.matrix.android.sdk.api.MatrixCallback +import okhttp3.OkHttpClient import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.auth.login.LoginWizard import org.matrix.android.sdk.api.auth.registration.RegisterThreePid import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.util.Cancelable -import org.matrix.android.sdk.api.util.NoOpCancellable import org.matrix.android.sdk.internal.auth.AuthAPI import org.matrix.android.sdk.internal.auth.PendingSessionStore import org.matrix.android.sdk.internal.auth.SessionCreator @@ -36,19 +34,12 @@ import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistration import org.matrix.android.sdk.internal.auth.registration.RegisterAddThreePidTask import org.matrix.android.sdk.internal.network.RetrofitFactory import org.matrix.android.sdk.internal.network.executeRequest -import org.matrix.android.sdk.internal.task.launchToCallback -import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.withContext -import okhttp3.OkHttpClient internal class DefaultLoginWizard( okHttpClient: OkHttpClient, retrofitFactory: RetrofitFactory, - private val coroutineDispatchers: MatrixCoroutineDispatchers, private val sessionCreator: SessionCreator, - private val pendingSessionStore: PendingSessionStore, - private val coroutineScope: CoroutineScope + private val pendingSessionStore: PendingSessionStore ) : LoginWizard { private var pendingSessionData: PendingSessionData = pendingSessionStore.getPendingSessionData() ?: error("Pending session data should exist here") @@ -56,34 +47,9 @@ internal class DefaultLoginWizard( private val authAPI = retrofitFactory.create(okHttpClient, pendingSessionData.homeServerConnectionConfig.homeServerUri.toString()) .create(AuthAPI::class.java) - override fun login(login: String, - password: String, - deviceName: String, - callback: MatrixCallback): Cancelable { - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - loginInternal(login, password, deviceName) - } - } - - /** - * Ref: https://matrix.org/docs/spec/client_server/latest#handling-the-authentication-endpoint - */ - override fun loginWithToken(loginToken: String, callback: MatrixCallback): Cancelable { - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - val loginParams = TokenLoginParams( - token = loginToken - ) - val credentials = executeRequest(null) { - apiCall = authAPI.login(loginParams) - } - - sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig) - } - } - - private suspend fun loginInternal(login: String, - password: String, - deviceName: String) = withContext(coroutineDispatchers.computation) { + override suspend fun login(login: String, + password: String, + deviceName: String): Session { val loginParams = if (Patterns.EMAIL_ADDRESS.matcher(login).matches()) { PasswordLoginParams.thirdPartyIdentifier(ThreePidMedium.EMAIL, login, password, deviceName) } else { @@ -93,16 +59,24 @@ internal class DefaultLoginWizard( apiCall = authAPI.login(loginParams) } - sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig) + return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig) } - override fun resetPassword(email: String, newPassword: String, callback: MatrixCallback): Cancelable { - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - resetPasswordInternal(email, newPassword) + /** + * Ref: https://matrix.org/docs/spec/client_server/latest#handling-the-authentication-endpoint + */ + override suspend fun loginWithToken(loginToken: String): Session { + val loginParams = TokenLoginParams( + token = loginToken + ) + val credentials = executeRequest(null) { + apiCall = authAPI.login(loginParams) } + + return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig) } - private suspend fun resetPasswordInternal(email: String, newPassword: String) { + override suspend fun resetPassword(email: String, newPassword: String) { val param = RegisterAddThreePidTask.Params( RegisterThreePid.Email(email), pendingSessionData.clientSecret, @@ -120,21 +94,14 @@ internal class DefaultLoginWizard( .also { pendingSessionStore.savePendingSessionData(it) } } - override fun resetPasswordMailConfirmed(callback: MatrixCallback): Cancelable { - val safeResetPasswordData = pendingSessionData.resetPasswordData ?: run { - callback.onFailure(IllegalStateException("developer error, no reset password in progress")) - return NoOpCancellable - } - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - resetPasswordMailConfirmedInternal(safeResetPasswordData) - } - } + override suspend fun resetPasswordMailConfirmed() { + val safeResetPasswordData = pendingSessionData.resetPasswordData + ?: throw IllegalStateException("developer error, no reset password in progress") - private suspend fun resetPasswordMailConfirmedInternal(resetPasswordData: ResetPasswordData) { val param = ResetPasswordMailConfirmed.create( pendingSessionData.clientSecret, - resetPasswordData.addThreePidRegistrationResponse.sid, - resetPasswordData.newPassword + safeResetPasswordData.addThreePidRegistrationResponse.sid, + safeResetPasswordData.newPassword ) executeRequest(null) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt index 163009d918..09c82eaf61 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt @@ -16,10 +16,8 @@ package org.matrix.android.sdk.internal.auth.registration -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import okhttp3.OkHttpClient -import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.auth.data.LoginFlowTypes import org.matrix.android.sdk.api.auth.registration.RegisterThreePid import org.matrix.android.sdk.api.auth.registration.RegistrationResult @@ -27,15 +25,11 @@ import org.matrix.android.sdk.api.auth.registration.RegistrationWizard import org.matrix.android.sdk.api.auth.registration.toFlowResult import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.failure.Failure.RegistrationFlowError -import org.matrix.android.sdk.api.util.Cancelable -import org.matrix.android.sdk.api.util.NoOpCancellable import org.matrix.android.sdk.internal.auth.AuthAPI import org.matrix.android.sdk.internal.auth.PendingSessionStore import org.matrix.android.sdk.internal.auth.SessionCreator import org.matrix.android.sdk.internal.auth.db.PendingSessionData import org.matrix.android.sdk.internal.network.RetrofitFactory -import org.matrix.android.sdk.internal.task.launchToCallback -import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers /** * This class execute the registration request and is responsible to keep the session of interactive authentication @@ -43,10 +37,8 @@ import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers internal class DefaultRegistrationWizard( private val okHttpClient: OkHttpClient, private val retrofitFactory: RetrofitFactory, - private val coroutineDispatchers: MatrixCoroutineDispatchers, private val sessionCreator: SessionCreator, - private val pendingSessionStore: PendingSessionStore, - private val coroutineScope: CoroutineScope + private val pendingSessionStore: PendingSessionStore ) : RegistrationWizard { private var pendingSessionData: PendingSessionData = pendingSessionStore.getPendingSessionData() ?: error("Pending session data should exist here") @@ -59,82 +51,66 @@ internal class DefaultRegistrationWizard( override val currentThreePid: String? get() { return when (val threePid = pendingSessionData.currentThreePidData?.threePid) { - is RegisterThreePid.Email -> threePid.email + is RegisterThreePid.Email -> threePid.email is RegisterThreePid.Msisdn -> { // Take formatted msisdn if provided by the server pendingSessionData.currentThreePidData?.addThreePidRegistrationResponse?.formattedMsisdn?.takeIf { it.isNotBlank() } ?: threePid.msisdn } - null -> null + null -> null } } override val isRegistrationStarted: Boolean get() = pendingSessionData.isRegistrationStarted - override fun getRegistrationFlow(callback: MatrixCallback): Cancelable { + override suspend fun getRegistrationFlow(): RegistrationResult { val params = RegistrationParams() - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - performRegistrationRequest(params) - } + return performRegistrationRequest(params) } - override fun createAccount(userName: String, - password: String, - initialDeviceDisplayName: String?, - callback: MatrixCallback): Cancelable { + override suspend fun createAccount(userName: String, + password: String, + initialDeviceDisplayName: String?): RegistrationResult { val params = RegistrationParams( username = userName, password = password, initialDeviceDisplayName = initialDeviceDisplayName ) - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - performRegistrationRequest(params) - .also { - pendingSessionData = pendingSessionData.copy(isRegistrationStarted = true) - .also { pendingSessionStore.savePendingSessionData(it) } - } - } + return performRegistrationRequest(params) + .also { + pendingSessionData = pendingSessionData.copy(isRegistrationStarted = true) + .also { pendingSessionStore.savePendingSessionData(it) } + } } - override fun performReCaptcha(response: String, callback: MatrixCallback): Cancelable { - val safeSession = pendingSessionData.currentSession ?: run { - callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) - return NoOpCancellable - } + override suspend fun performReCaptcha(response: String): RegistrationResult { + val safeSession = pendingSessionData.currentSession + ?: throw IllegalStateException("developer error, call createAccount() method first") + val params = RegistrationParams(auth = AuthParams.createForCaptcha(safeSession, response)) - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - performRegistrationRequest(params) - } + return performRegistrationRequest(params) } - override fun acceptTerms(callback: MatrixCallback): Cancelable { - val safeSession = pendingSessionData.currentSession ?: run { - callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) - return NoOpCancellable - } + override suspend fun acceptTerms(): RegistrationResult { + val safeSession = pendingSessionData.currentSession + ?: throw IllegalStateException("developer error, call createAccount() method first") + val params = RegistrationParams(auth = AuthParams(type = LoginFlowTypes.TERMS, session = safeSession)) - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - performRegistrationRequest(params) - } + return performRegistrationRequest(params) } - override fun addThreePid(threePid: RegisterThreePid, callback: MatrixCallback): Cancelable { - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - pendingSessionData = pendingSessionData.copy(currentThreePidData = null) - .also { pendingSessionStore.savePendingSessionData(it) } + override suspend fun addThreePid(threePid: RegisterThreePid): RegistrationResult { + pendingSessionData = pendingSessionData.copy(currentThreePidData = null) + .also { pendingSessionStore.savePendingSessionData(it) } - sendThreePid(threePid) - } + return sendThreePid(threePid) } - override fun sendAgainThreePid(callback: MatrixCallback): Cancelable { - val safeCurrentThreePid = pendingSessionData.currentThreePidData?.threePid ?: run { - callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) - return NoOpCancellable - } - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - sendThreePid(safeCurrentThreePid) - } + override suspend fun sendAgainThreePid(): RegistrationResult { + val safeCurrentThreePid = pendingSessionData.currentThreePidData?.threePid + ?: throw IllegalStateException("developer error, call createAccount() method first") + + return sendThreePid(safeCurrentThreePid) } private suspend fun sendThreePid(threePid: RegisterThreePid): RegistrationResult { @@ -173,20 +149,15 @@ internal class DefaultRegistrationWizard( return performRegistrationRequest(params) } - override fun checkIfEmailHasBeenValidated(delayMillis: Long, callback: MatrixCallback): Cancelable { - val safeParam = pendingSessionData.currentThreePidData?.registrationParams ?: run { - callback.onFailure(IllegalStateException("developer error, no pending three pid")) - return NoOpCancellable - } - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - performRegistrationRequest(safeParam, delayMillis) - } + override suspend fun checkIfEmailHasBeenValidated(delayMillis: Long): RegistrationResult { + val safeParam = pendingSessionData.currentThreePidData?.registrationParams + ?: throw IllegalStateException("developer error, no pending three pid") + + return performRegistrationRequest(safeParam, delayMillis) } - override fun handleValidateThreePid(code: String, callback: MatrixCallback): Cancelable { - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - validateThreePid(code) - } + override suspend fun handleValidateThreePid(code: String): RegistrationResult { + return validateThreePid(code) } private suspend fun validateThreePid(code: String): RegistrationResult { @@ -210,15 +181,12 @@ internal class DefaultRegistrationWizard( } } - override fun dummy(callback: MatrixCallback): Cancelable { - val safeSession = pendingSessionData.currentSession ?: run { - callback.onFailure(IllegalStateException("developer error, call createAccount() method first")) - return NoOpCancellable - } - return coroutineScope.launchToCallback(coroutineDispatchers.main, callback) { - val params = RegistrationParams(auth = AuthParams(type = LoginFlowTypes.DUMMY, session = safeSession)) - performRegistrationRequest(params) - } + override suspend fun dummy(): RegistrationResult { + val safeSession = pendingSessionData.currentSession + ?: throw IllegalStateException("developer error, call createAccount() method first") + + val params = RegistrationParams(auth = AuthParams(type = LoginFlowTypes.DUMMY, session = safeSession)) + return performRegistrationRequest(params) } private suspend fun performRegistrationRequest(registrationParams: RegistrationParams, diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt index 7bf0b98841..fa1a504041 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt @@ -19,6 +19,7 @@ package im.vector.app.features.login import android.content.Context import android.net.Uri import androidx.fragment.app.FragmentActivity +import androidx.lifecycle.viewModelScope import com.airbnb.mvrx.ActivityViewModelContext import com.airbnb.mvrx.Fail import com.airbnb.mvrx.Loading @@ -27,8 +28,8 @@ import com.airbnb.mvrx.Success import com.airbnb.mvrx.Uninitialized import com.airbnb.mvrx.ViewModelContext import dagger.assisted.Assisted -import dagger.assisted.AssistedInject import dagger.assisted.AssistedFactory +import dagger.assisted.AssistedInject import im.vector.app.R import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.extensions.configureAndStart @@ -37,7 +38,8 @@ import im.vector.app.core.platform.VectorViewModel import im.vector.app.core.resources.StringProvider import im.vector.app.core.utils.ensureTrailingSlash import im.vector.app.features.signout.soft.SoftLogoutActivity -import org.matrix.android.sdk.api.MatrixCallback +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch import org.matrix.android.sdk.api.auth.AuthenticationService import org.matrix.android.sdk.api.auth.HomeServerHistoryService import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig @@ -51,7 +53,6 @@ import org.matrix.android.sdk.api.auth.registration.Stage import org.matrix.android.sdk.api.auth.wellknown.WellknownResult import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.util.Cancelable import timber.log.Timber import java.util.concurrent.CancellationException @@ -117,7 +118,12 @@ class LoginViewModel @AssistedInject constructor( private var loginConfig: LoginConfig? = null - private var currentTask: Cancelable? = null + private var currentJob: Job? = null + set(value) { + // Cancel any previous Job + field?.cancel() + field = value + } override fun handle(action: LoginAction) { when (action) { @@ -186,22 +192,20 @@ class LoginViewModel @AssistedInject constructor( ) } - currentTask = safeLoginWizard.loginWithToken( - action.loginToken, - object : MatrixCallback { - override fun onSuccess(data: Session) { - onSessionCreated(data) - } - - override fun onFailure(failure: Throwable) { - _viewEvents.post(LoginViewEvents.Failure(failure)) - setState { - copy( - asyncLoginAction = Fail(failure) - ) - } - } - }) + currentJob = viewModelScope.launch { + try { + safeLoginWizard.loginWithToken(action.loginToken) + } catch (failure: Throwable) { + _viewEvents.post(LoginViewEvents.Failure(failure)) + setState { + copy( + asyncLoginAction = Fail(failure) + ) + } + null + } + ?.let { onSessionCreated(it) } + } } } @@ -231,46 +235,49 @@ class LoginViewModel @AssistedInject constructor( private fun handleCheckIfEmailHasBeenValidated(action: LoginAction.CheckIfEmailHasBeenValidated) { // We do not want the common progress bar to be displayed, so we do not change asyncRegistration value in the state - currentTask?.cancel() - currentTask = registrationWizard?.checkIfEmailHasBeenValidated(action.delayMillis, registrationCallback) + currentJob = executeRegistrationStep(withLoading = false) { + it.checkIfEmailHasBeenValidated(action.delayMillis) + } } private fun handleStopEmailValidationCheck() { - currentTask?.cancel() - currentTask = null + currentJob = null } private fun handleValidateThreePid(action: LoginAction.ValidateThreePid) { - setState { copy(asyncRegistration = Loading()) } - currentTask = registrationWizard?.handleValidateThreePid(action.code, registrationCallback) + currentJob = executeRegistrationStep { + it.handleValidateThreePid(action.code) + } } - private val registrationCallback = object : MatrixCallback { - override fun onSuccess(data: RegistrationResult) { - /* - // Simulate registration disabled - onFailure(Failure.ServerError(MatrixError( - code = MatrixError.FORBIDDEN, - message = "Registration is disabled" - ), 403)) - */ - - setState { - copy( - asyncRegistration = Uninitialized - ) - } - - when (data) { - is RegistrationResult.Success -> onSessionCreated(data.session) - is RegistrationResult.FlowResponse -> onFlowResponse(data.flowResult) - } + private fun executeRegistrationStep(withLoading: Boolean = true, + block: suspend (RegistrationWizard) -> RegistrationResult): Job { + if (withLoading) { + setState { copy(asyncRegistration = Loading()) } } - - override fun onFailure(failure: Throwable) { - if (failure !is CancellationException) { - _viewEvents.post(LoginViewEvents.Failure(failure)) + return viewModelScope.launch { + try { + registrationWizard?.let { block(it) } + /* + // Simulate registration disabled + throw Failure.ServerError(MatrixError( + code = MatrixError.FORBIDDEN, + message = "Registration is disabled" + ), 403)) + */ + } catch (failure: Throwable) { + if (failure !is CancellationException) { + _viewEvents.post(LoginViewEvents.Failure(failure)) + } + null } + ?.let { data -> + when (data) { + is RegistrationResult.Success -> onSessionCreated(data.session) + is RegistrationResult.FlowResponse -> onFlowResponse(data.flowResult) + } + } + setState { copy( asyncRegistration = Uninitialized @@ -281,78 +288,68 @@ class LoginViewModel @AssistedInject constructor( private fun handleAddThreePid(action: LoginAction.AddThreePid) { setState { copy(asyncRegistration = Loading()) } - currentTask = registrationWizard?.addThreePid(action.threePid, object : MatrixCallback { - override fun onSuccess(data: RegistrationResult) { - setState { - copy( - asyncRegistration = Uninitialized - ) - } - } - - override fun onFailure(failure: Throwable) { + currentJob = viewModelScope.launch { + try { + registrationWizard?.addThreePid(action.threePid) + } catch (failure: Throwable) { _viewEvents.post(LoginViewEvents.Failure(failure)) - setState { - copy( - asyncRegistration = Uninitialized - ) - } } - }) + setState { + copy( + asyncRegistration = Uninitialized + ) + } + } } private fun handleSendAgainThreePid() { setState { copy(asyncRegistration = Loading()) } - currentTask = registrationWizard?.sendAgainThreePid(object : MatrixCallback { - override fun onSuccess(data: RegistrationResult) { - setState { - copy( - asyncRegistration = Uninitialized - ) - } - } - - override fun onFailure(failure: Throwable) { + currentJob = viewModelScope.launch { + try { + registrationWizard?.sendAgainThreePid() + } catch (failure: Throwable) { _viewEvents.post(LoginViewEvents.Failure(failure)) - setState { - copy( - asyncRegistration = Uninitialized - ) - } } - }) + setState { + copy( + asyncRegistration = Uninitialized + ) + } + } } private fun handleAcceptTerms() { - setState { copy(asyncRegistration = Loading()) } - currentTask = registrationWizard?.acceptTerms(registrationCallback) + currentJob = executeRegistrationStep { + it.acceptTerms() + } } private fun handleRegisterDummy() { - setState { copy(asyncRegistration = Loading()) } - currentTask = registrationWizard?.dummy(registrationCallback) + currentJob = executeRegistrationStep { + it.dummy() + } } private fun handleRegisterWith(action: LoginAction.LoginOrRegister) { - setState { copy(asyncRegistration = Loading()) } reAuthHelper.data = action.password - currentTask = registrationWizard?.createAccount( - action.username, - action.password, - action.initialDeviceName, - registrationCallback - ) + currentJob = executeRegistrationStep { + it.createAccount( + action.username, + action.password, + action.initialDeviceName + ) + } } private fun handleCaptchaDone(action: LoginAction.CaptchaDone) { - setState { copy(asyncRegistration = Loading()) } - currentTask = registrationWizard?.performReCaptcha(action.captchaResponse, registrationCallback) + currentJob = executeRegistrationStep { + it.performReCaptcha(action.captchaResponse) + } } private fun handleResetAction(action: LoginAction.ResetAction) { // Cancel any request - currentTask?.cancel() - currentTask = null + currentJob = null when (action) { LoginAction.ResetHomeServerType -> { @@ -363,16 +360,17 @@ class LoginViewModel @AssistedInject constructor( } } LoginAction.ResetHomeServerUrl -> { - authenticationService.reset() - - setState { - copy( - asyncHomeServerLoginFlowRequest = Uninitialized, - homeServerUrl = null, - loginMode = LoginMode.Unknown, - serverType = ServerType.Unknown, - loginModeSupportedTypes = emptyList() - ) + viewModelScope.launch { + authenticationService.reset() + setState { + copy( + asyncHomeServerLoginFlowRequest = Uninitialized, + homeServerUrl = null, + loginMode = LoginMode.Unknown, + serverType = ServerType.Unknown, + loginModeSupportedTypes = emptyList() + ) + } } } LoginAction.ResetSignMode -> { @@ -386,13 +384,14 @@ class LoginViewModel @AssistedInject constructor( } } LoginAction.ResetLogin -> { - authenticationService.cancelPendingLoginOrRegistration() - - setState { - copy( - asyncLoginAction = Uninitialized, - asyncRegistration = Uninitialized - ) + viewModelScope.launch { + authenticationService.cancelPendingLoginOrRegistration() + setState { + copy( + asyncLoginAction = Uninitialized, + asyncRegistration = Uninitialized + ) + } } } LoginAction.ResetResetPassword -> { @@ -473,26 +472,27 @@ class LoginViewModel @AssistedInject constructor( ) } - currentTask = safeLoginWizard.resetPassword(action.email, action.newPassword, object : MatrixCallback { - override fun onSuccess(data: Unit) { - setState { - copy( - asyncResetPassword = Success(data), - resetPasswordEmail = action.email - ) - } - - _viewEvents.post(LoginViewEvents.OnResetPasswordSendThreePidDone) - } - - override fun onFailure(failure: Throwable) { + currentJob = viewModelScope.launch { + try { + safeLoginWizard.resetPassword(action.email, action.newPassword) + } catch (failure: Throwable) { setState { copy( asyncResetPassword = Fail(failure) ) } + return@launch } - }) + + setState { + copy( + asyncResetPassword = Success(Unit), + resetPasswordEmail = action.email + ) + } + + _viewEvents.post(LoginViewEvents.OnResetPasswordSendThreePidDone) + } } } @@ -514,26 +514,26 @@ class LoginViewModel @AssistedInject constructor( ) } - currentTask = safeLoginWizard.resetPasswordMailConfirmed(object : MatrixCallback { - override fun onSuccess(data: Unit) { - setState { - copy( - asyncResetMailConfirmed = Success(data), - resetPasswordEmail = null - ) - } - - _viewEvents.post(LoginViewEvents.OnResetPasswordMailConfirmationSuccess) - } - - override fun onFailure(failure: Throwable) { + currentJob = viewModelScope.launch { + try { + safeLoginWizard.resetPasswordMailConfirmed() + } catch (failure: Throwable) { setState { copy( asyncResetMailConfirmed = Fail(failure) ) } + return@launch } - }) + setState { + copy( + asyncResetMailConfirmed = Success(Unit), + resetPasswordEmail = null + ) + } + + _viewEvents.post(LoginViewEvents.OnResetPasswordMailConfirmationSuccess) + } } } @@ -553,36 +553,36 @@ class LoginViewModel @AssistedInject constructor( ) } - authenticationService.getWellKnownData(action.username, homeServerConnectionConfig, object : MatrixCallback { - override fun onSuccess(data: WellknownResult) { - when (data) { - is WellknownResult.Prompt -> - onWellknownSuccess(action, data, homeServerConnectionConfig) - is WellknownResult.FailPrompt -> - // Relax on IS discovery if home server is valid - if (data.homeServerUrl != null && data.wellKnown != null) { - onWellknownSuccess(action, WellknownResult.Prompt(data.homeServerUrl!!, null, data.wellKnown!!), homeServerConnectionConfig) - } else { - onWellKnownError() - } - is WellknownResult.InvalidMatrixId -> { - setState { - copy( - asyncLoginAction = Uninitialized - ) - } - _viewEvents.post(LoginViewEvents.Failure(Exception(stringProvider.getString(R.string.login_signin_matrix_id_error_invalid_matrix_id)))) - } - else -> { + currentJob = viewModelScope.launch { + val data = try { + authenticationService.getWellKnownData(action.username, homeServerConnectionConfig) + } catch (failure: Throwable) { + onDirectLoginError(failure) + return@launch + } + when (data) { + is WellknownResult.Prompt -> + onWellknownSuccess(action, data, homeServerConnectionConfig) + is WellknownResult.FailPrompt -> + // Relax on IS discovery if home server is valid + if (data.homeServerUrl != null && data.wellKnown != null) { + onWellknownSuccess(action, WellknownResult.Prompt(data.homeServerUrl!!, null, data.wellKnown!!), homeServerConnectionConfig) + } else { onWellKnownError() } - }.exhaustive - } - - override fun onFailure(failure: Throwable) { - onDirectLoginError(failure) - } - }) + is WellknownResult.InvalidMatrixId -> { + setState { + copy( + asyncLoginAction = Uninitialized + ) + } + _viewEvents.post(LoginViewEvents.Failure(Exception(stringProvider.getString(R.string.login_signin_matrix_id_error_invalid_matrix_id)))) + } + else -> { + onWellKnownError() + } + }.exhaustive + } } private fun onWellKnownError() { @@ -594,9 +594,9 @@ class LoginViewModel @AssistedInject constructor( _viewEvents.post(LoginViewEvents.Failure(Exception(stringProvider.getString(R.string.autodiscover_well_known_error)))) } - private fun onWellknownSuccess(action: LoginAction.LoginOrRegister, - wellKnownPrompt: WellknownResult.Prompt, - homeServerConnectionConfig: HomeServerConnectionConfig?) { + private suspend fun onWellknownSuccess(action: LoginAction.LoginOrRegister, + wellKnownPrompt: WellknownResult.Prompt, + homeServerConnectionConfig: HomeServerConnectionConfig?) { val alteredHomeServerConnectionConfig = homeServerConnectionConfig ?.copy( homeServerUri = Uri.parse(wellKnownPrompt.homeServerUrl), @@ -607,20 +607,17 @@ class LoginViewModel @AssistedInject constructor( identityServerUri = wellKnownPrompt.identityServerUrl?.let { Uri.parse(it) } ) - authenticationService.directAuthentication( - alteredHomeServerConnectionConfig, - action.username, - action.password, - action.initialDeviceName, - object : MatrixCallback { - override fun onSuccess(data: Session) { - onSessionCreated(data) - } - - override fun onFailure(failure: Throwable) { - onDirectLoginError(failure) - } - }) + val data = try { + authenticationService.directAuthentication( + alteredHomeServerConnectionConfig, + action.username, + action.password, + action.initialDeviceName) + } catch (failure: Throwable) { + onDirectLoginError(failure) + return + } + onSessionCreated(data) } private fun onDirectLoginError(failure: Throwable) { @@ -657,35 +654,33 @@ class LoginViewModel @AssistedInject constructor( ) } - currentTask = safeLoginWizard.login( - action.username, - action.password, - action.initialDeviceName, - object : MatrixCallback { - override fun onSuccess(data: Session) { + currentJob = viewModelScope.launch { + try { + safeLoginWizard.login( + action.username, + action.password, + action.initialDeviceName + ) + } catch (failure: Throwable) { + setState { + copy( + asyncLoginAction = Fail(failure) + ) + } + null + } + ?.let { reAuthHelper.data = action.password - onSessionCreated(data) + onSessionCreated(it) } - - override fun onFailure(failure: Throwable) { - setState { - copy( - asyncLoginAction = Fail(failure) - ) - } - } - }) + } } } private fun startRegistrationFlow() { - setState { - copy( - asyncRegistration = Loading() - ) + currentJob = executeRegistrationStep { + it.getRegistrationFlow() } - - currentTask = registrationWizard?.getRegistrationFlow(registrationCallback) } private fun startAuthenticationFlow() { @@ -706,8 +701,9 @@ class LoginViewModel @AssistedInject constructor( } } - private fun onSessionCreated(session: Session) { + private suspend fun onSessionCreated(session: Session) { activeSessionHolder.setActiveSession(session) + authenticationService.reset() session.configureAndStart(applicationContext) setState { @@ -724,15 +720,17 @@ class LoginViewModel @AssistedInject constructor( // Should not happen Timber.w("homeServerConnectionConfig is null") } else { - authenticationService.createSessionFromSso(homeServerConnectionConfigFinal, action.credentials, object : MatrixCallback { - override fun onSuccess(data: Session) { - onSessionCreated(data) + currentJob = viewModelScope.launch { + try { + authenticationService.createSessionFromSso(homeServerConnectionConfigFinal, action.credentials) + } catch (failure: Throwable) { + setState { + copy(asyncLoginAction = Fail(failure)) + } + null } - - override fun onFailure(failure: Throwable) = setState { - copy(asyncLoginAction = Fail(failure)) - } - }) + ?.let { onSessionCreated(it) } + } } } @@ -749,21 +747,21 @@ class LoginViewModel @AssistedInject constructor( private fun getLoginFlow(homeServerConnectionConfig: HomeServerConnectionConfig) { currentHomeServerConnectionConfig = homeServerConnectionConfig - currentTask?.cancel() - currentTask = null - authenticationService.cancelPendingLoginOrRegistration() + currentJob = viewModelScope.launch { + authenticationService.cancelPendingLoginOrRegistration() - setState { - copy( - asyncHomeServerLoginFlowRequest = Loading(), - // If user has entered https://matrix.org, ensure that server type is ServerType.MatrixOrg - // It is also useful to set the value again in the case of a certificate error on matrix.org - serverType = if (homeServerConnectionConfig.homeServerUri.toString() == matrixOrgUrl) ServerType.MatrixOrg else serverType - ) - } + setState { + copy( + asyncHomeServerLoginFlowRequest = Loading(), + // If user has entered https://matrix.org, ensure that server type is ServerType.MatrixOrg + // It is also useful to set the value again in the case of a certificate error on matrix.org + serverType = if (homeServerConnectionConfig.homeServerUri.toString() == matrixOrgUrl) ServerType.MatrixOrg else serverType + ) + } - currentTask = authenticationService.getLoginFlow(homeServerConnectionConfig, object : MatrixCallback { - override fun onFailure(failure: Throwable) { + try { + authenticationService.getLoginFlow(homeServerConnectionConfig) + } catch (failure: Throwable) { _viewEvents.post(LoginViewEvents.Failure(failure)) setState { copy( @@ -772,47 +770,42 @@ class LoginViewModel @AssistedInject constructor( serverType = if (serverType == ServerType.MatrixOrg) ServerType.Unknown else serverType ) } + null } + ?.let { data -> + // Valid Homeserver, add it to the history. + // Note: we add what the user has input, data.homeServerUrl can be different + rememberHomeServer(homeServerConnectionConfig.homeServerUri.toString()) - override fun onSuccess(data: LoginFlowResult) { - // Valid Homeserver, add it to the history. - // Note: we add what the user has input, data.homeServerUrl can be different - rememberHomeServer(homeServerConnectionConfig.homeServerUri.toString()) + when (data) { + is LoginFlowResult.Success -> { + val loginMode = when { + // SSO login is taken first + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) + && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password + else -> LoginMode.Unsupported + } - when (data) { - is LoginFlowResult.Success -> { - val loginMode = when { - // SSO login is taken first - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) - && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password - else -> LoginMode.Unsupported - } - - // FIXME We should post a view event here normally? - setState { - copy( - asyncHomeServerLoginFlowRequest = Uninitialized, - homeServerUrl = data.homeServerUrl, - loginMode = loginMode, - loginModeSupportedTypes = data.supportedLoginTypes.toList() - ) - } - if ((loginMode == LoginMode.Password && !data.isLoginAndRegistrationSupported) - || data.isOutdatedHomeserver) { - // Notify the UI - _viewEvents.post(LoginViewEvents.OutdatedHomeserver) + // FIXME We should post a view event here normally? + setState { + copy( + asyncHomeServerLoginFlowRequest = Uninitialized, + homeServerUrl = data.homeServerUrl, + loginMode = loginMode, + loginModeSupportedTypes = data.supportedLoginTypes.toList() + ) + } + if ((loginMode == LoginMode.Password && !data.isLoginAndRegistrationSupported) + || data.isOutdatedHomeserver) { + // Notify the UI + _viewEvents.post(LoginViewEvents.OutdatedHomeserver) + } + } } } - } - } - }) - } - - override fun onCleared() { - currentTask?.cancel() - super.onCleared() + } } fun getInitialHomeServerUrl(): String? { diff --git a/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt b/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt index 98ec2d07de..ab8fb292da 100644 --- a/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt @@ -25,19 +25,17 @@ import com.airbnb.mvrx.Success import com.airbnb.mvrx.Uninitialized import com.airbnb.mvrx.ViewModelContext import dagger.assisted.Assisted -import dagger.assisted.AssistedInject import dagger.assisted.AssistedFactory +import dagger.assisted.AssistedInject import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.extensions.hasUnsavedKeys import im.vector.app.core.platform.VectorViewModel import im.vector.app.features.login.LoginMode import kotlinx.coroutines.launch -import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.auth.AuthenticationService import org.matrix.android.sdk.api.auth.data.LoginFlowResult import org.matrix.android.sdk.api.auth.data.LoginFlowTypes import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.util.Cancelable import timber.log.Timber /** @@ -76,54 +74,52 @@ class SoftLogoutViewModel @AssistedInject constructor( } } - private var currentTask: Cancelable? = null - init { // Get the supported login flow getSupportedLoginFlow() } private fun getSupportedLoginFlow() { - currentTask?.cancel() - currentTask = null - authenticationService.cancelPendingLoginOrRegistration() + viewModelScope.launch { + authenticationService.cancelPendingLoginOrRegistration() - setState { - copy( - asyncHomeServerLoginFlowRequest = Loading() - ) - } + setState { + copy( + asyncHomeServerLoginFlowRequest = Loading() + ) + } - currentTask = authenticationService.getLoginFlowOfSession(session.sessionId, object : MatrixCallback { - override fun onFailure(failure: Throwable) { + try { + authenticationService.getLoginFlowOfSession(session.sessionId) + } catch (failure: Throwable) { setState { copy( asyncHomeServerLoginFlowRequest = Fail(failure) ) } + null } + ?.let { data -> + when (data) { + is LoginFlowResult.Success -> { + val loginMode = when { + // SSO login is taken first + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) + && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password + else -> LoginMode.Unsupported + } - override fun onSuccess(data: LoginFlowResult) { - when (data) { - is LoginFlowResult.Success -> { - val loginMode = when { - // SSO login is taken first - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) - && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password - else -> LoginMode.Unsupported - } - - setState { - copy( - asyncHomeServerLoginFlowRequest = Success(loginMode) - ) + setState { + copy( + asyncHomeServerLoginFlowRequest = Success(loginMode) + ) + } + } } } - } - } - }) + } } override fun handle(action: SoftLogoutAction) { @@ -227,9 +223,4 @@ class SoftLogoutViewModel @AssistedInject constructor( ) } } - - override fun onCleared() { - currentTask?.cancel() - super.onCleared() - } } From d19cedef881bc06a40a876785f099ade885d5fa1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 17 Feb 2021 16:11:02 +0100 Subject: [PATCH 2/8] Less code --- .../internal/auth/DefaultAuthenticationService.kt | 6 ++---- .../sdk/internal/auth/login/DefaultLoginWizard.kt | 8 +------- .../registration/DefaultRegistrationWizard.kt | 15 +++------------ 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt index 4344f74b54..ec64b7e757 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt @@ -317,8 +317,7 @@ internal class DefaultAuthenticationService @Inject constructor( ?: let { pendingSessionData?.homeServerConnectionConfig?.let { DefaultRegistrationWizard( - buildClient(it), - retrofitFactory, + buildAuthAPI(it), sessionCreator, pendingSessionStore ).also { @@ -336,8 +335,7 @@ internal class DefaultAuthenticationService @Inject constructor( ?: let { pendingSessionData?.homeServerConnectionConfig?.let { DefaultLoginWizard( - buildClient(it), - retrofitFactory, + buildAuthAPI(it), sessionCreator, pendingSessionStore ).also { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt index ec0cc169bd..4167875849 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/login/DefaultLoginWizard.kt @@ -17,7 +17,6 @@ package org.matrix.android.sdk.internal.auth.login import android.util.Patterns -import okhttp3.OkHttpClient import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.auth.login.LoginWizard import org.matrix.android.sdk.api.auth.registration.RegisterThreePid @@ -32,21 +31,16 @@ import org.matrix.android.sdk.internal.auth.db.PendingSessionData import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistrationParams import org.matrix.android.sdk.internal.auth.registration.AddThreePidRegistrationResponse import org.matrix.android.sdk.internal.auth.registration.RegisterAddThreePidTask -import org.matrix.android.sdk.internal.network.RetrofitFactory import org.matrix.android.sdk.internal.network.executeRequest internal class DefaultLoginWizard( - okHttpClient: OkHttpClient, - retrofitFactory: RetrofitFactory, + private val authAPI: AuthAPI, private val sessionCreator: SessionCreator, private val pendingSessionStore: PendingSessionStore ) : LoginWizard { private var pendingSessionData: PendingSessionData = pendingSessionStore.getPendingSessionData() ?: error("Pending session data should exist here") - private val authAPI = retrofitFactory.create(okHttpClient, pendingSessionData.homeServerConnectionConfig.homeServerUri.toString()) - .create(AuthAPI::class.java) - override suspend fun login(login: String, password: String, deviceName: String): Session { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt index 09c82eaf61..91e414e689 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt @@ -17,7 +17,6 @@ package org.matrix.android.sdk.internal.auth.registration import kotlinx.coroutines.delay -import okhttp3.OkHttpClient import org.matrix.android.sdk.api.auth.data.LoginFlowTypes import org.matrix.android.sdk.api.auth.registration.RegisterThreePid import org.matrix.android.sdk.api.auth.registration.RegistrationResult @@ -29,21 +28,18 @@ import org.matrix.android.sdk.internal.auth.AuthAPI import org.matrix.android.sdk.internal.auth.PendingSessionStore import org.matrix.android.sdk.internal.auth.SessionCreator import org.matrix.android.sdk.internal.auth.db.PendingSessionData -import org.matrix.android.sdk.internal.network.RetrofitFactory /** * This class execute the registration request and is responsible to keep the session of interactive authentication */ internal class DefaultRegistrationWizard( - private val okHttpClient: OkHttpClient, - private val retrofitFactory: RetrofitFactory, + authAPI: AuthAPI, private val sessionCreator: SessionCreator, private val pendingSessionStore: PendingSessionStore ) : RegistrationWizard { private var pendingSessionData: PendingSessionData = pendingSessionStore.getPendingSessionData() ?: error("Pending session data should exist here") - private val authAPI = buildAuthAPI() private val registerTask = DefaultRegisterTask(authAPI) private val registerAddThreePidTask = DefaultRegisterAddThreePidTask(authAPI) private val validateCodeTask = DefaultValidateCodeTask(authAPI) @@ -51,12 +47,12 @@ internal class DefaultRegistrationWizard( override val currentThreePid: String? get() { return when (val threePid = pendingSessionData.currentThreePidData?.threePid) { - is RegisterThreePid.Email -> threePid.email + is RegisterThreePid.Email -> threePid.email is RegisterThreePid.Msisdn -> { // Take formatted msisdn if provided by the server pendingSessionData.currentThreePidData?.addThreePidRegistrationResponse?.formattedMsisdn?.takeIf { it.isNotBlank() } ?: threePid.msisdn } - null -> null + null -> null } } @@ -207,9 +203,4 @@ internal class DefaultRegistrationWizard( val session = sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig) return RegistrationResult.Success(session) } - - private fun buildAuthAPI(): AuthAPI { - val retrofit = retrofitFactory.create(okHttpClient, pendingSessionData.homeServerConnectionConfig.homeServerUri.toString()) - return retrofit.create(AuthAPI::class.java) - } } From 0e322630f16c588ab92517ab8f8eaee419288bf0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 17 Feb 2021 16:35:24 +0100 Subject: [PATCH 3/8] Ignore url override from credential if it is not valid (#2822) --- CHANGES.md | 1 + .../android/sdk/internal/auth/AuthModule.kt | 7 +- .../auth/IsValidClientServerApiTask.kt | 75 +++++++++++++++++++ .../sdk/internal/auth/SessionCreator.kt | 30 ++++++-- 4 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/IsValidClientServerApiTask.kt diff --git a/CHANGES.md b/CHANGES.md index 14bde7c5d6..091b49791d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,7 @@ Bugfix 🐛: - VoIP : fix audio devices output - Fix crash after initial sync on Dendrite - Fix crash reported by PlayStore (#2707) + - Ignore url override from credential if it is not valid (#2822) Translations 🗣: - diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt index 2ec8900f7c..451fc93fc8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt @@ -20,7 +20,9 @@ import android.content.Context import dagger.Binds import dagger.Module import dagger.Provides +import io.realm.RealmConfiguration import org.matrix.android.sdk.api.auth.AuthenticationService +import org.matrix.android.sdk.api.auth.HomeServerHistoryService import org.matrix.android.sdk.api.legacy.LegacySessionImporter import org.matrix.android.sdk.internal.auth.db.AuthRealmMigration import org.matrix.android.sdk.internal.auth.db.AuthRealmModule @@ -32,8 +34,6 @@ import org.matrix.android.sdk.internal.database.RealmKeysUtils import org.matrix.android.sdk.internal.di.AuthDatabase import org.matrix.android.sdk.internal.legacy.DefaultLegacySessionImporter import org.matrix.android.sdk.internal.wellknown.WellknownModule -import io.realm.RealmConfiguration -import org.matrix.android.sdk.api.auth.HomeServerHistoryService import java.io.File @Module(includes = [WellknownModule::class]) @@ -82,6 +82,9 @@ internal abstract class AuthModule { @Binds abstract fun bindDirectLoginTask(task: DefaultDirectLoginTask): DirectLoginTask + @Binds + abstract fun bindGetVersionTask(task: DefaultIsValidClientServerApiTask): IsValidClientServerApiTask + @Binds abstract fun bindHomeServerHistoryService(service: DefaultHomeServerHistoryService): HomeServerHistoryService } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/IsValidClientServerApiTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/IsValidClientServerApiTask.kt new file mode 100644 index 0000000000..b8416d69bf --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/IsValidClientServerApiTask.kt @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. + * + * 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 org.matrix.android.sdk.internal.auth + +import dagger.Lazy +import okhttp3.OkHttpClient +import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig +import org.matrix.android.sdk.api.failure.Failure +import org.matrix.android.sdk.internal.auth.data.LoginFlowResponse +import org.matrix.android.sdk.internal.di.Unauthenticated +import org.matrix.android.sdk.internal.network.RetrofitFactory +import org.matrix.android.sdk.internal.network.executeRequest +import org.matrix.android.sdk.internal.network.httpclient.addSocketFactory +import org.matrix.android.sdk.internal.task.Task +import javax.inject.Inject +import javax.net.ssl.HttpsURLConnection + +internal interface IsValidClientServerApiTask : Task { + data class Params( + val homeServerConnectionConfig: HomeServerConnectionConfig + ) +} + +internal class DefaultIsValidClientServerApiTask @Inject constructor( + @Unauthenticated + private val okHttpClient: Lazy, + private val retrofitFactory: RetrofitFactory +) : IsValidClientServerApiTask { + + override suspend fun execute(params: IsValidClientServerApiTask.Params): Boolean { + val client = buildClient(params.homeServerConnectionConfig) + val homeServerUrl = params.homeServerConnectionConfig.homeServerUri.toString() + + val authAPI = retrofitFactory.create(client, homeServerUrl) + .create(AuthAPI::class.java) + + return try { + executeRequest(null) { + apiCall = authAPI.getLoginFlows() + } + // We get a response, so the API is valid + true + } catch (failure: Throwable) { + if (failure is Failure.OtherServerError + && failure.httpCode == HttpsURLConnection.HTTP_NOT_FOUND /* 404 */) { + // Probably not valid + false + } else { + // Other error + throw failure + } + } + } + + private fun buildClient(homeServerConnectionConfig: HomeServerConnectionConfig): OkHttpClient { + return okHttpClient.get() + .newBuilder() + .addSocketFactory(homeServerConnectionConfig) + .build() + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/SessionCreator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/SessionCreator.kt index 6743e7336e..7c4a0c38ec 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/SessionCreator.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/SessionCreator.kt @@ -20,6 +20,7 @@ import android.net.Uri import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig import org.matrix.android.sdk.api.auth.data.SessionParams +import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.internal.SessionManager import timber.log.Timber @@ -32,7 +33,8 @@ internal interface SessionCreator { internal class DefaultSessionCreator @Inject constructor( private val sessionParamsStore: SessionParamsStore, private val sessionManager: SessionManager, - private val pendingSessionStore: PendingSessionStore + private val pendingSessionStore: PendingSessionStore, + private val isValidClientServerApiTask: IsValidClientServerApiTask ) : SessionCreator { /** @@ -43,16 +45,28 @@ internal class DefaultSessionCreator @Inject constructor( // We can cleanup the pending session params pendingSessionStore.delete() + val overriddenUrl = credentials.discoveryInformation?.homeServer?.baseURL + // remove trailing "/" + ?.trim { it == '/' } + ?.takeIf { it.isNotBlank() } + ?.also { Timber.d("Overriding homeserver url to $it (will check if valid)") } + ?.let { Uri.parse(it) } + ?.takeIf { + // Validate the URL, if the configuration is wrong server side, do not override + tryOrNull { + isValidClientServerApiTask.execute( + IsValidClientServerApiTask.Params( + homeServerConnectionConfig.copy(homeServerUri = it) + ) + ) + .also { Timber.d("Overriding homeserver url: $it") } + } ?: true // In case of other error (no network, etc.), consider it is valid... + } + val sessionParams = SessionParams( credentials = credentials, homeServerConnectionConfig = homeServerConnectionConfig.copy( - homeServerUri = credentials.discoveryInformation?.homeServer?.baseURL - // remove trailing "/" - ?.trim { it == '/' } - ?.takeIf { it.isNotBlank() } - ?.also { Timber.d("Overriding homeserver url to $it") } - ?.let { Uri.parse(it) } - ?: homeServerConnectionConfig.homeServerUri, + homeServerUri = overriddenUrl ?: homeServerConnectionConfig.homeServerUri, identityServerUri = credentials.discoveryInformation?.identityServer?.baseURL // remove trailing "/" ?.trim { it == '/' } From fcee1f1150519e7f685ac4d32813ba754b1fe587 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 17 Feb 2021 18:15:23 +0100 Subject: [PATCH 4/8] Fix glitch on screen transition --- .../res/layout/fragment_login_signup_signin_selection.xml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/vector/src/main/res/layout/fragment_login_signup_signin_selection.xml b/vector/src/main/res/layout/fragment_login_signup_signin_selection.xml index 56d4e37f1e..f193a217b8 100644 --- a/vector/src/main/res/layout/fragment_login_signup_signin_selection.xml +++ b/vector/src/main/res/layout/fragment_login_signup_signin_selection.xml @@ -81,17 +81,19 @@ app:layout_constraintTop_toBottomOf="@+id/loginSignupSigninSubmit" tools:visibility="visible" /> - + + app:layout_constraintTop_toBottomOf="@id/loginSignupSigninSignIn" + tools:visibility="visible"> Date: Wed, 17 Feb 2021 17:30:13 +0100 Subject: [PATCH 5/8] typo and doc --- .../org/matrix/android/sdk/api/auth/AuthenticationService.kt | 2 +- .../sdk/internal/auth/DefaultAuthenticationService.kt | 5 +++++ .../main/java/im/vector/app/features/login/LoginViewModel.kt | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt index 09ac140db6..a7f5163774 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt @@ -30,7 +30,7 @@ import org.matrix.android.sdk.api.session.Session interface AuthenticationService { /** * Request the supported login flows for this homeserver. - * This is the first method to call to be able to get a wizard to login or the create an account + * This is the first method to call to be able to get a wizard to login or to create an account */ suspend fun getLoginFlow(homeServerConnectionConfig: HomeServerConnectionConfig): LoginFlowResult diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt index ec64b7e757..4f3451cf30 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt @@ -130,6 +130,11 @@ internal class DefaultAuthenticationService @Inject constructor( ?.trim { it == '/' } } + /** + * This is the entry point of the authentication service. + * homeServerConnectionConfig contains a homeserver URL probably entered by the user, which can be a + * valid homeserver API url, the url of Element Web, or anything else. + */ override suspend fun getLoginFlow(homeServerConnectionConfig: HomeServerConnectionConfig): LoginFlowResult { pendingSessionData = null diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt index fa1a504041..72da28230e 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt @@ -146,7 +146,7 @@ class LoginViewModel @AssistedInject constructor( } private fun handleUserAcceptCertificate(action: LoginAction.UserAcceptCertificate) { - // It happen when we get the login flow, or during direct authentication. + // It happens when we get the login flow, or during direct authentication. // So alter the homeserver config and retrieve again the login flow when (val finalLastAction = lastAction) { is LoginAction.UpdateHomeServer -> { From dea76fd81b58006f49eb7ae53d52ba552385c855 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 18 Feb 2021 15:35:54 +0100 Subject: [PATCH 6/8] Fix test compilation --- .../sdk/account/DeactivateAccountTest.kt | 31 ++++++------- .../android/sdk/common/CommonTestHelper.kt | 45 +++++++++---------- .../sdk/session/search/SearchMessagesTest.kt | 3 +- .../im/vector/app/VerificationTestBase.kt | 23 ++++++---- 4 files changed, 50 insertions(+), 52 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/DeactivateAccountTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/DeactivateAccountTest.kt index b0df6fcb44..78b9cb20ed 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/DeactivateAccountTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/account/DeactivateAccountTest.kt @@ -26,15 +26,12 @@ import org.matrix.android.sdk.InstrumentedTest import org.matrix.android.sdk.api.auth.UIABaseAuth import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor import org.matrix.android.sdk.api.auth.UserPasswordAuth -import org.matrix.android.sdk.api.auth.data.LoginFlowResult import org.matrix.android.sdk.api.auth.registration.RegistrationFlowResponse -import org.matrix.android.sdk.api.auth.registration.RegistrationResult import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.failure.MatrixError import org.matrix.android.sdk.common.CommonTestHelper import org.matrix.android.sdk.common.SessionTestParams import org.matrix.android.sdk.common.TestConstants -import org.matrix.android.sdk.common.TestMatrixCallback import kotlin.coroutines.Continuation import kotlin.coroutines.resume @@ -75,23 +72,23 @@ class DeactivateAccountTest : InstrumentedTest { // Try to create an account with the deactivate account user id, it will fail (M_USER_IN_USE) val hs = commonTestHelper.createHomeServerConfig() - commonTestHelper.doSync { - commonTestHelper.matrix.authenticationService.getLoginFlow(hs, it) + commonTestHelper.runBlockingTest { + commonTestHelper.matrix.authenticationService.getLoginFlow(hs) } var accountCreationError: Throwable? = null - commonTestHelper.waitWithLatch { - commonTestHelper.matrix.authenticationService - .getRegistrationWizard() - .createAccount(session.myUserId.substringAfter("@").substringBefore(":"), - TestConstants.PASSWORD, - null, - object : TestMatrixCallback(it, false) { - override fun onFailure(failure: Throwable) { - accountCreationError = failure - super.onFailure(failure) - } - }) + commonTestHelper.runBlockingTest { + try { + commonTestHelper.matrix.authenticationService + .getRegistrationWizard() + .createAccount( + session.myUserId.substringAfter("@").substringBefore(":"), + TestConstants.PASSWORD, + null + ) + } catch (failure: Throwable) { + accountCreationError = failure + } } // Test the error diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt index a4dbd70b11..c677d91f0a 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt @@ -23,7 +23,6 @@ import org.matrix.android.sdk.api.Matrix import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.MatrixConfiguration import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig -import org.matrix.android.sdk.api.auth.data.LoginFlowResult import org.matrix.android.sdk.api.auth.registration.RegistrationResult import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.events.model.EventType @@ -210,22 +209,21 @@ class CommonTestHelper(context: Context) { sessionTestParams: SessionTestParams): Session { val hs = createHomeServerConfig() - doSync { - matrix.authenticationService - .getLoginFlow(hs, it) + runBlockingTest { + matrix.authenticationService.getLoginFlow(hs) } - doSync(timeout = 60_000) { + runBlockingTest(timeout = 60_000) { matrix.authenticationService .getRegistrationWizard() - .createAccount(userName, password, null, it) + .createAccount(userName, password, null) } // Perform dummy step - val registrationResult = doSync(timeout = 60_000) { + val registrationResult = runBlockingTest(timeout = 60_000) { matrix.authenticationService .getRegistrationWizard() - .dummy(it) + .dummy() } assertTrue(registrationResult is RegistrationResult.Success) @@ -249,15 +247,14 @@ class CommonTestHelper(context: Context) { sessionTestParams: SessionTestParams): Session { val hs = createHomeServerConfig() - doSync { - matrix.authenticationService - .getLoginFlow(hs, it) + runBlockingTest { + matrix.authenticationService.getLoginFlow(hs) } - val session = doSync { + val session = runBlockingTest { matrix.authenticationService .getLoginWizard() - .login(userName, password, "myDevice", it) + .login(userName, password, "myDevice") } if (sessionTestParams.withInitialSync) { @@ -277,21 +274,19 @@ class CommonTestHelper(context: Context) { password: String): Throwable { val hs = createHomeServerConfig() - doSync { - matrix.authenticationService - .getLoginFlow(hs, it) + runBlockingTest { + matrix.authenticationService.getLoginFlow(hs) } var requestFailure: Throwable? = null - waitWithLatch { latch -> - matrix.authenticationService - .getLoginWizard() - .login(userName, password, "myDevice", object : TestMatrixCallback(latch, onlySuccessful = false) { - override fun onFailure(failure: Throwable) { - requestFailure = failure - super.onFailure(failure) - } - }) + runBlockingTest { + try { + matrix.authenticationService + .getLoginWizard() + .login(userName, password, "myDevice") + } catch (failure: Throwable) { + requestFailure = failure + } } assertNotNull(requestFailure) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/search/SearchMessagesTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/search/SearchMessagesTest.kt index ae300c936d..cadb83ca00 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/search/SearchMessagesTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/search/SearchMessagesTest.kt @@ -61,7 +61,7 @@ class SearchMessagesTest : InstrumentedTest { 2) run { - var lock = CountDownLatch(1) + val lock = CountDownLatch(1) val eventListener = commonTestHelper.createEventListener(lock) { snapshot -> snapshot.count { it.root.content.toModel()?.body?.startsWith(MESSAGE).orFalse() } == 2 @@ -70,7 +70,6 @@ class SearchMessagesTest : InstrumentedTest { aliceTimeline.addListener(eventListener) commonTestHelper.await(lock) - lock = CountDownLatch(1) val data = commonTestHelper.runBlockingTest { aliceSession .searchService() diff --git a/vector/src/androidTest/java/im/vector/app/VerificationTestBase.kt b/vector/src/androidTest/java/im/vector/app/VerificationTestBase.kt index a4b9983ff4..285f40aaf3 100644 --- a/vector/src/androidTest/java/im/vector/app/VerificationTestBase.kt +++ b/vector/src/androidTest/java/im/vector/app/VerificationTestBase.kt @@ -23,11 +23,11 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import org.junit.Assert import org.matrix.android.sdk.api.Matrix import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig -import org.matrix.android.sdk.api.auth.data.LoginFlowResult import org.matrix.android.sdk.api.auth.registration.RegistrationResult import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.sync.SyncState @@ -47,22 +47,21 @@ abstract class VerificationTestBase { withInitialSync: Boolean): Session { val hs = createHomeServerConfig() - doSync { - matrix.authenticationService() - .getLoginFlow(hs, it) + runBlockingTest { + matrix.authenticationService().getLoginFlow(hs) } - doSync { + runBlockingTest { matrix.authenticationService() .getRegistrationWizard() - .createAccount(userName, password, null, it) + .createAccount(userName, password, null) } // Perform dummy step - val registrationResult = doSync { + val registrationResult = runBlockingTest { matrix.authenticationService() .getRegistrationWizard() - .dummy(it) + .dummy() } Assert.assertTrue(registrationResult is RegistrationResult.Success) @@ -80,6 +79,14 @@ abstract class VerificationTestBase { .build() } + protected fun runBlockingTest(timeout: Long = 20_000, block: suspend () -> T): T { + return runBlocking { + withTimeout(timeout) { + block() + } + } + } + // Transform a method with a MatrixCallback to a synchronous method inline fun doSync(block: (MatrixCallback) -> Unit): T { val lock = CountDownLatch(1) From b9f5863b531494b66f8175d6114b2526dc5c6929 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 18 Feb 2021 16:14:02 +0100 Subject: [PATCH 7/8] Avoid long lines --- .../app/features/login/LoginViewModel.kt | 61 +++++++++---------- .../signout/soft/SoftLogoutViewModel.kt | 37 ++++++----- 2 files changed, 46 insertions(+), 52 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt index 72da28230e..792c74e019 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt @@ -759,7 +759,7 @@ class LoginViewModel @AssistedInject constructor( ) } - try { + val data = try { authenticationService.getLoginFlow(homeServerConnectionConfig) } catch (failure: Throwable) { _viewEvents.post(LoginViewEvents.Failure(failure)) @@ -772,39 +772,36 @@ class LoginViewModel @AssistedInject constructor( } null } - ?.let { data -> - // Valid Homeserver, add it to the history. - // Note: we add what the user has input, data.homeServerUrl can be different - rememberHomeServer(homeServerConnectionConfig.homeServerUri.toString()) - when (data) { - is LoginFlowResult.Success -> { - val loginMode = when { - // SSO login is taken first - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) - && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password - else -> LoginMode.Unsupported - } + if (data is LoginFlowResult.Success) { + // Valid Homeserver, add it to the history. + // Note: we add what the user has input, data.homeServerUrl can be different + rememberHomeServer(homeServerConnectionConfig.homeServerUri.toString()) - // FIXME We should post a view event here normally? - setState { - copy( - asyncHomeServerLoginFlowRequest = Uninitialized, - homeServerUrl = data.homeServerUrl, - loginMode = loginMode, - loginModeSupportedTypes = data.supportedLoginTypes.toList() - ) - } - if ((loginMode == LoginMode.Password && !data.isLoginAndRegistrationSupported) - || data.isOutdatedHomeserver) { - // Notify the UI - _viewEvents.post(LoginViewEvents.OutdatedHomeserver) - } - } - } - } + val loginMode = when { + // SSO login is taken first + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) + && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password + else -> LoginMode.Unsupported + } + + // FIXME We should post a view event here normally? + setState { + copy( + asyncHomeServerLoginFlowRequest = Uninitialized, + homeServerUrl = data.homeServerUrl, + loginMode = loginMode, + loginModeSupportedTypes = data.supportedLoginTypes.toList() + ) + } + if ((loginMode == LoginMode.Password && !data.isLoginAndRegistrationSupported) + || data.isOutdatedHomeserver) { + // Notify the UI + _viewEvents.post(LoginViewEvents.OutdatedHomeserver) + } + } } } diff --git a/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt b/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt index ab8fb292da..2c0b78a546 100644 --- a/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt @@ -89,7 +89,7 @@ class SoftLogoutViewModel @AssistedInject constructor( ) } - try { + val data = try { authenticationService.getLoginFlowOfSession(session.sessionId) } catch (failure: Throwable) { setState { @@ -99,26 +99,23 @@ class SoftLogoutViewModel @AssistedInject constructor( } null } - ?.let { data -> - when (data) { - is LoginFlowResult.Success -> { - val loginMode = when { - // SSO login is taken first - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) - && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) - data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password - else -> LoginMode.Unsupported - } - setState { - copy( - asyncHomeServerLoginFlowRequest = Success(loginMode) - ) - } - } - } - } + if (data is LoginFlowResult.Success) { + val loginMode = when { + // SSO login is taken first + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) + && data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.SsoAndPassword(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.SSO) -> LoginMode.Sso(data.ssoIdentityProviders) + data.supportedLoginTypes.contains(LoginFlowTypes.PASSWORD) -> LoginMode.Password + else -> LoginMode.Unsupported + } + + setState { + copy( + asyncHomeServerLoginFlowRequest = Success(loginMode) + ) + } + } } } From e8026c6d3fa3bf57b590e17e85daec4687bac8c9 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 19 Feb 2021 13:53:38 +0100 Subject: [PATCH 8/8] Naming convention --- .../java/org/matrix/android/sdk/internal/auth/AuthModule.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt index 451fc93fc8..bb62dbbfe9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/AuthModule.kt @@ -83,7 +83,7 @@ internal abstract class AuthModule { abstract fun bindDirectLoginTask(task: DefaultDirectLoginTask): DirectLoginTask @Binds - abstract fun bindGetVersionTask(task: DefaultIsValidClientServerApiTask): IsValidClientServerApiTask + abstract fun bindIsValidClientServerApiTask(task: DefaultIsValidClientServerApiTask): IsValidClientServerApiTask @Binds abstract fun bindHomeServerHistoryService(service: DefaultHomeServerHistoryService): HomeServerHistoryService