Workaround AndroidX lifecycle requirements in OpenKeychain auth (#1168)

* Workaround AndroidX lifecycle requirements in OpenKeychain auth

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

* CHANGELOG: add OpenKeychain fix

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

Co-authored-by: Fabian Henneke <FabianHenneke@users.noreply.github.com>

12 files changed, 70 insertions, 44 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 61c0135f..f43615cc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Fixed
+
+- OpenKeychain authentication would fail with `LifecycleOwner com.zeapo.pwdstore.git.GitServerConfigActivity@f578da1 is attempting to register while current state is RESUMED. LifecycleOwners must call register before they are STARTED.`
+
 ### Added
 
 -  Add support for domain-level autofill in DuckDuckGo's F-Droid builds.
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 36b8b6fb..b47647c1 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/BaseGitActivity.kt
@@ -20,6 +20,7 @@ import com.zeapo.pwdstore.git.operation.PullOperation
 import com.zeapo.pwdstore.git.operation.PushOperation
 import com.zeapo.pwdstore.git.operation.ResetToRemoteOperation
 import com.zeapo.pwdstore.git.operation.SyncOperation
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 import com.zeapo.pwdstore.utils.PreferenceKeys
 import com.zeapo.pwdstore.utils.getEncryptedGitPrefs
 import com.zeapo.pwdstore.utils.sharedPrefs
@@ -33,7 +34,7 @@ import net.schmizz.sshj.userauth.UserAuthException
  * Abstract [AppCompatActivity] that holds some information that is commonly shared across git-related
  * tasks and makes sense to be held here.
  */
-abstract class BaseGitActivity : AppCompatActivity() {
+abstract class BaseGitActivity : ContinuationContainerActivity() {
 
     /**
      * Enum of possible Git operations than can be run through [launchGitOperation].
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/BreakOutOfDetached.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/BreakOutOfDetached.kt
index 6c7fee3a..f7759ada 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/operation/BreakOutOfDetached.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/BreakOutOfDetached.kt
@@ -4,12 +4,12 @@
  */
 package com.zeapo.pwdstore.git.operation
 
-import androidx.appcompat.app.AppCompatActivity
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import com.zeapo.pwdstore.R
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 import org.eclipse.jgit.api.RebaseCommand
 
-class BreakOutOfDetached(callingActivity: AppCompatActivity) : GitOperation(callingActivity) {
+class BreakOutOfDetached(callingActivity: ContinuationContainerActivity) : GitOperation(callingActivity) {
 
     override val commands = arrayOf(
         // abort the rebase
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/CloneOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/CloneOperation.kt
index 256437a8..2a94a283 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/operation/CloneOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/CloneOperation.kt
@@ -4,7 +4,7 @@
  */
 package com.zeapo.pwdstore.git.operation
 
-import androidx.appcompat.app.AppCompatActivity
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 import org.eclipse.jgit.api.Git
 import org.eclipse.jgit.api.GitCommand
 
@@ -14,7 +14,7 @@ import org.eclipse.jgit.api.GitCommand
  * @param uri URL to clone the repository from
  * @param callingActivity the calling activity
  */
-class CloneOperation(callingActivity: AppCompatActivity, uri: String) : GitOperation(callingActivity) {
+class CloneOperation(callingActivity: ContinuationContainerActivity, uri: String) : GitOperation(callingActivity) {
 
     override val commands: Array<GitCommand<out Any>> = arrayOf(
         Git.cloneRepository().setBranch(remoteBranch).setDirectory(repository.workTree).setURI(uri),
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt
index d7ff7c4c..491feaf0 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/GitOperation.kt
@@ -19,6 +19,7 @@ import com.zeapo.pwdstore.UserPreference
 import com.zeapo.pwdstore.git.GitCommandExecutor
 import com.zeapo.pwdstore.git.config.AuthMode
 import com.zeapo.pwdstore.git.config.GitSettings
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 import com.zeapo.pwdstore.git.sshj.SshAuthMethod
 import com.zeapo.pwdstore.git.sshj.SshKey
 import com.zeapo.pwdstore.git.sshj.SshjSessionFactory
@@ -55,6 +56,7 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
     protected val repository = PasswordRepository.getRepository(null)!!
     protected val git = Git(repository)
     protected val remoteBranch = GitSettings.branch
+    private val authActivity get() = callingActivity as ContinuationContainerActivity
 
     private class HttpsCredentialsProvider(private val passwordFinder: PasswordFinder) : CredentialsProvider() {
 
@@ -154,7 +156,7 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
                     }
                     when (result) {
                         is BiometricAuthenticator.Result.Success -> {
-                            registerAuthProviders(SshAuthMethod.SshKey(callingActivity))
+                            registerAuthProviders(SshAuthMethod.SshKey(authActivity))
                         }
                         is BiometricAuthenticator.Result.Cancelled -> {
                             return Err(SSHException(DisconnectReason.AUTH_CANCELLED_BY_USER))
@@ -172,7 +174,7 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
                         }
                     }
                 } else {
-                    registerAuthProviders(SshAuthMethod.SshKey(callingActivity))
+                    registerAuthProviders(SshAuthMethod.SshKey(authActivity))
                 }
             } else {
                 onMissingSshKeyFile()
@@ -180,10 +182,10 @@ abstract class GitOperation(protected val callingActivity: FragmentActivity) {
                 // error, allowing users to make the SSH key selection.
                 return Err(SSHException(DisconnectReason.AUTH_CANCELLED_BY_USER))
             }
-            AuthMode.OpenKeychain -> registerAuthProviders(SshAuthMethod.OpenKeychain(callingActivity))
+            AuthMode.OpenKeychain -> registerAuthProviders(SshAuthMethod.OpenKeychain(authActivity))
             AuthMode.Password -> {
                 val httpsCredentialProvider = HttpsCredentialsProvider(CredentialFinder(callingActivity, AuthMode.Password))
-                registerAuthProviders(SshAuthMethod.Password(callingActivity), httpsCredentialProvider)
+                registerAuthProviders(SshAuthMethod.Password(authActivity), httpsCredentialProvider)
             }
             AuthMode.None -> {
             }
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 8173105c..4dcc3ec4 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
@@ -4,10 +4,10 @@
  */
 package com.zeapo.pwdstore.git.operation
 
-import androidx.appcompat.app.AppCompatActivity
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 import org.eclipse.jgit.api.GitCommand
 
-class PullOperation(callingActivity: AppCompatActivity) : GitOperation(callingActivity) {
+class PullOperation(callingActivity: ContinuationContainerActivity) : GitOperation(callingActivity) {
 
     /**
      * The story of why the pull operation is committing files goes like this: Once upon a time when
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/PushOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/PushOperation.kt
index d3bee209..28d7a6fa 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/operation/PushOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/PushOperation.kt
@@ -4,10 +4,10 @@
  */
 package com.zeapo.pwdstore.git.operation
 
-import androidx.appcompat.app.AppCompatActivity
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 import org.eclipse.jgit.api.GitCommand
 
-class PushOperation(callingActivity: AppCompatActivity) : GitOperation(callingActivity) {
+class PushOperation(callingActivity: ContinuationContainerActivity) : GitOperation(callingActivity) {
 
     override val commands: Array<GitCommand<out Any>> = arrayOf(
         git.push().setPushAll().setRemote("origin"),
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/ResetToRemoteOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/ResetToRemoteOperation.kt
index 2df7a2b4..ad0fb823 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/operation/ResetToRemoteOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/ResetToRemoteOperation.kt
@@ -4,10 +4,10 @@
  */
 package com.zeapo.pwdstore.git.operation
 
-import androidx.appcompat.app.AppCompatActivity
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 import org.eclipse.jgit.api.ResetCommand
 
-class ResetToRemoteOperation(callingActivity: AppCompatActivity) : GitOperation(callingActivity) {
+class ResetToRemoteOperation(callingActivity: ContinuationContainerActivity) : GitOperation(callingActivity) {
 
     override val commands = arrayOf(
         // Stage all files
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/operation/SyncOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/operation/SyncOperation.kt
index a0a80fe3..8c1ec1fa 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/operation/SyncOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/operation/SyncOperation.kt
@@ -4,9 +4,9 @@
  */
 package com.zeapo.pwdstore.git.operation
 
-import androidx.appcompat.app.AppCompatActivity
+import com.zeapo.pwdstore.git.sshj.ContinuationContainerActivity
 
-class SyncOperation(callingActivity: AppCompatActivity) : GitOperation(callingActivity) {
+class SyncOperation(callingActivity: ContinuationContainerActivity) : GitOperation(callingActivity) {
 
     override val commands = arrayOf(
         // Stage all files
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/sshj/ContinuationContainerActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/sshj/ContinuationContainerActivity.kt
new file mode 100644
index 00000000..32ee64a8
--- /dev/null
+++ b/app/src/main/java/com/zeapo/pwdstore/git/sshj/ContinuationContainerActivity.kt
@@ -0,0 +1,37 @@
+/*
+ * Copyright © 2014-2020 The Android Password Store Authors. All Rights Reserved.
+ * SPDX-License-Identifier: GPL-3.0-only
+ */
+package com.zeapo.pwdstore.git.sshj
+
+import android.content.Intent
+import androidx.activity.result.contract.ActivityResultContracts
+import androidx.annotation.LayoutRes
+import androidx.appcompat.app.AppCompatActivity
+import kotlin.coroutines.Continuation
+import kotlin.coroutines.resume
+import kotlin.coroutines.resumeWithException
+import net.schmizz.sshj.common.DisconnectReason
+import net.schmizz.sshj.userauth.UserAuthException
+
+/**
+ * Workaround for https://msfjarvis.dev/aps/issue/1164
+ */
+open class ContinuationContainerActivity : AppCompatActivity {
+
+    constructor() : super()
+    constructor(@LayoutRes layoutRes: Int) : super(layoutRes)
+
+    var stashedCont: Continuation<Intent>? = null
+
+    val continueAfterUserInteraction = registerForActivityResult(ActivityResultContracts.StartIntentSenderForResult()) { result ->
+        stashedCont?.let { cont ->
+            stashedCont = null
+            val data = result.data
+            if (data != null)
+                cont.resume(data)
+            else
+                cont.resumeWithException(UserAuthException(DisconnectReason.AUTH_CANCELLED_BY_USER))
+        }
+    }
+}
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/sshj/OpenKeychainKeyProvider.kt b/app/src/main/java/com/zeapo/pwdstore/git/sshj/OpenKeychainKeyProvider.kt
index cdaa0342..97774d92 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/sshj/OpenKeychainKeyProvider.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/sshj/OpenKeychainKeyProvider.kt
@@ -7,18 +7,14 @@ package com.zeapo.pwdstore.git.sshj
 import android.app.PendingIntent
 import android.content.Intent
 import androidx.activity.result.IntentSenderRequest
-import androidx.activity.result.contract.ActivityResultContracts
 import androidx.core.content.edit
-import androidx.fragment.app.FragmentActivity
 import com.github.ajalt.timberkt.d
 import com.zeapo.pwdstore.utils.OPENPGP_PROVIDER
 import com.zeapo.pwdstore.utils.PreferenceKeys
 import com.zeapo.pwdstore.utils.sharedPrefs
 import java.io.Closeable
 import java.security.PublicKey
-import kotlin.coroutines.Continuation
 import kotlin.coroutines.resume
-import kotlin.coroutines.resumeWithException
 import kotlin.coroutines.suspendCoroutine
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
@@ -39,11 +35,11 @@ import org.openintents.ssh.authentication.response.Response
 import org.openintents.ssh.authentication.response.SigningResponse
 import org.openintents.ssh.authentication.response.SshPublicKeyResponse
 
-class OpenKeychainKeyProvider private constructor(activity: FragmentActivity) : KeyProvider, Closeable {
+class OpenKeychainKeyProvider private constructor(val activity: ContinuationContainerActivity) : KeyProvider, Closeable {
 
     companion object {
 
-        suspend fun prepareAndUse(activity: FragmentActivity, block: (provider: OpenKeychainKeyProvider) -> Unit) {
+        suspend fun prepareAndUse(activity: ContinuationContainerActivity, block: (provider: OpenKeychainKeyProvider) -> Unit) {
             withContext(Dispatchers.Main) {
                 OpenKeychainKeyProvider(activity)
             }.prepareAndUse(block)
@@ -59,21 +55,8 @@ class OpenKeychainKeyProvider private constructor(activity: FragmentActivity) :
     private val context = activity.applicationContext
     private val sshServiceConnection = SshAuthenticationConnection(context, OPENPGP_PROVIDER)
     private val preferences = context.sharedPrefs
-    private val continueAfterUserInteraction =
-        activity.registerForActivityResult(ActivityResultContracts.StartIntentSenderForResult()) { result ->
-            currentCont?.let { cont ->
-                currentCont = null
-                val data = result.data
-                if (data != null)
-                    cont.resume(data)
-                else
-                    cont.resumeWithException(UserAuthException(DisconnectReason.AUTH_CANCELLED_BY_USER))
-            }
-        }
-
     private lateinit var sshServiceApi: SshAuthenticationApi
 
-    private var currentCont: Continuation<Intent>? = null
     private var keyId
         get() = preferences.getString(PreferenceKeys.SSH_OPENKEYSTORE_KEYID, null)
         set(value) {
@@ -164,8 +147,8 @@ class OpenKeychainKeyProvider private constructor(activity: FragmentActivity) :
                 val pendingIntent: PendingIntent = result.getParcelableExtra(SshAuthenticationApi.EXTRA_PENDING_INTENT)!!
                 val resultOfUserInteraction: Intent = withContext(Dispatchers.Main) {
                     suspendCoroutine { cont ->
-                        currentCont = cont
-                        continueAfterUserInteraction.launch(IntentSenderRequest.Builder(pendingIntent).build())
+                        activity.stashedCont = cont
+                        activity.continueAfterUserInteraction.launch(IntentSenderRequest.Builder(pendingIntent).build())
                     }
                 }
                 executeApiRequest(request, resultOfUserInteraction)
@@ -196,7 +179,7 @@ class OpenKeychainKeyProvider private constructor(activity: FragmentActivity) :
     }
 
     override fun close() {
-        continueAfterUserInteraction.unregister()
+        activity.continueAfterUserInteraction.unregister()
         sshServiceConnection.disconnect()
     }
 
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/sshj/SshjSessionFactory.kt b/app/src/main/java/com/zeapo/pwdstore/git/sshj/SshjSessionFactory.kt
index f064df33..c36f27b3 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/sshj/SshjSessionFactory.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/sshj/SshjSessionFactory.kt
@@ -5,7 +5,6 @@
 package com.zeapo.pwdstore.git.sshj
 
 import android.util.Base64
-import androidx.fragment.app.FragmentActivity
 import com.github.ajalt.timberkt.d
 import com.github.ajalt.timberkt.w
 import com.github.michaelbull.result.getOrElse
@@ -40,10 +39,10 @@ import org.eclipse.jgit.transport.SshSessionFactory
 import org.eclipse.jgit.transport.URIish
 import org.eclipse.jgit.util.FS
 
-sealed class SshAuthMethod(val activity: FragmentActivity) {
-    class Password(activity: FragmentActivity) : SshAuthMethod(activity)
-    class SshKey(activity: FragmentActivity) : SshAuthMethod(activity)
-    class OpenKeychain(activity: FragmentActivity) : SshAuthMethod(activity)
+sealed class SshAuthMethod(val activity: ContinuationContainerActivity) {
+    class Password(activity: ContinuationContainerActivity) : SshAuthMethod(activity)
+    class SshKey(activity: ContinuationContainerActivity) : SshAuthMethod(activity)
+    class OpenKeychain(activity: ContinuationContainerActivity) : SshAuthMethod(activity)
 }
 
 abstract class InteractivePasswordFinder : PasswordFinder {