Bugfixes and changes based on feedback

5 files changed, 39 insertions, 36 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java
index 152585040..e8e888c7a 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java
@@ -83,7 +83,7 @@ public class EditKeyOperation extends BaseOperation {
                     CanonicalizedSecretKeyRing secRing =
                             mProviderHelper.getCanonicalizedSecretKeyRing(saveParcel.mMasterKeyId);
 
-                    modifyResult = keyOperations.modifySecretKeyRing(secRing, cryptoInput, saveParcel, log);
+                    modifyResult = keyOperations.modifySecretKeyRing(secRing, cryptoInput, saveParcel, log, 2);
                     if (modifyResult.isPending()) {
                         return modifyResult;
                     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
index 065ca710c..f14589774 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
@@ -495,6 +495,9 @@ public abstract class OperationResult implements Parcelable {
         MSG_MF_ERROR_SIG (LogLevel.ERROR, R.string.msg_mf_error_sig),
         MSG_MF_ERROR_SUBKEY_MISSING(LogLevel.ERROR, R.string.msg_mf_error_subkey_missing),
         MSG_MF_ERROR_CONFLICTING_NFC_COMMANDS(LogLevel.ERROR, R.string.msg_mf_error_conflicting_nfc_commands),
+        MSG_MF_ERROR_BAD_NFC_ALGO(LogLevel.ERROR, R.string.edit_key_error_bad_nfc_algo),
+        MSG_MF_ERROR_BAD_NFC_SIZE(LogLevel.ERROR, R.string.edit_key_error_bad_nfc_size),
+        MSG_MF_ERROR_BAD_NFC_STRIPPED(LogLevel.ERROR, R.string.edit_key_error_bad_nfc_stripped),
         MSG_MF_MASTER (LogLevel.DEBUG, R.string.msg_mf_master),
         MSG_MF_NOTATION_PIN (LogLevel.DEBUG, R.string.msg_mf_notation_pin),
         MSG_MF_NOTATION_EMPTY (LogLevel.DEBUG, R.string.msg_mf_notation_empty),
@@ -739,11 +742,6 @@ public abstract class OperationResult implements Parcelable {
         MSG_EXPORT_LOG_EXPORT_ERROR_FOPEN(LogLevel.ERROR,R.string.msg_export_log_error_fopen),
         MSG_EXPORT_LOG_EXPORT_ERROR_WRITING(LogLevel.ERROR,R.string.msg_export_log_error_writing),
         MSG_EXPORT_LOG_EXPORT_SUCCESS (LogLevel.OK, R.string.msg_export_log_success),
-
-        // NFC keytocard
-        MSG_K2C_ERROR_BAD_ALGO(LogLevel.ERROR, R.string.edit_key_error_bad_nfc_algo),
-        MSG_K2C_ERROR_BAD_SIZE(LogLevel.ERROR, R.string.edit_key_error_bad_nfc_size),
-        MSG_K2C_ERROR_BAD_STRIPPED(LogLevel.ERROR, R.string.edit_key_error_bad_nfc_stripped),
         ;
 
         public final int mMsgId;
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 b6e978f48..566ffd44b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -329,7 +329,7 @@ public class PgpKeyOperation {
 
             subProgressPush(50, 100);
             CryptoInputParcel cryptoInput = new CryptoInputParcel(new Date(), new Passphrase(""));
-            return internal(sKR, masterSecretKey, add.mFlags, add.mExpiry, cryptoInput, saveParcel, log);
+            return internal(sKR, masterSecretKey, add.mFlags, add.mExpiry, cryptoInput, saveParcel, log, indent);
 
         } catch (PGPException e) {
             log.add(LogType.MSG_CR_ERROR_INTERNAL_PGP, indent);
@@ -363,15 +363,14 @@ public class PgpKeyOperation {
     public PgpEditKeyResult modifySecretKeyRing(CanonicalizedSecretKeyRing wsKR,
                                                 CryptoInputParcel cryptoInput,
                                                 SaveKeyringParcel saveParcel) {
-        return modifySecretKeyRing(wsKR, cryptoInput, saveParcel, new OperationLog());
+        return modifySecretKeyRing(wsKR, cryptoInput, saveParcel, new OperationLog(), 0);
     }
 
     public PgpEditKeyResult modifySecretKeyRing(CanonicalizedSecretKeyRing wsKR,
             CryptoInputParcel cryptoInput,
             SaveKeyringParcel saveParcel,
-            OperationLog log) {
-
-        int indent = 0;
+            OperationLog log,
+            int indent) {
 
         /*
          * 1. Unlock private key
@@ -429,7 +428,7 @@ public class PgpKeyOperation {
 
         if (isDummy(masterSecretKey) || saveParcel.isRestrictedOnly()) {
             log.add(LogType.MSG_MF_RESTRICTED_MODE, indent);
-            return internalRestricted(sKR, saveParcel, log);
+            return internalRestricted(sKR, saveParcel, log, indent + 1);
         }
 
         // Do we require a passphrase? If so, pass it along
@@ -447,7 +446,7 @@ public class PgpKeyOperation {
         Date expiryTime = wsKR.getPublicKey().getExpiryTime();
         long masterKeyExpiry = expiryTime != null ? expiryTime.getTime() / 1000 : 0L;
 
-        return internal(sKR, masterSecretKey, masterKeyFlags, masterKeyExpiry, cryptoInput, saveParcel, log);
+        return internal(sKR, masterSecretKey, masterKeyFlags, masterKeyExpiry, cryptoInput, saveParcel, log, indent);
 
     }
 
@@ -455,9 +454,8 @@ public class PgpKeyOperation {
                                      int masterKeyFlags, long masterKeyExpiry,
                                      CryptoInputParcel cryptoInput,
                                      SaveKeyringParcel saveParcel,
-                                     OperationLog log) {
-
-        int indent = 1;
+                                     OperationLog log,
+                                     int indent) {
 
         NfcSignOperationsBuilder nfcSignOps = new NfcSignOperationsBuilder(
                 cryptoInput.getSignatureTime(), masterSecretKey.getKeyID(),
@@ -1048,9 +1046,7 @@ public class PgpKeyOperation {
      * otherwise.
      */
     private PgpEditKeyResult internalRestricted(PGPSecretKeyRing sKR, SaveKeyringParcel saveParcel,
-                                                OperationLog log) {
-
-        int indent = 1;
+                                                OperationLog log, int indent) {
 
         progress(R.string.progress_modify, 0);
 
@@ -1543,20 +1539,20 @@ public class PgpKeyOperation {
         if (algorithm != PublicKeyAlgorithmTags.RSA_ENCRYPT &&
                 algorithm != PublicKeyAlgorithmTags.RSA_SIGN &&
                 algorithm != PublicKeyAlgorithmTags.RSA_GENERAL) {
-            log.add(OperationResult.LogType.MSG_K2C_ERROR_BAD_ALGO, indent + 1);
-            return true;
+            log.add(LogType.MSG_MF_ERROR_BAD_NFC_ALGO, indent + 1);
+            return false;
         }
 
         // Key size must be 2048
         int keySize = publicKey.getBitStrength();
         if (keySize != 2048) {
-            log.add(OperationResult.LogType.MSG_K2C_ERROR_BAD_SIZE, indent + 1);
+            log.add(LogType.MSG_MF_ERROR_BAD_NFC_SIZE, indent + 1);
             return false;
         }
 
         // Secret key parts must be available
         if (isDivertToCard(key) || isDummy(key)) {
-            log.add(OperationResult.LogType.MSG_K2C_ERROR_BAD_STRIPPED, indent + 1);
+            log.add(LogType.MSG_MF_ERROR_BAD_NFC_STRIPPED, indent + 1);
             return false;
         }
 
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 acc0c32b8..1f7a0eb0d 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;
@@ -43,7 +42,7 @@ import org.sufficientlysecure.keychain.compatibility.DialogFragmentWorkaround;
 import org.sufficientlysecure.keychain.operations.results.OperationResult;
 import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType;
 import org.sufficientlysecure.keychain.operations.results.SingletonResult;
-import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType;
 import org.sufficientlysecure.keychain.pgp.KeyRing;
 import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException;
 import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing;
@@ -420,9 +419,8 @@ public class EditKeyFragment extends CryptoOperationFragment implements
                         }
                         break;
                     case EditSubkeyDialogFragment.MESSAGE_STRIP: {
-                        CanonicalizedSecretKey.SecretKeyType secretKeyType =
-                                mSubkeysAdapter.getSecretKeyType(position);
-                        if (secretKeyType == CanonicalizedSecretKey.SecretKeyType.GNU_DUMMY) {
+                        SecretKeyType secretKeyType = mSubkeysAdapter.getSecretKeyType(position);
+                        if (secretKeyType == SecretKeyType.GNU_DUMMY) {
                             // Key is already stripped; this is a no-op.
                             break;
                         }
@@ -441,15 +439,27 @@ public class EditKeyFragment extends CryptoOperationFragment implements
                         break;
                     }
                     case EditSubkeyDialogFragment.MESSAGE_KEYTOCARD: {
-                        CanonicalizedSecretKey.SecretKeyType secretKeyType =
-                                mSubkeysAdapter.getSecretKeyType(position);
-                        if (secretKeyType == CanonicalizedSecretKey.SecretKeyType.DIVERT_TO_CARD ||
-                            secretKeyType == CanonicalizedSecretKey.SecretKeyType.GNU_DUMMY) {
-                            Toast.makeText(EditKeyFragment.this.getActivity(),
-                                    R.string.edit_key_error_bad_nfc_stripped, Toast.LENGTH_SHORT)
-                                    .show();
+                        Activity activity = EditKeyFragment.this.getActivity();
+                        SecretKeyType secretKeyType = mSubkeysAdapter.getSecretKeyType(position);
+                        if (secretKeyType == SecretKeyType.DIVERT_TO_CARD ||
+                            secretKeyType == SecretKeyType.GNU_DUMMY) {
+                            Notify.create(activity, R.string.edit_key_error_bad_nfc_stripped, Notify.Style.ERROR)
+                                    .show((ViewGroup) activity.findViewById(R.id.import_snackbar));
                             break;
                         }
+                        int algorithm = mSubkeysAdapter.getAlgorithm(position);
+                        // these are the PGP constants for RSA_GENERAL, RSA_ENCRYPT and RSA_SIGN
+                        if (algorithm != 1 && algorithm != 2 && algorithm != 3) {
+                            Notify.create(activity, R.string.edit_key_error_bad_nfc_algo, Notify.Style.ERROR)
+                                    .show((ViewGroup) activity.findViewById(R.id.import_snackbar));
+                            break;
+                        }
+                        if (mSubkeysAdapter.getKeySize(position) != 2048) {
+                            Notify.create(activity, R.string.edit_key_error_bad_nfc_size, Notify.Style.ERROR)
+                                    .show((ViewGroup) activity.findViewById(R.id.import_snackbar));
+                            break;
+                        }
+
 
                         SubkeyChange change;
                         change = mSaveKeyringParcel.getSubkeyChange(keyId);
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
index bed846dd3..3d7058ae5 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
@@ -104,7 +104,6 @@ public class NfcOperationActivity extends BaseNfcActivity {
                     throw new IOException("Couldn't find subkey for key to card operation.");
                 }
 
-                // Note: we're abusing mInputHashes to hold the subkey IDs we need to export.
                 for (int i = 0; i < mRequiredInput.mInputHashes.length; i++) {
                     byte[] subkeyBytes = mRequiredInput.mInputHashes[i];
                     ByteBuffer buf = ByteBuffer.wrap(subkeyBytes);