From cdf45bc323bd21eeb6252c9d9b70a990f7b39b75 Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Sun, 27 Oct 2019 00:14:42 +0530 Subject: [PATCH] Add setting to save OpenKeychain auth keyid (#554) * Add setting to save OpenKeychain auth keyid * Hide pref not disable Co-Authored-By: Reagan Sanders Signed-off-by: Harsh Shandilya --- .../java/com/zeapo/pwdstore/UserPreference.kt | 8 ++++ .../com/zeapo/pwdstore/git/CloneOperation.kt | 1 + .../com/zeapo/pwdstore/git/GitActivity.kt | 12 +++++- .../com/zeapo/pwdstore/git/GitOperation.kt | 13 +++--- .../com/zeapo/pwdstore/git/PullOperation.kt | 1 + .../com/zeapo/pwdstore/git/PushOperation.kt | 1 + .../pwdstore/git/ResetToRemoteOperation.kt | 1 + .../com/zeapo/pwdstore/git/SyncOperation.kt | 1 + .../git/config/SshApiSessionFactory.java | 40 ++++++++++++++++--- app/src/main/res/values/strings.xml | 1 + app/src/main/res/xml/preference.xml | 3 ++ 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("git_server_info") + val openkeystoreIdPreference = findPreference("ssh_openkeystore_clear_keyid") val gitConfigPreference = findPreference("git_config") val sshKeyPreference = findPreference("ssh_key") val sshKeygenPreference = findPreference("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 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 @@ Enable biometric authentication When enabled, Password Store will prompt you for your fingerprint when launching the app Fingerprint hardware not accessible or missing + Clear remembered OpenKeystore SSH Key ID 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 @@ -19,6 +19,9 @@ +