try cache in certify operation (fixes #1242)

5 files changed, 61 insertions, 74 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java
index 318eee6ba..d0377cc24 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java
@@ -29,6 +29,7 @@ import org.sufficientlysecure.keychain.operations.results.SaveKeyringResult;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
+import org.sufficientlysecure.keychain.pgp.PassphraseCacheInterface;
 import org.sufficientlysecure.keychain.pgp.PgpCertifyOperation;
 import org.sufficientlysecure.keychain.pgp.PgpCertifyOperation.PgpCertifyResult;
 import org.sufficientlysecure.keychain.pgp.Progressable;
@@ -90,14 +91,21 @@ public class CertifyOperation extends BaseOperation<CertifyActionsParcel> {
                 case PIN:
                 case PATTERN:
                 case PASSPHRASE:
-                    if (!cryptoInput.hasPassphrase()) {
+                    passphrase = cryptoInput.getPassphrase();
+                    if (passphrase == null) {
+                        try {
+                            passphrase = getCachedPassphrase(certificationKey.getKeyId(), certificationKey.getKeyId());
+                        } catch (PassphraseCacheInterface.NoSecretKeyException ignored) {
+                            // treat as a cache miss for error handling purposes
+                        }
+                    }
+
+                    if (passphrase == null) {
                         return new CertifyResult(log,
                                 RequiredInputParcel.createRequiredSignPassphrase(
                                 certificationKey.getKeyId(), certificationKey.getKeyId(), null)
                         );
                     }
-                    // certification is always with the master key id, so use that one
-                    passphrase = cryptoInput.getPassphrase();
                     break;
 
                 case PASSPHRASE_EMPTY:
@@ -105,6 +113,7 @@ public class CertifyOperation extends BaseOperation<CertifyActionsParcel> {
                     break;
 
                 case DIVERT_TO_CARD:
+                    // the unlock operation will succeed for passphrase == null in a divertToCard key
                     passphrase = null;
                     break;
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java
index ab631c6fe..9d6e7d096 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java
@@ -331,9 +331,4 @@ public class CertifyKeyFragment
         getActivity().finish();
     }
 
-    @Override
-    public void onCryptoOperationCancelled() {
-        super.onCryptoOperationCancelled();
-    }
-
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java
index da3aae123..a50696cf3 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java
@@ -31,7 +31,6 @@ import android.os.AsyncTask;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.Handler;
-import android.os.Handler.Callback;
 import android.os.Message;
 import android.os.Messenger;
 import android.provider.ContactsContract;
@@ -57,7 +56,6 @@ import com.getbase.floatingactionbutton.FloatingActionButton;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing;
-import org.sufficientlysecure.keychain.operations.results.CertifyResult;
 import org.sufficientlysecure.keychain.operations.results.ImportKeyResult;
 import org.sufficientlysecure.keychain.operations.results.OperationResult;
 import org.sufficientlysecure.keychain.pgp.KeyRing;
@@ -67,10 +65,7 @@ import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.service.ImportKeyringParcel;
-import org.sufficientlysecure.keychain.service.KeychainService;
-import org.sufficientlysecure.keychain.service.ServiceProgressHandler;
 import org.sufficientlysecure.keychain.service.ServiceProgressHandler.MessageStatus;
-import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.ui.base.BaseNfcActivity;
 import org.sufficientlysecure.keychain.ui.base.CryptoOperationHelper;
 import org.sufficientlysecure.keychain.ui.dialog.DeleteKeyDialogFragment;
@@ -102,6 +97,8 @@ public class ViewKeyActivity extends BaseNfcActivity implements
     static final int REQUEST_QR_FINGERPRINT = 1;
     static final int REQUEST_DELETE = 2;
     static final int REQUEST_EXPORT = 3;
+    static final int REQUEST_CERTIFY = 4;
+
     public static final String EXTRA_DISPLAY_RESULT = "display_result";
 
     ProviderHelper mProviderHelper;
@@ -158,6 +155,7 @@ public class ViewKeyActivity extends BaseNfcActivity implements
         super.onCreate(savedInstanceState);
 
         mProviderHelper = new ProviderHelper(this);
+        mOperationHelper = new CryptoOperationHelper<>(this, this, null);
 
         setTitle(null);
 
@@ -382,37 +380,14 @@ public class ViewKeyActivity extends BaseNfcActivity implements
         Intent intent = new Intent(this, CertifyFingerprintActivity.class);
         intent.setData(dataUri);
 
-        startCertifyIntent(intent);
+        startActivityForResult(intent, REQUEST_CERTIFY);
     }
 
     private void certifyImmediate() {
         Intent intent = new Intent(this, CertifyKeyActivity.class);
         intent.putExtra(CertifyKeyActivity.EXTRA_KEY_IDS, new long[]{mMasterKeyId});
 
-        startCertifyIntent(intent);
-    }
-
-    private void startCertifyIntent(Intent intent) {
-        // Message is received after signing is done in KeychainService
-        ServiceProgressHandler saveHandler = new ServiceProgressHandler(this) {
-            @Override
-            public void handleMessage(Message message) {
-                // handle messages by standard KeychainIntentServiceHandler first
-                super.handleMessage(message);
-
-                if (message.arg1 == MessageStatus.OKAY.ordinal()) {
-                    Bundle data = message.getData();
-                    CertifyResult result = data.getParcelable(CertifyResult.EXTRA_RESULT);
-
-                    result.createNotify(ViewKeyActivity.this).show();
-                }
-            }
-        };
-        // Create a new Messenger for the communication back
-        Messenger messenger = new Messenger(saveHandler);
-        intent.putExtra(KeychainService.EXTRA_MESSENGER, messenger);
-
-        startActivityForResult(intent, 0);
+        startActivityForResult(intent, REQUEST_CERTIFY);
     }
 
     private void showQrCodeDialog() {
@@ -482,49 +457,58 @@ public class ViewKeyActivity extends BaseNfcActivity implements
 
     @Override
     protected void onActivityResult(int requestCode, int resultCode, Intent data) {
-        if (mOperationHelper != null) {
-            mOperationHelper.handleActivityResult(requestCode, resultCode, data);
+        if (mOperationHelper.handleActivityResult(requestCode, resultCode, data)) {
+            return;
+        }
+
+        if (resultCode != Activity.RESULT_OK) {
+            return;
         }
 
-        if (requestCode == REQUEST_QR_FINGERPRINT && resultCode == Activity.RESULT_OK) {
+        switch (requestCode) {
+            case REQUEST_QR_FINGERPRINT: {
+
+                // If there is an EXTRA_RESULT, that's an error. Just show it.
+                if (data.hasExtra(OperationResult.EXTRA_RESULT)) {
+                    OperationResult result = data.getParcelableExtra(OperationResult.EXTRA_RESULT);
+                    result.createNotify(this).show();
+                    return;
+                }
 
-            // If there is an EXTRA_RESULT, that's an error. Just show it.
-            if (data.hasExtra(OperationResult.EXTRA_RESULT)) {
-                OperationResult result = data.getParcelableExtra(OperationResult.EXTRA_RESULT);
-                result.createNotify(this).show();
+                String fp = data.getStringExtra(ImportKeysProxyActivity.EXTRA_FINGERPRINT);
+                if (fp == null) {
+                    Notify.create(this, R.string.error_scan_fp, Notify.LENGTH_LONG, Style.ERROR).show();
+                    return;
+                }
+                if (mFingerprint.equalsIgnoreCase(fp)) {
+                    certifyImmediate();
+                } else {
+                    Notify.create(this, R.string.error_scan_match, Notify.LENGTH_LONG, Style.ERROR).show();
+                }
                 return;
             }
 
-            String fp = data.getStringExtra(ImportKeysProxyActivity.EXTRA_FINGERPRINT);
-            if (fp == null) {
-                Notify.create(this, "Error scanning fingerprint!",
-                        Notify.LENGTH_LONG, Notify.Style.ERROR).show();
+            case REQUEST_DELETE: {
+                deleteKey();
                 return;
             }
-            if (mFingerprint.equalsIgnoreCase(fp)) {
-                certifyImmediate();
-            } else {
-                Notify.create(this, "Fingerprints did not match!",
-                        Notify.LENGTH_LONG, Notify.Style.ERROR).show();
-            }
 
-            return;
-        }
+            case REQUEST_EXPORT: {
+                exportToFile(mDataUri, mProviderHelper);
+                return;
+            }
 
-        if (requestCode == REQUEST_DELETE && resultCode == Activity.RESULT_OK) {
-            deleteKey();
+            case REQUEST_CERTIFY: {
+                if (data.hasExtra(OperationResult.EXTRA_RESULT)) {
+                    OperationResult result = data.getParcelableExtra(OperationResult.EXTRA_RESULT);
+                    result.createNotify(this).show();
+                }
+                return;
+            }
         }
 
-        if (requestCode == REQUEST_EXPORT && resultCode == Activity.RESULT_OK) {
-            exportToFile(mDataUri, mProviderHelper);
-        }
+        super.onActivityResult(requestCode, resultCode, data);
 
-        if (data != null && data.hasExtra(OperationResult.EXTRA_RESULT)) {
-            OperationResult result = data.getParcelableExtra(OperationResult.EXTRA_RESULT);
-            result.createNotify(this).show();
-        } else {
-            super.onActivityResult(requestCode, resultCode, data);
-        }
     }
 
     @Override
@@ -863,6 +847,7 @@ public class ViewKeyActivity extends BaseNfcActivity implements
                             mActionNfc.setVisibility(View.GONE);
                         }
                         mFab.setVisibility(View.VISIBLE);
+                        // noinspection deprecation (no getDrawable with theme at current minApi level 15!)
                         mFab.setIconDrawable(getResources().getDrawable(R.drawable.ic_repeat_white_24dp));
                     } else {
                         mActionEncryptFile.setVisibility(View.VISIBLE);
@@ -952,7 +937,6 @@ public class ViewKeyActivity extends BaseNfcActivity implements
             mKeyserver = cloudPrefs.keyserver;
         }
 
-        mOperationHelper = new CryptoOperationHelper<>(this, this, null);
         mOperationHelper.cryptoOperation();
     }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java
index 0db46a585..f72e87ca7 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/CryptoOperationHelper.java
@@ -208,12 +208,9 @@ public class CryptoOperationHelper<T extends Parcelable, S extends OperationResu
                     return true;
                 }
             }
-
-            default: {
-                return false;
-            }
         }
-        return true;
+
+        return false;
     }
 
     protected void dismissProgress() {
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 2e6552b0f..cae2faef2 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -1432,5 +1432,7 @@
     <string name="file_delete_exception">"Original file could not be deleted!"</string>
     <string name="error_clipboard_empty">"Clipboard is empty!"</string>
     <string name="error_clipboard_copy">"Error copying data to clipboard!"</string>
+    <string name="error_scan_fp">Error scanning fingerprint!</string>
+    <string name="error_scan_match">Fingerprints did not match!</string>
 
 </resources>