Properly handle uneditable key types in EditKeyFragment

More specifically, return an OperationResultParcel for stripped and
divert-to-card keys, with an appropriate error message in the log.

part of #828

4 files changed, 54 insertions, 9 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
index ddb00305a..3f5528b05 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -381,6 +381,10 @@ public class PgpKeyOperation {
      * This is a natural workflow since pgp keyrings are immutable data structures: Old semantics
      * are changed by adding new certificates, which implicitly override older certificates.
      *
+     * Note that this method does not care about any "special" type of master key. If unlocking
+     * with a passphrase fails, the operation will fail with an unlocking error. More specific
+     * handling of errors should be done in UI code!
+     *
      */
     public EditKeyResult modifySecretKeyRing(CanonicalizedSecretKeyRing wsKR, SaveKeyringParcel saveParcel,
                                                String passphrase) {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
index 438443644..fefc3e4b1 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -427,6 +427,12 @@ public class OperationResultParcel implements Parcelable {
         MSG_CON_SUCCESS (R.string.msg_con_success),
         MSG_CON_WARN_DELETE_PUBLIC (R.string.msg_con_warn_delete_public),
         MSG_CON_WARN_DELETE_SECRET (R.string.msg_con_warn_delete_secret),
+
+        // messages used in UI code
+        MSG_EK_ERROR_DIVERT (R.string.msg_ek_error_divert),
+        MSG_EK_ERROR_DUMMY (R.string.msg_ek_error_dummy),
+        MSG_EK_ERROR_NOT_FOUND (R.string.msg_ek_error_not_found),
+
         ;
 
         private final int mMsgId;
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
index 830e856aa..3de01adcd 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
@@ -35,7 +35,6 @@ import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.widget.AdapterView;
 import android.widget.ListView;
-import android.widget.Toast;
 
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
@@ -46,9 +45,13 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException;
 import org.sufficientlysecure.keychain.service.KeychainIntentService;
 import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler;
 import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter;
@@ -169,15 +172,26 @@ public class EditKeyFragment extends LoaderFragment implements
             Uri secretUri = KeychainContract.KeyRings.buildUnifiedKeyRingUri(mDataUri);
             CachedPublicKeyRing keyRing =
                     new ProviderHelper(getActivity()).getCachedPublicKeyRing(secretUri);
+            long masterKeyId = keyRing.getMasterKeyId();
+
+            // check if this is a master secret key we can work with
+            switch (keyRing.getSecretKeyType(masterKeyId)) {
+                case GNU_DUMMY:
+                    finishWithError(LogType.MSG_EK_ERROR_DUMMY);
+                    return;
+                case DIVERT_TO_CARD:
+                    finishWithError(LogType.MSG_EK_ERROR_DIVERT);
+                    break;
+            }
 
-            mSaveKeyringParcel = new SaveKeyringParcel(keyRing.getMasterKeyId(),
-                    keyRing.getFingerprint());
+            mSaveKeyringParcel = new SaveKeyringParcel(masterKeyId, keyRing.getFingerprint());
             mPrimaryUserId = keyRing.getPrimaryUserIdWithFallback();
 
+        } catch (NotFoundException e) {
+            finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND);
+            return;
         } catch (PgpGeneralException e) {
-            Log.e(Constants.TAG, "PgpGeneralException", e);
-            Toast.makeText(getActivity(), R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show();
-            getActivity().finish();
+            finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND);
             return;
         }
 
@@ -185,9 +199,7 @@ public class EditKeyFragment extends LoaderFragment implements
             mCurrentPassphrase = PassphraseCacheService.getCachedPassphrase(getActivity(),
                     mSaveKeyringParcel.mMasterKeyId);
         } catch (PassphraseCacheService.KeyNotFoundException e) {
-            Log.e(Constants.TAG, "Key not found!", e);
-            Toast.makeText(getActivity(), R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show();
-            getActivity().finish();
+            finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND);
             return;
         }
 
@@ -564,4 +576,22 @@ public class EditKeyFragment extends LoaderFragment implements
         getActivity().startService(intent);
 
     }
+
+    /** Closes this activity, returning a result parcel with a single error log entry. */
+    void finishWithError(LogType reason) {
+
+        // Prepare the log
+        OperationLog log = new OperationLog();
+        log.add(LogLevel.ERROR, reason, 0);
+
+        // Prepare an intent with an EXTRA_RESULT
+        Intent intent = new Intent();
+        intent.putExtra(OperationResultParcel.EXTRA_RESULT,
+                new OperationResultParcel(OperationResultParcel.RESULT_ERROR, log));
+
+        // Finish with result
+        getActivity().setResult(EditKeyActivity.RESULT_OK, intent);
+        getActivity().finish();
+    }
+
 }
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 24effb7f1..2a89f15bc 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -756,6 +756,11 @@
     <string name="msg_con_warn_delete_public">"Exception deleting public cache file"</string>
     <string name="msg_con_warn_delete_secret">"Exception deleting secret cache file"</string>
 
+    <!-- Other messages used in OperationLogs -->
+    <string name="msg_ek_error_divert">"editing of nfc keys is not (yet) supported"</string>
+    <string name="msg_ek_error_dummy">"cannot edit keyring with stripped master key!"</string>
+    <string name="msg_ek_error_not_found">"key not found"</string>
+
     <!-- PassphraseCache -->
     <string name="passp_cache_notif_click_to_clear">"Click to clear cached passphrases"</string>
     <string name="passp_cache_notif_n_keys">"OpenKeychain has cached %d passphrases"</string>