From d0b15cec49a41ec1ea51890ac7b7c8cb8515f6a4 Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Mon, 24 Aug 2020 12:03:33 +0200 Subject: [PATCH] Improve Git server config activity (#1051) --- CHANGELOG.md | 5 ++ .../java/com/zeapo/pwdstore/MigrationsTest.kt | 11 ++- .../java/com/zeapo/pwdstore/Migrations.kt | 6 +- .../pwdstore/git/GitServerConfigActivity.kt | 83 ++++++++++++------- .../zeapo/pwdstore/git/config/GitSettings.kt | 31 +++++-- app/src/main/res/values/strings.xml | 2 + 6 files changed, 94 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3e4118c..52236278 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,14 @@ All notable changes to this project will be documented in this file. - Allow sorting by recently used - Add [Bromite](https://www.bromite.org/) and [Ungoogled Chromium](https://git.droidware.info/wchen342/ungoogled-chromium-android) to supported browsers list for Autofill +### Changed + +- A descriptive error message is shown if no username is specified in the Git server settings + ### Fixed - Password creation UI will scroll if it does not fit on the screen +- Git server protocol and authentication mode are only updated when explicitly saved ## [1.11.2] - 2020-08-24 diff --git a/app/src/androidTest/java/com/zeapo/pwdstore/MigrationsTest.kt b/app/src/androidTest/java/com/zeapo/pwdstore/MigrationsTest.kt index d1b04fc3..c87bdae3 100644 --- a/app/src/androidTest/java/com/zeapo/pwdstore/MigrationsTest.kt +++ b/app/src/androidTest/java/com/zeapo/pwdstore/MigrationsTest.kt @@ -8,6 +8,8 @@ package com.zeapo.pwdstore import android.content.Context import androidx.core.content.edit +import com.zeapo.pwdstore.git.config.ConnectionMode +import com.zeapo.pwdstore.git.config.Protocol import com.zeapo.pwdstore.utils.PreferenceKeys import com.zeapo.pwdstore.utils.getString import com.zeapo.pwdstore.utils.sharedPrefs @@ -33,7 +35,8 @@ class MigrationsTest { putString(PreferenceKeys.GIT_REMOTE_USERNAME, "msfjarvis") putString(PreferenceKeys.GIT_REMOTE_LOCATION, "/mnt/disk3/pass-repo") putString(PreferenceKeys.GIT_REMOTE_SERVER, "192.168.0.102") - putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, "ssh://") + putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, Protocol.Ssh.pref) + putString(PreferenceKeys.GIT_REMOTE_AUTH, ConnectionMode.Password.pref) } runMigrations(context) checkOldKeysAreRemoved(context) @@ -51,7 +54,8 @@ class MigrationsTest { putString(PreferenceKeys.GIT_REMOTE_USERNAME, "msfjarvis") putString(PreferenceKeys.GIT_REMOTE_LOCATION, "/mnt/disk3/pass-repo") putString(PreferenceKeys.GIT_REMOTE_SERVER, "192.168.0.102") - putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, "ssh://") + putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, Protocol.Ssh.pref) + putString(PreferenceKeys.GIT_REMOTE_AUTH, ConnectionMode.SshKey.pref) } runMigrations(context) checkOldKeysAreRemoved(context) @@ -69,7 +73,8 @@ class MigrationsTest { putString(PreferenceKeys.GIT_REMOTE_USERNAME, "msfjarvis") putString(PreferenceKeys.GIT_REMOTE_LOCATION, "Android-Password-Store/pass-test") putString(PreferenceKeys.GIT_REMOTE_SERVER, "github.com") - putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, "https://") + putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, Protocol.Https.pref) + putString(PreferenceKeys.GIT_REMOTE_AUTH, ConnectionMode.None.pref) } runMigrations(context) checkOldKeysAreRemoved(context) diff --git a/app/src/main/java/com/zeapo/pwdstore/Migrations.kt b/app/src/main/java/com/zeapo/pwdstore/Migrations.kt index 1e1943d8..d0221d9e 100644 --- a/app/src/main/java/com/zeapo/pwdstore/Migrations.kt +++ b/app/src/main/java/com/zeapo/pwdstore/Migrations.kt @@ -76,7 +76,11 @@ private fun migrateToGitUrlBasedConfig(context: Context) { remove(PreferenceKeys.GIT_REMOTE_SERVER) remove(PreferenceKeys.GIT_REMOTE_USERNAME) } - if (url == null || !GitSettings.updateUrlIfValid(url)) { + if (url == null || GitSettings.updateConnectionSettingsIfValid( + newProtocol = protocol, + newConnectionMode = GitSettings.connectionMode, + newUrl = url, + newBranch = GitSettings.branch) != GitSettings.UpdateConnectionSettingsResult.Valid) { e { "Failed to migrate to URL-based Git config, generated URL is invalid" } } } diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt index 82e97e17..bb991449 100644 --- a/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt @@ -32,6 +32,9 @@ class GitServerConfigActivity : BaseGitActivity() { private val binding by viewBinding(ActivityGitCloneBinding::inflate) + private lateinit var newProtocol: Protocol + private lateinit var newConnectionMode: ConnectionMode + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) val isClone = intent?.extras?.getInt(REQUEST_ARG_OP) ?: -1 == REQUEST_CLONE @@ -41,22 +44,26 @@ class GitServerConfigActivity : BaseGitActivity() { setContentView(binding.root) supportActionBar?.setDisplayHomeAsUpEnabled(true) - binding.cloneProtocolGroup.check(when (GitSettings.protocol) { - Protocol.Ssh -> R.id.clone_protocol_ssh - Protocol.Https -> R.id.clone_protocol_https - }) - binding.cloneProtocolGroup.addOnButtonCheckedListener { _, checkedId, checked -> - if (checked) { - when (checkedId) { - R.id.clone_protocol_https -> GitSettings.protocol = Protocol.Https - R.id.clone_protocol_ssh -> GitSettings.protocol = Protocol.Ssh + newProtocol = GitSettings.protocol + binding.cloneProtocolGroup.apply { + when (newProtocol) { + Protocol.Ssh -> check(R.id.clone_protocol_ssh) + Protocol.Https -> check(R.id.clone_protocol_https) + } + addOnButtonCheckedListener { _, checkedId, checked -> + if (checked) { + when (checkedId) { + R.id.clone_protocol_https -> newProtocol = Protocol.Https + R.id.clone_protocol_ssh -> newProtocol = Protocol.Ssh + } + updateConnectionModeToggleGroup() } - updateConnectionModeToggleGroup() } } + newConnectionMode = GitSettings.connectionMode binding.connectionModeGroup.apply { - when (GitSettings.connectionMode) { + when (newConnectionMode) { ConnectionMode.SshKey -> check(R.id.connection_mode_ssh_key) ConnectionMode.Password -> check(R.id.connection_mode_password) ConnectionMode.OpenKeychain -> check(R.id.connection_mode_open_keychain) @@ -64,10 +71,10 @@ class GitServerConfigActivity : BaseGitActivity() { } addOnButtonCheckedListener { _, _, _ -> when (checkedButtonId) { - R.id.connection_mode_ssh_key -> GitSettings.connectionMode = ConnectionMode.SshKey - R.id.connection_mode_open_keychain -> GitSettings.connectionMode = ConnectionMode.OpenKeychain - R.id.connection_mode_password -> GitSettings.connectionMode = ConnectionMode.Password - View.NO_ID -> GitSettings.connectionMode = ConnectionMode.None + R.id.connection_mode_ssh_key -> newConnectionMode = ConnectionMode.SshKey + R.id.connection_mode_open_keychain -> newConnectionMode = ConnectionMode.OpenKeychain + R.id.connection_mode_password -> newConnectionMode = ConnectionMode.Password + View.NO_ID -> newConnectionMode = ConnectionMode.None } } } @@ -77,36 +84,50 @@ class GitServerConfigActivity : BaseGitActivity() { binding.serverBranch.setText(GitSettings.branch) binding.saveButton.setOnClickListener { - if (isClone && PasswordRepository.getRepository(null) == null) - PasswordRepository.initialize() - GitSettings.branch = binding.serverBranch.text.toString().trim() - if (GitSettings.updateUrlIfValid(binding.serverUrl.text.toString().trim())) { - if (!isClone) { - Snackbar.make(binding.root, getString(R.string.git_server_config_save_success), Snackbar.LENGTH_SHORT).show() - Handler().postDelayed(500) { finish() } - } else { - cloneRepository() + when (GitSettings.updateConnectionSettingsIfValid( + newProtocol = newProtocol, + newConnectionMode = newConnectionMode, + newUrl = binding.serverUrl.text.toString().trim(), + newBranch = binding.serverBranch.text.toString().trim())) { + GitSettings.UpdateConnectionSettingsResult.FailedToParseUrl -> { + Snackbar.make(binding.root, getString(R.string.git_server_config_save_error), Snackbar.LENGTH_LONG).show() + } + GitSettings.UpdateConnectionSettingsResult.MissingUsername -> { + when (newProtocol) { + Protocol.Https -> Snackbar.make(binding.root, getString(R.string.git_server_config_save_missing_username_https), Snackbar.LENGTH_LONG).show() + Protocol.Ssh -> Snackbar.make(binding.root, getString(R.string.git_server_config_save_missing_username_ssh), Snackbar.LENGTH_LONG).show() + } + } + else -> { + if (isClone && PasswordRepository.getRepository(null) == null) + PasswordRepository.initialize() + if (!isClone) { + Snackbar.make(binding.root, getString(R.string.git_server_config_save_success), Snackbar.LENGTH_SHORT).show() + Handler().postDelayed(500) { finish() } + } else { + cloneRepository() + } } - } else { - Snackbar.make(binding.root, getString(R.string.git_server_config_save_error), Snackbar.LENGTH_LONG).show() } } } private fun updateConnectionModeToggleGroup() { - if (GitSettings.protocol == Protocol.Ssh) { - // Reset connection mode to SSH key if the current value (none) is not valid for SSH - if (binding.connectionModeGroup.checkedButtonIds.isEmpty()) - binding.connectionModeGroup.check(R.id.connection_mode_ssh_key) + if (newProtocol == Protocol.Ssh) { binding.connectionModeSshKey.isEnabled = true binding.connectionModeOpenKeychain.isEnabled = true + // Reset connection mode to SSH key if the current value (none) is not valid for SSH. + // Important note: This has to happen after enabling the other toggle buttons or they + // won't check. + if (binding.connectionModeGroup.checkedButtonIds.isEmpty()) + binding.connectionModeGroup.check(R.id.connection_mode_ssh_key) binding.connectionModeGroup.isSelectionRequired = true } else { binding.connectionModeGroup.isSelectionRequired = false // Reset connection mode to password if the current value is not valid for HTTPS // Important note: This has to happen before disabling the other toggle buttons or they // won't uncheck. - if (GitSettings.connectionMode !in listOf(ConnectionMode.None, ConnectionMode.Password)) + if (newConnectionMode !in listOf(ConnectionMode.None, ConnectionMode.Password)) binding.connectionModeGroup.check(R.id.connection_mode_password) binding.connectionModeSshKey.isEnabled = false binding.connectionModeOpenKeychain.isEnabled = false diff --git a/app/src/main/java/com/zeapo/pwdstore/git/config/GitSettings.kt b/app/src/main/java/com/zeapo/pwdstore/git/config/GitSettings.kt index b81a8f8f..2efae40c 100644 --- a/app/src/main/java/com/zeapo/pwdstore/git/config/GitSettings.kt +++ b/app/src/main/java/com/zeapo/pwdstore/git/config/GitSettings.kt @@ -55,14 +55,14 @@ object GitSettings { var protocol get() = Protocol.fromString(settings.getString(PreferenceKeys.GIT_REMOTE_PROTOCOL)) - set(value) { + private set(value) { settings.edit { putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, value.pref) } } var connectionMode get() = ConnectionMode.fromString(settings.getString(PreferenceKeys.GIT_REMOTE_AUTH)) - set(value) { + private set(value) { settings.edit { putString(PreferenceKeys.GIT_REMOTE_AUTH, value.pref) } @@ -71,6 +71,8 @@ object GitSettings { get() = settings.getString(PreferenceKeys.GIT_REMOTE_URL) private set(value) { require(value != null) + if (value == url) + return settings.edit { putString(PreferenceKeys.GIT_REMOTE_URL, value) } @@ -96,20 +98,31 @@ object GitSettings { } var branch get() = settings.getString(PreferenceKeys.GIT_BRANCH_NAME) ?: DEFAULT_BRANCH - set(value) { + private set(value) { settings.edit { putString(PreferenceKeys.GIT_BRANCH_NAME, value) } } - fun updateUrlIfValid(newUrl: String): Boolean { - try { + enum class UpdateConnectionSettingsResult { + Valid, + FailedToParseUrl, + MissingUsername, + } + + fun updateConnectionSettingsIfValid(newProtocol: Protocol, newConnectionMode: ConnectionMode, newUrl: String, newBranch: String): UpdateConnectionSettingsResult { + val parsedUrl = try { URIish(newUrl) } catch (_: Exception) { - return false + return UpdateConnectionSettingsResult.FailedToParseUrl } - if (newUrl != url) - url = newUrl - return true + if (newConnectionMode != ConnectionMode.None && parsedUrl.user.isNullOrBlank()) + return UpdateConnectionSettingsResult.MissingUsername + + url = newUrl + protocol = newProtocol + connectionMode = newConnectionMode + branch = newBranch + return UpdateConnectionSettingsResult.Valid } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 1b32e29c..86e1c722 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -328,6 +328,8 @@ OpenKeychain Successfully saved configuration The provided repository URL is not valid + Please specify the HTTPS username in the form https://username@example.com/… + Please specify the SSH username in the form username@example.com:… empty hostname please verify your settings and try again port must be numeric