Add setting to save OpenKeychain auth keyid (#554)

* Add setting to save OpenKeychain auth keyid

* Hide pref not disable

Co-Authored-By: Reagan Sanders <vexofp@gmail.com>
Signed-off-by: Harsh Shandilya <msfjarvis@gmail.com>

11 files changed, 67 insertions, 15 deletions

diff --git a/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt b/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
index 61fab3fd..db303efc 100644
--- a/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
@@ -66,6 +66,7 @@ class UserPreference : AppCompatActivity() {
 
             // Git preferences
             val gitServerPreference = findPreference<Preference>("git_server_info")
+            val openkeystoreIdPreference = findPreference<Preference>("ssh_openkeystore_clear_keyid")
             val gitConfigPreference = findPreference<Preference>("git_config")
             val sshKeyPreference = findPreference<Preference>("ssh_key")
             val sshKeygenPreference = findPreference<Preference>("ssh_keygen")
@@ -114,6 +115,7 @@ class UserPreference : AppCompatActivity() {
                     OpenPgpUtils.convertKeyIdToHex(java.lang.Long.valueOf(s))
                 }
             }
+            openkeystoreIdPreference?.isVisible = sharedPreferences.getString("ssh_openkeystore_keyid", null)?.isNotEmpty() ?: false
 
             // see if the autofill service is enabled and check the preference accordingly
             autoFillEnablePreference?.isChecked = callingActivity.isServiceEnabled
@@ -156,6 +158,12 @@ class UserPreference : AppCompatActivity() {
                 true
             }
 
+            openkeystoreIdPreference?.onPreferenceClickListener = ClickListener {
+                sharedPreferences.edit().putString("ssh_openkeystore_keyid", null).apply()
+                it.isVisible = false
+                true
+            }
+
             gitServerPreference?.onPreferenceClickListener = ClickListener {
                 val intent = Intent(callingActivity, GitActivity::class.java)
                 intent.putExtra("Operation", GitActivity.EDIT_SERVER)
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/CloneOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/CloneOperation.kt
index a1e85584..cb805abe 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/CloneOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/CloneOperation.kt
@@ -64,6 +64,7 @@ class CloneOperation(fileDir: File, callingActivity: Activity) : GitOperation(fi
     }
 
     override fun onError(errorMessage: String) {
+        super.onError(errorMessage)
         MaterialAlertDialogBuilder(callingActivity)
                 .setTitle(callingActivity.resources.getString(R.string.jgit_error_dialog_title))
                 .setMessage("Error occured during the clone operation, " +
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/GitActivity.kt b/app/src/main/java/com/zeapo/pwdstore/git/GitActivity.kt
index cf9c3442..53ed3461 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/GitActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/GitActivity.kt
@@ -613,7 +613,7 @@ open class GitActivity : AppCompatActivity() {
         }
     }
 
-    override fun onActivityResult(
+    public override fun onActivityResult(
         requestCode: Int,
         resultCode: Int,
         data: Intent?
@@ -625,9 +625,17 @@ open class GitActivity : AppCompatActivity() {
         // background thread - the actual signing of the SSH challenge. We pass through the
         // completed signature to the ApiIdentity, which will be blocked in the other thread
         // waiting for it.
-        if (requestCode == SshApiSessionFactory.POST_SIGNATURE && identity != null)
+        if (requestCode == SshApiSessionFactory.POST_SIGNATURE && identity != null) {
             identity!!.postSignature(data)
 
+            // If the signature failed (usually because it was cancelled), reset state
+            if (data == null) {
+                identity = null
+                identityBuilder = null
+            }
+            return
+        }
+
         if (resultCode == AppCompatActivity.RESULT_CANCELED) {
             setResult(AppCompatActivity.RESULT_CANCELED)
             finish()
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 cbc64b3e..3b3c6f22 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/GitOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/GitOperation.kt
@@ -235,14 +235,11 @@ abstract class GitOperation(fileDir: File, internal val callingActivity: Activit
      * Action to execute on error
      */
     open fun onError(errorMessage: String) {
-        MaterialAlertDialogBuilder(callingActivity)
-                .setTitle(callingActivity.resources.getString(R.string.jgit_error_dialog_title))
-                .setMessage(callingActivity.resources.getString(R.string.jgit_error_dialog_text) + errorMessage)
-                .setPositiveButton(callingActivity.resources.getString(R.string.dialog_ok)) { _, _ ->
-                    callingActivity.setResult(Activity.RESULT_CANCELED)
-                    callingActivity.finish()
-                }
-                .show()
+        if (SshSessionFactory.getInstance() is SshApiSessionFactory) {
+            // Clear stored key id from settings on auth failure
+            PreferenceManager.getDefaultSharedPreferences(callingActivity.applicationContext)
+                    .edit().putString("ssh_openkeystore_keyid", null).apply()
+        }
     }
 
     /**
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/PullOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/PullOperation.kt
index 6c41093e..aaf53b99 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/PullOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/PullOperation.kt
@@ -38,6 +38,7 @@ class PullOperation(fileDir: File, callingActivity: Activity) : GitOperation(fil
     }
 
     override fun onError(errorMessage: String) {
+        super.onError(errorMessage)
         MaterialAlertDialogBuilder(callingActivity)
                 .setTitle(callingActivity.resources.getString(R.string.jgit_error_dialog_title))
                 .setMessage("Error occured during the pull operation, " +
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/PushOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/PushOperation.kt
index 68baf904..0b3782b5 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/PushOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/PushOperation.kt
@@ -39,6 +39,7 @@ class PushOperation(fileDir: File, callingActivity: Activity) : GitOperation(fil
 
     override fun onError(errorMessage: String) {
         // TODO handle the "Nothing to push" case
+        super.onError(errorMessage)
         MaterialAlertDialogBuilder(callingActivity)
                 .setTitle(callingActivity.resources.getString(R.string.jgit_error_dialog_title))
                 .setMessage(callingActivity.getString(R.string.jgit_error_push_dialog_text) + errorMessage)
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/ResetToRemoteOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/ResetToRemoteOperation.kt
index e3eb4b44..53238bb1 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/ResetToRemoteOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/ResetToRemoteOperation.kt
@@ -44,6 +44,7 @@ class ResetToRemoteOperation(fileDir: File, callingActivity: Activity) : GitOper
     }
 
     override fun onError(errorMessage: String) {
+        super.onError(errorMessage)
         MaterialAlertDialogBuilder(callingActivity)
                 .setTitle(callingActivity.resources.getString(R.string.jgit_error_dialog_title))
                 .setMessage("Error occured during the sync operation, " +
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/SyncOperation.kt b/app/src/main/java/com/zeapo/pwdstore/git/SyncOperation.kt
index ba5d6c55..5518be19 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/SyncOperation.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/SyncOperation.kt
@@ -52,6 +52,7 @@ class SyncOperation(fileDir: File, callingActivity: Activity) : GitOperation(fil
     }
 
     override fun onError(errorMessage: String) {
+        super.onError(errorMessage)
         MaterialAlertDialogBuilder(callingActivity)
                 .setTitle(callingActivity.resources.getString(R.string.jgit_error_dialog_title))
                 .setMessage("Error occured during the sync operation, " +
diff --git a/app/src/main/java/com/zeapo/pwdstore/git/config/SshApiSessionFactory.java b/app/src/main/java/com/zeapo/pwdstore/git/config/SshApiSessionFactory.java
index ba807979..39a31991 100644
--- a/app/src/main/java/com/zeapo/pwdstore/git/config/SshApiSessionFactory.java
+++ b/app/src/main/java/com/zeapo/pwdstore/git/config/SshApiSessionFactory.java
@@ -8,7 +8,9 @@ import android.app.Activity;
 import android.app.PendingIntent;
 import android.content.Intent;
 import android.content.IntentSender;
+import android.content.SharedPreferences;
 import androidx.annotation.NonNull;
+import androidx.preference.PreferenceManager;
 import com.google.android.material.dialog.MaterialAlertDialogBuilder;
 import com.jcraft.jsch.Identity;
 import com.jcraft.jsch.JSch;
@@ -16,6 +18,7 @@ import com.jcraft.jsch.JSchException;
 import com.jcraft.jsch.Session;
 import com.jcraft.jsch.UserInfo;
 import com.zeapo.pwdstore.R;
+import com.zeapo.pwdstore.git.GitActivity;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import org.eclipse.jgit.errors.UnsupportedCredentialItem;
@@ -104,7 +107,8 @@ public class SshApiSessionFactory extends GitConfigSessionFactory {
         private SshAuthenticationApi api;
         private String keyId, description, alg;
         private byte[] publicKey;
-        private Activity callingActivity;
+        private GitActivity callingActivity;
+        private SharedPreferences settings;
 
         /**
          * Construct a new IdentityBuilder
@@ -112,7 +116,7 @@ public class SshApiSessionFactory extends GitConfigSessionFactory {
          * @param callingActivity Activity that will be used to launch pending intents and that will
          *     receive and handle the results.
          */
-        public IdentityBuilder(Activity callingActivity) {
+        public IdentityBuilder(GitActivity callingActivity) {
             this.callingActivity = callingActivity;
 
             List<String> providers =
@@ -124,6 +128,11 @@ public class SshApiSessionFactory extends GitConfigSessionFactory {
             // TODO: Handle multiple available providers? Are there actually any in practice beyond
             // OpenKeychain?
             connection = new SshAuthenticationConnection(callingActivity, providers.get(0));
+
+            settings =
+                    PreferenceManager.getDefaultSharedPreferences(
+                            callingActivity.getApplicationContext());
+            keyId = settings.getString("ssh_openkeystore_keyid", null);
         }
 
         /** Free any resources associated with this IdentityBuilder */
@@ -146,7 +155,24 @@ public class SshApiSessionFactory extends GitConfigSessionFactory {
                 case SshAuthenticationApi.RESULT_CODE_ERROR:
                     SshAuthenticationApiError error =
                             result.getParcelableExtra(SshAuthenticationApi.EXTRA_ERROR);
-                    throw new RuntimeException(error.getMessage());
+                    // On an OpenKeychain SSH API error, clear out the stored keyid
+                    settings.edit().putString("ssh_openkeystore_keyid", null).apply();
+
+                    switch (error.getError()) {
+                            // If the problem was just a bad keyid, reset to allow them to choose a
+                            // different one
+                        case (SshAuthenticationApiError.NO_SUCH_KEY):
+                        case (SshAuthenticationApiError.NO_AUTH_KEY):
+                            keyId = null;
+                            publicKey = null;
+                            description = null;
+                            alg = null;
+                            return executeApi(new KeySelectionRequest(), requestCode);
+
+                            // Other errors are fatal
+                        default:
+                            throw new RuntimeException(error.getMessage());
+                    }
                 case SshAuthenticationApi.RESULT_CODE_SUCCESS:
                     break;
                 case SshAuthenticationApi.RESULT_CODE_USER_INTERACTION_REQUIRED:
@@ -181,6 +207,7 @@ public class SshApiSessionFactory extends GitConfigSessionFactory {
             if (intent.hasExtra(SshAuthenticationApi.EXTRA_KEY_ID)) {
                 keyId = intent.getStringExtra(SshAuthenticationApi.EXTRA_KEY_ID);
                 description = intent.getStringExtra(SshAuthenticationApi.EXTRA_KEY_DESCRIPTION);
+                settings.edit().putString("ssh_openkeystore_keyid", keyId).apply();
             }
 
             if (intent.hasExtra(SshAuthenticationApi.EXTRA_SSH_PUBLIC_KEY)) {
@@ -209,7 +236,8 @@ public class SshApiSessionFactory extends GitConfigSessionFactory {
                                 // We can immediately try the next phase without needing to post
                                 // back
                                 // though onActivityResult
-                                tryBuild(requestCode);
+                                callingActivity.onActivityResult(
+                                        requestCode, Activity.RESULT_OK, null);
                             }
 
                             @Override
@@ -342,7 +370,9 @@ public class SshApiSessionFactory extends GitConfigSessionFactory {
          */
         public void postSignature(Intent data) {
             try {
-                signature = handleSignResult(data);
+                if (data != null) {
+                    signature = handleSignResult(data);
+                }
             } finally {
                 if (latch != null) latch.countDown();
             }
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index ebb4b413..62e478e5 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -271,4 +271,5 @@
     <string name="biometric_auth_title">Enable biometric authentication</string>
     <string name="biometric_auth_summary">When enabled, Password Store will prompt you for your fingerprint when launching the app</string>
     <string name="biometric_auth_summary_error">Fingerprint hardware not accessible or missing</string>
+    <string name="ssh_openkeystore_clear_keyid">Clear remembered OpenKeystore SSH Key ID</string>
 </resources>
diff --git a/app/src/main/res/xml/preference.xml b/app/src/main/res/xml/preference.xml
index 9124d218..b6d720a5 100644
--- a/app/src/main/res/xml/preference.xml
+++ b/app/src/main/res/xml/preference.xml
@@ -20,6 +20,9 @@
             android:key="hotp_remember_clear_choice"
             android:title="@string/hotp_remember_clear_choice" />
         <androidx.preference.Preference
+            android:key="ssh_openkeystore_clear_keyid"
+            android:title="@string/ssh_openkeystore_clear_keyid" />
+        <androidx.preference.Preference
             android:key="ssh_see_key"
             android:title="@string/pref_ssh_see_key_title" />
         <androidx.preference.Preference