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> (cherry picked from commit 66b31f1432)
2020-10-23 15:23:47 +05:30 · 2020-10-23 15:23:47 +05:30 · 2b25171bf2
commit 2b25171bf2
parent 41fb76d0f2
12 changed files with 70 additions and 44 deletions
--- 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.
--- 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].
--- 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
--- 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),
--- 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 -> {
            }
--- 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
--- 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"),
--- 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
--- 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
--- a/app/src/main/java/com/zeapo/pwdstore/git/sshj/ContinuationContainerActivity.kt
+++ 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))
+        }
+    }
+}
--- 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()
    }

--- 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 {