Fix protocol validation (#726)

* GitOperation: code cleanup

* WIP: Fix validation

* Fixup SSH validation

* Spotless

* Remove logging of MalformedURLException

Co-authored-by: Fabian Henneke <fabian@henneke.me>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>

3 files changed, 36 insertions, 37 deletions

diff --git a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
index 8ccdd0c8..2cfeebae 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
@@ -4,6 +4,7 @@
  */
 package com.zeapo.pwdstore.git
 
+import android.app.Activity
 import android.content.Intent
 import android.content.SharedPreferences
 import android.os.Bundle
@@ -13,6 +14,7 @@ import androidx.appcompat.app.AppCompatActivity
 import androidx.core.content.edit
 import androidx.core.text.isDigitsOnly
 import androidx.preference.PreferenceManager
+import com.github.ajalt.timberkt.e
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import com.zeapo.pwdstore.git.config.ConnectionMode
 import com.zeapo.pwdstore.git.config.Protocol
@@ -21,7 +23,7 @@ import com.zeapo.pwdstore.utils.PasswordRepository
 import com.zeapo.pwdstore.utils.getEncryptedPrefs
 import java.io.File
 import java.net.MalformedURLException
-import java.net.URL
+import java.net.URI
 import timber.log.Timber
 
 /**
@@ -31,7 +33,7 @@ import timber.log.Timber
 abstract class BaseGitActivity : AppCompatActivity() {
     lateinit var protocol: Protocol
     lateinit var connectionMode: ConnectionMode
-    lateinit var url: String
+    var url: String? = null
     lateinit var serverHostname: String
     lateinit var serverPort: String
     lateinit var serverUser: String
@@ -90,48 +92,43 @@ abstract class BaseGitActivity : AppCompatActivity() {
         if (serverHostname.isEmpty() || !serverPort.isDigitsOnly())
             return false
 
-        val previousUrl = if (::url.isInitialized) url else ""
+        val previousUrl = url ?: ""
         val hostnamePart = serverHostname
         val pathPart = if (serverPath.startsWith('/')) serverPath else "/$serverPath"
-        url = when (protocol) {
+        val newUrl = when (protocol) {
             Protocol.Ssh -> {
                 val userPart = if (serverUser.isEmpty()) "" else "$serverUser@"
                 val portPart =
                     if (serverPort == "22" || serverPort.isEmpty()) "" else ":$serverPort"
+                if (hostnamePart.startsWith("ssh://"))
+                    hostnamePart.replace("ssh://", "")
                 // We have to specify the ssh scheme as this is the only way to pass a custom port.
-                val urlWithFreeEntryScheme = "$userPart$hostnamePart$portPart$pathPart"
-                val parsedUrl = try {
-                    URL(urlWithFreeEntryScheme)
-                } catch (_: MalformedURLException) {
-                    return false
-                }
-                if (parsedUrl.protocol == null)
-                    "ssh://$urlWithFreeEntryScheme"
-                else
-                    urlWithFreeEntryScheme
+                "ssh://$userPart$hostnamePart$portPart$pathPart"
             }
             Protocol.Https -> {
                 val portPart =
                     if (serverPort == "443" || serverPort.isEmpty()) "" else ":$serverPort"
                 val urlWithFreeEntryScheme = "$hostnamePart$portPart$pathPart"
-                val parsedUrl = try {
-                    URL(urlWithFreeEntryScheme)
-                } catch (_: MalformedURLException) {
-                    return false
-                }
-                when (parsedUrl.protocol) {
-                    null -> "https://$urlWithFreeEntryScheme"
-                    "http" -> urlWithFreeEntryScheme.replaceFirst("http:", "https:")
-                    else -> urlWithFreeEntryScheme
+                when {
+                    urlWithFreeEntryScheme.startsWith("https://") -> urlWithFreeEntryScheme
+                    urlWithFreeEntryScheme.startsWith("http://") -> urlWithFreeEntryScheme.replaceFirst("http", "https")
+                    else -> "https://$urlWithFreeEntryScheme"
                 }
             }
         }
+        try {
+            if (URI(newUrl).rawAuthority == null)
+                return false
+        } catch (_: MalformedURLException) {
+            return false
+        }
         if (PasswordRepository.isInitialized)
-            PasswordRepository.addRemote("origin", url, true)
+            PasswordRepository.addRemote("origin", newUrl, true)
         // HTTPS authentication sends the password to the server, so we must wipe the password when
         // the server is changed.
-        if (url != previousUrl && protocol == Protocol.Https)
+        if (newUrl != previousUrl && protocol == Protocol.Https)
             encryptedSettings.edit { remove("https_password") }
+        url = newUrl
         return true
     }
 
@@ -145,8 +142,11 @@ abstract class BaseGitActivity : AppCompatActivity() {
      * @param operation The type of git operation to launch
      */
     fun launchGitOperation(operation: Int) {
-        val op: GitOperation
-        val localDir = requireNotNull(PasswordRepository.getRepositoryDirectory(this))
+        if (url == null) {
+            setResult(Activity.RESULT_CANCELED)
+            finish()
+            return
+        }
         try {
             // Before launching the operation with OpenKeychain auth, we need to issue several requests
             // to the OpenKeychain API. IdentityBuild will take care of launching the relevant intents,
@@ -163,8 +163,9 @@ abstract class BaseGitActivity : AppCompatActivity() {
                     return
             }
 
-            op = when (operation) {
-                REQUEST_CLONE, GitOperation.GET_SSH_KEY_FROM_CLONE -> CloneOperation(localDir, this).setCommand(url)
+            val localDir = requireNotNull(PasswordRepository.getRepositoryDirectory(this))
+            val op = when (operation) {
+                REQUEST_CLONE, GitOperation.GET_SSH_KEY_FROM_CLONE -> CloneOperation(localDir, this).setCommand(url!!)
                 REQUEST_PULL -> PullOperation(localDir, this).setCommand()
                 REQUEST_PUSH -> PushOperation(localDir, this).setCommand()
                 REQUEST_SYNC -> SyncOperation(localDir, this).setCommands()
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitOperation.kt
index 78a9ca69..e115c482 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/GitOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/GitOperation.kt
@@ -183,9 +183,8 @@ abstract class GitOperation(fileDir: File, internal val callingActivity: Activit
                                     .setView(dialogView)
                                     .setPositiveButton(callingActivity.resources.getString(R.string.dialog_ok)) { _, _ ->
                                         if (keyPair.decrypt(passphrase.text.toString())) {
-                                            val rememberPassphrase = dialogView.findViewById<MaterialCheckBox>(R.id.git_auth_remember_passphrase).isChecked
-                                            if (rememberPassphrase) {
-                                                encryptedSettings.edit().putString("ssh_key_local_passphrase", passphrase.text.toString()).apply()
+                                            if (dialogView.findViewById<MaterialCheckBox>(R.id.git_auth_remember_passphrase).isChecked) {
+                                                encryptedSettings.edit { putString("ssh_key_local_passphrase", passphrase.text.toString()) }
                                             }
                                             // Authenticate using the ssh-key and then execute the command
                                             setAuthentication(sshKey, username, passphrase.text.toString()).execute()
@@ -233,9 +232,8 @@ abstract class GitOperation(fileDir: File, internal val callingActivity: Activit
                         .setMessage(callingActivity.resources.getString(R.string.password_dialog_text))
                         .setView(dialogView)
                         .setPositiveButton(callingActivity.resources.getString(R.string.dialog_ok)) { _, _ ->
-                            val rememberPassphrase = dialogView.findViewById<MaterialCheckBox>(R.id.git_auth_remember_passphrase).isChecked
-                            if (rememberPassphrase) {
-                                encryptedSettings.edit().putString("https_password", passwordView.text.toString()).apply()
+                            if (dialogView.findViewById<MaterialCheckBox>(R.id.git_auth_remember_passphrase).isChecked) {
+                                encryptedSettings.edit { putString("https_password", passwordView.text.toString()) }
                             }
                             // authenticate using the user/pwd and then execute the command
                             setAuthentication(username, passwordView.text.toString()).execute()
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitOperationActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitOperationActivity.kt
index bf7d5acc..ec73dae6 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/GitOperationActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/GitOperationActivity.kt
@@ -50,7 +50,7 @@ open class GitOperationActivity : BaseGitActivity() {
      * @param operation the operation to execute can be REQUEST_PULL or REQUEST_PUSH
      */
     private fun syncRepository(operation: Int) {
-        if (serverUser.isEmpty() || serverHostname.isEmpty() || url.isEmpty())
+        if (serverUser.isEmpty() || serverHostname.isEmpty() || url.isNullOrEmpty())
             MaterialAlertDialogBuilder(this)
                     .setMessage(getString(R.string.set_information_dialog_text))
                     .setPositiveButton(getString(R.string.dialog_positive)) { _, _ ->
@@ -65,7 +65,7 @@ open class GitOperationActivity : BaseGitActivity() {
                     .show()
         else {
             // check that the remote origin is here, else add it
-            PasswordRepository.addRemote("origin", url, true)
+            PasswordRepository.addRemote("origin", url!!, true)
             launchGitOperation(operation)
         }
     }