From 3aa37e5d3c0d4c3158c4c1c56c2300be4a1fcc73 Mon Sep 17 00:00:00 2001 From: Ryan Harg Date: Tue, 10 Aug 2021 14:54:37 +0200 Subject: [PATCH] #80: Display error messages for user when login failes --- .../funkwhale/ffa/activities/LoginActivity.kt | 102 ++++++++++-------- .../audio/funkwhale/ffa/utils/Extensions.kt | 5 + .../audio/funkwhale/ffa/utils/FuelResult.kt | 22 ++++ .../java/audio/funkwhale/ffa/utils/OAuth.kt | 64 ++++++----- app/src/main/res/values-de/strings.xml | 2 + app/src/main/res/values/colors.xml | 2 +- app/src/main/res/values/strings.xml | 2 + changes/changelog.d/80.feature | 1 + 8 files changed, 119 insertions(+), 81 deletions(-) create mode 100644 app/src/main/java/audio/funkwhale/ffa/utils/FuelResult.kt create mode 100644 changes/changelog.d/80.feature diff --git a/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt b/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt index 123f79e..4b7c0c0 100644 --- a/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt +++ b/app/src/main/java/audio/funkwhale/ffa/activities/LoginActivity.kt @@ -11,10 +11,7 @@ import androidx.lifecycle.lifecycleScope import audio.funkwhale.ffa.R import audio.funkwhale.ffa.databinding.ActivityLoginBinding import audio.funkwhale.ffa.fragments.LoginDialog -import audio.funkwhale.ffa.utils.AppContext -import audio.funkwhale.ffa.utils.OAuth -import audio.funkwhale.ffa.utils.Userinfo -import audio.funkwhale.ffa.utils.log +import audio.funkwhale.ffa.utils.* import com.github.kittinunf.fuel.Fuel import com.github.kittinunf.fuel.coroutines.awaitObjectResponseResult import com.github.kittinunf.fuel.gson.gsonDeserializerOf @@ -22,6 +19,7 @@ import com.github.kittinunf.result.Result import com.preference.PowerPreference import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import org.koin.java.KoinJavaComponent.inject data class FwCredentials(val token: String, val non_field_errors: List?) @@ -74,27 +72,27 @@ class LoginActivity : AppCompatActivity() { var hostname = hostname.text.toString().trim() try { - if (hostname.isEmpty()) throw Exception(getString(R.string.login_error_hostname)) + validateHostname(hostname, cleartext.isChecked)?.let { + hostnameField.error = it + return@setOnClickListener + } - Uri.parse(hostname).apply { - if (!cleartext.isChecked && scheme == "http") { - throw Exception(getString(R.string.login_error_hostname_https)) - } - - if (scheme == null) { - hostname = when (cleartext.isChecked) { - true -> "http://$hostname" - false -> "https://$hostname" - } + val uri = Uri.parse(hostname) + if (uri.scheme == null) { + hostname = when (cleartext.isChecked) { + true -> "http://$hostname" + false -> "https://$hostname" } } hostnameField.error = "" - when (anonymous.isChecked) { + val fuelResult = when (anonymous.isChecked) { false -> authedLogin(hostname) true -> anonymousLogin(hostname) } + + hostnameField.error = mapFuelResultToError(fuelResult) } catch (e: Exception) { val message = if (e.message?.isEmpty() == true) getString(R.string.login_error_hostname) @@ -106,56 +104,66 @@ class LoginActivity : AppCompatActivity() { } } + private fun mapFuelResultToError(fuelResult: FuelResult) = when { + fuelResult.httpStatus == 404 -> + getString(R.string.login_error_funkwhale_not_found) + + !fuelResult.success -> + getString(R.string.login_error, fuelResult.message) + + else -> "" + } + override fun onConfigurationChanged(newConfig: Configuration) { super.onConfigurationChanged(newConfig) limitContainerWidth() } - private fun authedLogin(hostname: String) { - PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS).setString("hostname", hostname) + private fun validateHostname(hostname: String, cleartext: Boolean): String? { + if (hostname.isEmpty()) { + return getString(R.string.login_error_hostname) + } + if (!cleartext && hostname.startsWith("http")) { + return getString(R.string.login_error_hostname_https) + } + return null + } + + private fun authedLogin(hostname: String): FuelResult { oAuth.init(hostname) - oAuth.register { + return oAuth.register { + PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS).setString("hostname", hostname) oAuth.authorize(this) } } - private fun anonymousLogin(hostname: String) { + private fun anonymousLogin(hostname: String): FuelResult { val dialog = LoginDialog().apply { show(supportFragmentManager, "LoginDialog") } - lifecycleScope.launch(Main) { - try { - val (_, _, result) = Fuel.get("$hostname/api/v1/tracks/") - .awaitObjectResponseResult(gsonDeserializerOf(FwCredentials::class.java)) + val uri = "$hostname/api/v1/tracks/" + val (_, _, result) = runBlocking { + Fuel.get(uri).awaitObjectResponseResult(gsonDeserializerOf(FwCredentials::class.java)) + } - when (result) { - is Result.Success -> { - PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS).apply { - setString("hostname", hostname) - setBoolean("anonymous", true) - } - - dialog.dismiss() - startActivity(Intent(this@LoginActivity, MainActivity::class.java)) - finish() - } - - is Result.Failure -> { - dialog.dismiss() - - binding.hostnameField.error = result.error.localizedMessage - } + when (result) { + is Result.Success -> { + PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS).apply { + setString("hostname", hostname) + setBoolean("anonymous", true) } - } catch (e: Exception) { + dialog.dismiss() + startActivity(Intent(this@LoginActivity, MainActivity::class.java)) + finish() + return FuelResult.ok() + } - val message = - if (e.message?.isEmpty() == true) getString(R.string.login_error_hostname) - else e.message - - binding.hostnameField.error = message + is Result.Failure -> { + dialog.dismiss() + return FuelResult.from(result) } } } diff --git a/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt b/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt index ab083aa..5ac84d8 100644 --- a/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt +++ b/app/src/main/java/audio/funkwhale/ffa/utils/Extensions.kt @@ -8,6 +8,7 @@ import androidx.fragment.app.Fragment import audio.funkwhale.ffa.R import audio.funkwhale.ffa.fragments.BrowseFragment import audio.funkwhale.ffa.repositories.Repository +import com.github.kittinunf.fuel.core.FuelError import com.github.kittinunf.fuel.core.Request import com.google.android.exoplayer2.offline.Download import com.google.gson.Gson @@ -93,5 +94,9 @@ fun Request.authorize(context: Context, oAuth: OAuth): Request { } } +fun FuelError.formatResponseMessage(): String { + return "${response.statusCode}: ${response.url}" +} + fun Download.getMetadata(): DownloadInfo? = Gson().fromJson(String(this.request.data), DownloadInfo::class.java) diff --git a/app/src/main/java/audio/funkwhale/ffa/utils/FuelResult.kt b/app/src/main/java/audio/funkwhale/ffa/utils/FuelResult.kt new file mode 100644 index 0000000..3429eb1 --- /dev/null +++ b/app/src/main/java/audio/funkwhale/ffa/utils/FuelResult.kt @@ -0,0 +1,22 @@ +package audio.funkwhale.ffa.utils + +import com.github.kittinunf.fuel.core.FuelError +import com.github.kittinunf.result.Result + +data class FuelResult(val httpStatus: Int? = null, val message: String? = null) { + + val success: Boolean get() = httpStatus == 200 + + companion object { + + fun ok() = FuelResult(200) + + fun from(result: Result.Failure): FuelResult { + return FuelResult(result.error.response.statusCode, result.error.response.responseMessage) + } + + fun failure(): FuelResult { + return FuelResult() + } + } +} \ No newline at end of file diff --git a/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt b/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt index 6a7ca00..cbcddfb 100644 --- a/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt +++ b/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt @@ -13,16 +13,7 @@ import com.github.kittinunf.fuel.gson.jsonBody import com.github.kittinunf.result.Result import com.preference.PowerPreference import kotlinx.coroutines.runBlocking -import net.openid.appauth.AuthState -import net.openid.appauth.AuthorizationException -import net.openid.appauth.AuthorizationRequest -import net.openid.appauth.AuthorizationResponse -import net.openid.appauth.AuthorizationService -import net.openid.appauth.AuthorizationServiceConfiguration -import net.openid.appauth.ClientSecretPost -import net.openid.appauth.RegistrationRequest -import net.openid.appauth.RegistrationResponse -import net.openid.appauth.ResponseTypeValues +import net.openid.appauth.* fun AuthState.save() { PowerPreference.getFileByName(AppContext.PREFS_CREDENTIALS).apply { @@ -84,8 +75,8 @@ class OAuth(private val authorizationServiceFactory: AuthorizationServiceFactory state: AuthState, context: Context ): Boolean { - if (state.needsTokenRefresh.also { it.log("needsTokenRefresh()") } - && state.refreshToken != null) { + if (state.needsTokenRefresh.also { it.log("needsTokenRefresh()") } && + state.refreshToken != null) { val refreshRequest = state.createTokenRefreshRequest() val auth = ClientSecretPost(state.clientSecret) runBlocking { @@ -119,39 +110,46 @@ class OAuth(private val authorizationServiceFactory: AuthorizationServiceFactory fun service(context: Context): AuthorizationService = authorizationServiceFactory.create(context) - fun register(authState: AuthState? = null, callback: () -> Unit) { + fun register(authState: AuthState? = null, callback: () -> Unit): FuelResult { (authState ?: state()).authorizationServiceConfiguration?.let { config -> - runBlocking { - val (_, _, result: Result) = Fuel.post(config.registrationEndpoint.toString()) + + val (_, _, result: Result) = runBlocking { + Fuel.post(config.registrationEndpoint.toString()) .header("Content-Type", "application/json") .jsonBody(registrationBody()) .awaitObjectResponseResult(gsonDeserializerOf(App::class.java)) + } - when (result) { - is Result.Success -> { - val app = result.get() + when (result) { + is Result.Success -> { + Log.i("OAuth", "OAuth client app created.") + val app = result.get() - val response = RegistrationResponse.Builder(registration()!!) - .setClientId(app.client_id) - .setClientSecret(app.client_secret) - .setClientIdIssuedAt(0) - .setClientSecretExpiresAt(null) - .build() + val response = RegistrationResponse.Builder(registration()!!) + .setClientId(app.client_id) + .setClientSecret(app.client_secret) + .setClientIdIssuedAt(0) + .setClientSecretExpiresAt(null) + .build() - state().apply { - update(response) - save() - - callback() - } + state().apply { + update(response) + save() + callback() + return FuelResult.ok() } + } - is Result.Failure -> { - result.log("register()") - } + is Result.Failure -> { + Log.i( + "OAuth", "Couldn't register client application ${result.error.formatResponseMessage()}" + ) + return FuelResult.from(result) } } } + Log.i("OAuth", "Missing AuthorizationServiceConfiguration") + return FuelResult.failure() } private fun registrationBody(): Map { diff --git a/app/src/main/res/values-de/strings.xml b/app/src/main/res/values-de/strings.xml index cb388df..4fa1722 100644 --- a/app/src/main/res/values-de/strings.xml +++ b/app/src/main/res/values-de/strings.xml @@ -7,6 +7,8 @@ Passwort Anmelden Einloggen + Anmeldung fehlgeschlagen: %s + Keine Funkwhale Pod gefunden Das ist keine gültige URL Der Zugriff auf den Funkwhale Server sollte über HTTPS erfolgen Suche diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml index a0c0b10..3bbec61 100644 --- a/app/src/main/res/values/colors.xml +++ b/app/src/main/res/values/colors.xml @@ -6,7 +6,7 @@ #476d85 #646568 #d35400 - #fdcfbb + #F75D28 #dadada #d35400 diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ef3deba..8aa9aa2 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -9,6 +9,8 @@ Password Log in Logging in + Login failed: %s + No Funkwhale pod found This could not be understood as a valid URL The Funkwhale hostname should be secure through HTTPS We could not retrieve information about your user diff --git a/changes/changelog.d/80.feature b/changes/changelog.d/80.feature new file mode 100644 index 0000000..35998db --- /dev/null +++ b/changes/changelog.d/80.feature @@ -0,0 +1 @@ +Show error messages to user when login failes (#80) \ No newline at end of file