Remove protocol selection from server config

The protocol is now extracted from the URL, and the authentication mode selection is validated by GitSettings

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>

app/src/androidTest/java/com/zeapo/pwdstore/MigrationsTest.kt

@@ -24,6 +24,7 @@ class MigrationsTest {
         assertNull(getString(PreferenceKeys.GIT_REMOTE_USERNAME))
         assertNull(getString(PreferenceKeys.GIT_REMOTE_SERVER))
         assertNull(getString(PreferenceKeys.GIT_REMOTE_LOCATION))
+        assertNull(getString(PreferenceKeys.GIT_REMOTE_PROTOCOL))
     }
 
     @Test

app/src/main/java/com/zeapo/pwdstore/Migrations.kt

@@ -75,13 +75,12 @@ private fun migrateToGitUrlBasedConfig(context: Context) {
         remove(PreferenceKeys.GIT_REMOTE_PORT)
         remove(PreferenceKeys.GIT_REMOTE_SERVER)
         remove(PreferenceKeys.GIT_REMOTE_USERNAME)
+        remove(PreferenceKeys.GIT_REMOTE_PROTOCOL)
     }
     if (url == null || GitSettings.updateConnectionSettingsIfValid(
-            newProtocol = protocol,
             newAuthMode = GitSettings.authMode,
             newUrl = url,
             newBranch = GitSettings.branch) != GitSettings.UpdateConnectionSettingsResult.Valid) {
         e { "Failed to migrate to URL-based Git config, generated URL is invalid" }
     }
 }
-

app/src/main/java/com/zeapo/pwdstore/git/GitServerConfigActivity.kt

@@ -32,7 +32,6 @@ class GitServerConfigActivity : BaseGitActivity() {
 
     private val binding by viewBinding(ActivityGitCloneBinding::inflate)
 
-    private lateinit var newProtocol: Protocol
     private lateinit var newAuthMode: AuthMode
 
     override fun onCreate(savedInstanceState: Bundle?) {
@@ -44,24 +43,8 @@ class GitServerConfigActivity : BaseGitActivity() {
         setContentView(binding.root)
         supportActionBar?.setDisplayHomeAsUpEnabled(true)
 
-        newProtocol = GitSettings.protocol
-        binding.protocolGroup.apply {
-            when (newProtocol) {
-                Protocol.Ssh -> check(R.id.protocol_ssh)
-                Protocol.Https -> check(R.id.protocol_https)
-            }
-            addOnButtonCheckedListener { _, checkedId, checked ->
-                if (checked) {
-                    when (checkedId) {
-                        R.id.protocol_https -> newProtocol = Protocol.Https
-                        R.id.protocol_ssh -> newProtocol = Protocol.Ssh
-                    }
-                    updateAuthModeToggleGroup()
-                }
-            }
-        }
-
         newAuthMode = GitSettings.authMode
+
         binding.authModeGroup.apply {
             when (newAuthMode) {
                 AuthMode.SshKey -> check(R.id.auth_mode_ssh_key)
@@ -78,22 +61,20 @@ class GitServerConfigActivity : BaseGitActivity() {
                 }
             }
         }
-        updateAuthModeToggleGroup()
 
         binding.serverUrl.setText(GitSettings.url)
         binding.serverBranch.setText(GitSettings.branch)
 
         binding.saveButton.setOnClickListener {
-            when (GitSettings.updateConnectionSettingsIfValid(
-                newProtocol = newProtocol,
+            when (val updateResult = GitSettings.updateConnectionSettingsIfValid(
                 newAuthMode = newAuthMode,
                 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) {
+                is GitSettings.UpdateConnectionSettingsResult.MissingUsername -> {
+                    when (updateResult.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()
                     }
@@ -111,32 +92,18 @@ class GitServerConfigActivity : BaseGitActivity() {
                         cloneRepository()
                     }
                 }
+                is GitSettings.UpdateConnectionSettingsResult.AuthModeMismatch -> {
+                    val message = getString(
+                        R.string.git_server_config_save_auth_mode_mismatch,
+                        updateResult.newProtocol,
+                        updateResult.validModes.joinToString(", ") { it.pref },
+                    )
+                    Snackbar.make(binding.root, message, Snackbar.LENGTH_LONG).show()
+                }
             }
         }
     }
 
-    private fun updateAuthModeToggleGroup() {
-        if (newProtocol == Protocol.Ssh) {
-            binding.authModeSshKey.isEnabled = true
-            binding.authModeOpenKeychain.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.authModeGroup.checkedButtonIds.isEmpty())
-                binding.authModeGroup.check(R.id.auth_mode_ssh_key)
-            binding.authModeGroup.isSelectionRequired = true
-        } else {
-            binding.authModeGroup.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 (newAuthMode !in listOf(AuthMode.None, AuthMode.Password))
-                binding.authModeGroup.check(R.id.auth_mode_password)
-            binding.authModeSshKey.isEnabled = false
-            binding.authModeOpenKeychain.isEnabled = false
-        }
-    }
-
     /**
      * Clones the repository, the directory exists, deletes it
      */

app/src/main/java/com/zeapo/pwdstore/git/config/GitSettings.kt

@@ -53,13 +53,6 @@ object GitSettings {
     private val settings by lazy { Application.instance.sharedPrefs }
     private val encryptedSettings by lazy { Application.instance.getEncryptedPrefs("git_operation") }
 
-    var protocol
-        get() = Protocol.fromString(settings.getString(PreferenceKeys.GIT_REMOTE_PROTOCOL))
-        private set(value) {
-            settings.edit {
-                putString(PreferenceKeys.GIT_REMOTE_PROTOCOL, value.pref)
-            }
-        }
     var authMode
         get() = AuthMode.fromString(settings.getString(PreferenceKeys.GIT_REMOTE_AUTH))
         private set(value) {
@@ -104,28 +97,40 @@ object GitSettings {
             }
         }
 
-    enum class UpdateConnectionSettingsResult {
-        Valid,
-        FailedToParseUrl,
-        MissingUsername,
-        ProtocolMismatch,
+    sealed class UpdateConnectionSettingsResult {
+        class MissingUsername(val newProtocol: Protocol) : UpdateConnectionSettingsResult()
+        class AuthModeMismatch(val newProtocol: Protocol, val validModes: List<AuthMode>) : UpdateConnectionSettingsResult()
+        object Valid : UpdateConnectionSettingsResult()
+        object FailedToParseUrl : UpdateConnectionSettingsResult()
+        object ProtocolMismatch : UpdateConnectionSettingsResult()
     }
 
-    fun updateConnectionSettingsIfValid(newProtocol: Protocol, newAuthMode: AuthMode, newUrl: String, newBranch: String): UpdateConnectionSettingsResult {
+    fun updateConnectionSettingsIfValid(newAuthMode: AuthMode, newUrl: String, newBranch: String): UpdateConnectionSettingsResult {
         val parsedUrl = try {
             URIish(newUrl)
         } catch (_: Exception) {
             return UpdateConnectionSettingsResult.FailedToParseUrl
         }
+        val newProtocol = when (parsedUrl.scheme) {
+            in listOf("http", "https") -> Protocol.Https
+            in listOf("ssh") -> Protocol.Ssh
+            else -> return UpdateConnectionSettingsResult.FailedToParseUrl
+        }
         if (newAuthMode != AuthMode.None && parsedUrl.user.isNullOrBlank())
-            return UpdateConnectionSettingsResult.MissingUsername
-        when (newProtocol) {
-            Protocol.Https -> if (parsedUrl.scheme !in listOf("http", "https")) return UpdateConnectionSettingsResult.ProtocolMismatch
-            Protocol.Ssh -> if (parsedUrl.scheme !in listOf(null, "git")) return UpdateConnectionSettingsResult.ProtocolMismatch
+            return UpdateConnectionSettingsResult.MissingUsername(newProtocol)
+
+        val validHttpsAuth = listOf(AuthMode.None, AuthMode.Password)
+        val validSshAuth = listOf(AuthMode.OpenKeychain, AuthMode.Password, AuthMode.SshKey)
+        when {
+            newProtocol == Protocol.Https && newAuthMode !in validHttpsAuth -> {
+                return UpdateConnectionSettingsResult.AuthModeMismatch(newProtocol, validHttpsAuth)
+            }
+            newProtocol == Protocol.Ssh && newAuthMode !in validSshAuth -> {
+                return UpdateConnectionSettingsResult.AuthModeMismatch(newProtocol, validSshAuth)
+            }
         }
 
         url = newUrl
-        protocol = newProtocol
         authMode = newAuthMode
         branch = newBranch
         return UpdateConnectionSettingsResult.Valid

app/src/main/java/com/zeapo/pwdstore/utils/PreferenceKeys.kt

@@ -32,6 +32,7 @@ object PreferenceKeys {
     const val GIT_REMOTE_LOCATION = "git_remote_location"
     @Deprecated("Use GIT_REMOTE_URL instead")
     const val GIT_REMOTE_PORT = "git_remote_port"
+    @Deprecated("Use GIT_REMOTE_URL instead")
     const val GIT_REMOTE_PROTOCOL = "git_remote_protocol"
     const val GIT_DELETE_REPO = "git_delete_repo"
     @Deprecated("Use GIT_REMOTE_URL instead")

app/src/main/res/layout/activity_git_clone.xml

@@ -44,8 +44,8 @@
                 android:layout_width="match_parent"
                 android:layout_height="wrap_content"
                 android:imeOptions="actionNext"
-                android:nextFocusForward="@id/server_branch"
-                android:inputType="textWebEmailAddress" />
+                android:inputType="textWebEmailAddress"
+                android:nextFocusForward="@id/server_branch" />
 
         </com.google.android.material.textfield.TextInputLayout>
 
@@ -55,55 +55,19 @@
             android:layout_height="wrap_content"
             android:layout_margin="8dp"
             android:hint="@string/server_branch"
-            app:layout_constraintStart_toStartOf="parent"
             app:layout_constraintEnd_toEndOf="parent"
+            app:layout_constraintStart_toStartOf="parent"
             app:layout_constraintTop_toBottomOf="@id/label_server_url">
 
             <com.google.android.material.textfield.TextInputEditText
                 android:id="@+id/server_branch"
-                android:imeOptions="actionDone"
                 android:layout_width="match_parent"
-                android:inputType="textNoSuggestions"
-                android:layout_height="wrap_content" />
+                android:layout_height="wrap_content"
+                android:imeOptions="actionDone"
+                android:inputType="textNoSuggestions" />
 
         </com.google.android.material.textfield.TextInputLayout>
 
-        <androidx.appcompat.widget.AppCompatTextView
-            android:id="@+id/label_server_protocol"
-            style="@style/TextAppearance.MaterialComponents.Headline6"
-            android:layout_width="0dp"
-            android:layout_height="wrap_content"
-            android:layout_margin="8dp"
-            android:text="@string/server_protocol"
-            app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toBottomOf="@id/label_server_branch" />
-
-        <com.google.android.material.button.MaterialButtonToggleGroup
-            android:id="@+id/protocol_group"
-            style="@style/TextAppearance.MaterialComponents.Headline1"
-            android:layout_width="0dp"
-            android:layout_height="wrap_content"
-            android:layout_margin="8dp"
-            app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toBottomOf="@id/label_server_protocol"
-            app:selectionRequired="true"
-            app:singleSelection="true">
-
-            <com.google.android.material.button.MaterialButton
-                android:id="@+id/protocol_ssh"
-                style="?attr/materialButtonOutlinedStyle"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:text="@string/clone_protocol_ssh" />
-
-            <com.google.android.material.button.MaterialButton
-                android:id="@+id/protocol_https"
-                style="?attr/materialButtonOutlinedStyle"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:text="@string/clone_protocol_https" />
-        </com.google.android.material.button.MaterialButtonToggleGroup>
-
         <androidx.appcompat.widget.AppCompatTextView
             android:id="@+id/label_auth_mode"
             style="@style/TextAppearance.MaterialComponents.Headline6"
@@ -114,7 +78,7 @@
             android:layout_marginBottom="16dp"
             android:text="@string/connection_mode"
             app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toBottomOf="@id/protocol_group" />
+            app:layout_constraintTop_toBottomOf="@id/label_server_branch" />
 
         <com.google.android.material.button.MaterialButtonToggleGroup
             android:id="@+id/auth_mode_group"

app/src/main/res/values/strings.xml

@@ -331,6 +331,7 @@
     <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_server_config_save_protocol_mismatch">The provided repository URL\'s scheme does not match the selected protocol</string>
+    <string name="git_server_config_save_auth_mode_mismatch">Valid authentication modes for %1$s: %2$s</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>