Disable SSH multiplexing if not supported (#1093)

Co-authored-by: Harsh Shandilya <me@msfjarvis.dev>

5 files changed, 44 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ef33a641..7672575f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,7 @@ All notable changes to this project will be documented in this file.
 -   Unable to use show/hide password option for password/passphrase after first attempt was wrong
 -   TOTP values shown might some times be stale and considered invalid by sites
 -   Symlinks are no longer clobbered by the app (only available on Android 8 and above)
+-   Workaround lack of SSH connection reuse capabilities on some Git hosts like Bitbucket
 
 ## [1.11.3] - 2020-08-27
 
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 a15972c1..19413efd 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
@@ -11,6 +11,8 @@ import com.github.ajalt.timberkt.d
 import com.github.ajalt.timberkt.e
 import com.github.michaelbull.result.Err
 import com.github.michaelbull.result.Result
+import com.github.michaelbull.result.andThen
+import com.github.michaelbull.result.mapError
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import com.zeapo.pwdstore.R
 import com.zeapo.pwdstore.git.config.GitSettings
@@ -42,6 +44,11 @@ abstract class BaseGitActivity : AppCompatActivity() {
         if (GitSettings.url == null) {
             return Err(IllegalStateException("Git url is not set!"))
         }
+        if (operation == REQUEST_SYNC && !GitSettings.useMultiplexing) {
+            // If the server does not support multiple SSH channels per connection, we cannot run
+            // a sync operation without reconnecting and thus break sync into its two parts.
+            return launchGitOperation(REQUEST_PULL).andThen { launchGitOperation(REQUEST_PUSH) }
+        }
         val op = when (operation) {
             REQUEST_CLONE, GitOperation.GET_SSH_KEY_FROM_CLONE -> CloneOperation(this, GitSettings.url!!)
             REQUEST_PULL -> PullOperation(this)
@@ -54,7 +61,15 @@ abstract class BaseGitActivity : AppCompatActivity() {
                 return Err(IllegalArgumentException("$operation is not a valid Git operation"))
             }
         }
-        return op.executeAfterAuthentication(GitSettings.authMode)
+        return op.executeAfterAuthentication(GitSettings.authMode).mapError { throwable ->
+            val err = rootCauseException(throwable)
+            if (err.message?.contains("cannot open additional channels") == true) {
+                GitSettings.useMultiplexing = false
+                SSHException(DisconnectReason.TOO_MANY_CONNECTIONS, "The server does not support multiple Git operations per SSH session. Please try again, a slower fallback mode will be used.")
+            } else {
+                err
+            }
+        }
     }
 
     fun finishOnSuccessHandler(@Suppress("UNUSED_PARAMETER") nothing: Unit) {
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 1169345b..f4cbc04f 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
@@ -73,7 +73,9 @@ object GitSettings {
             }
             if (PasswordRepository.isInitialized)
                 PasswordRepository.addRemote("origin", value, true)
-            // When the server changes, remote password and host key file should be deleted.
+            // When the server changes, remote password, multiplexing support and host key file
+            // should be deleted/reset.
+            useMultiplexing = true
             encryptedSettings.edit { remove(PreferenceKeys.HTTPS_PASSWORD) }
             File("${Application.instance.filesDir}/.host_key").delete()
         }
@@ -98,6 +100,13 @@ object GitSettings {
                 putString(PreferenceKeys.GIT_BRANCH_NAME, value)
             }
         }
+    var useMultiplexing
+        get() = settings.getBoolean(PreferenceKeys.GIT_REMOTE_USE_MULTIPLEXING, true)
+        set(value) {
+            settings.edit {
+                putBoolean(PreferenceKeys.GIT_REMOTE_USE_MULTIPLEXING, value)
+            }
+        }
 
     sealed class UpdateConnectionSettingsResult {
         class MissingUsername(val newProtocol: Protocol) : UpdateConnectionSettingsResult()
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/PullOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/PullOperation.kt
index d4ce6d01..8173105c 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/operation/PullOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/PullOperation.kt
@@ -9,7 +9,23 @@ import org.eclipse.jgit.api.GitCommand
 
 class PullOperation(callingActivity: AppCompatActivity) : GitOperation(callingActivity) {
 
+    /**
+     * The story of why the pull operation is committing files goes like this: Once upon a time when
+     * the world was burning and Blade Runner 2049 was real life (in the worst way), we were made
+     * aware that Bitbucket is actually bad, and disables a neat OpenSSH feature called multiplexing.
+     * So now, rather than being able to do a [SyncOperation], we'd have to first do a [PullOperation]
+     * and then a [PushOperation]. To make the behavior identical despite this suboptimal situation,
+     * we opted to replicate [SyncOperation]'s committing flow within [PullOperation], almost exactly
+     * replicating [SyncOperation] but leaving the pushing part to [PushOperation].
+     */
     override val commands: Array<GitCommand<out Any>> = arrayOf(
+        // Stage all files
+        git.add().addFilepattern("."),
+        // Populate the changed files count
+        git.status(),
+        // Commit everything! If needed, obviously.
+        git.commit().setAll(true).setMessage("[Android Password Store] Sync"),
+        // Pull and rebase on top of the remote branch
         git.pull().setRebase(true).setRemote("origin"),
     )
 }
diff --git a/app/src/main/java/com/zeapo/pwdstore/utils/PreferenceKeys.kt b/app/src/main/java/com/zeapo/pwdstore/utils/PreferenceKeys.kt
index 8180ccc0..1b2c7abb 100644
--- a/app/src/main/java/com/zeapo/pwdstore/utils/PreferenceKeys.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/utils/PreferenceKeys.kt
@@ -32,6 +32,7 @@ object PreferenceKeys {
 
     @Deprecated("Use GIT_REMOTE_URL instead")
     const val GIT_REMOTE_LOCATION = "git_remote_location"
+    const val GIT_REMOTE_USE_MULTIPLEXING = "git_remote_use_multiplexing"
 
     @Deprecated("Use GIT_REMOTE_URL instead")
     const val GIT_REMOTE_PORT = "git_remote_port"