Improve Git server config activity (#1051)

2020-08-24 12:03:33 +02:00 · 2020-08-24 12:03:33 +02:00 · d0b15cec49
commit d0b15cec49
parent cbe780f31c
6 changed files with 94 additions and 44 deletions
--- 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

--- 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)
--- 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" }
    }
 }
--- 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
--- 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
    }
 }
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@ -328,6 +328,8 @@
    <string name="connection_mode_openkeychain" translatable="false">OpenKeychain</string>
    <string name="git_server_config_save_success">Successfully saved configuration</string>
    <string name="git_server_config_save_error">The provided repository URL is not valid</string>
+    <string name="git_server_config_save_missing_username_https">Please specify the HTTPS username in the form https://username@example.com/…</string>
+    <string name="git_server_config_save_missing_username_ssh">Please specify the SSH username in the form username@example.com:…</string>
    <string name="git_config_error_hostname_empty">empty hostname</string>
    <string name="git_config_error_generic">please verify your settings and try again</string>
    <string name="git_config_error_nonnumeric_port">port must be numeric</string>