Refactored code to use functions already present in code, reduced liberties taken when modifying functions.

Todo:
Fix indentation for error messages

2 files changed, 29 insertions, 28 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/PassphraseChangeOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/PassphraseChangeOperation.java
index e95f35c21..fff4ef534 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/PassphraseChangeOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/PassphraseChangeOperation.java
@@ -63,15 +63,6 @@ public class PassphraseChangeOperation extends BaseOperation<PassphraseChangePar
 
                     CanonicalizedSecretKeyRing secRing =
                             mProviderHelper.getCanonicalizedSecretKeyRing(passphraseParcel.mMasterKeyId);
-                    CachedPublicKeyRing cachedRing =
-                            mProviderHelper.getCachedPublicKeyRing(passphraseParcel.mMasterKeyId);
-
-                    passphraseParcel.mValidSubkeyId = getFirstValidKeyId(secRing, cachedRing);
-
-                    if(passphraseParcel.mValidSubkeyId == null) {
-                        log.add(OperationResult.LogType.MSG_MF_ERROR_ALL_KEYS_STRIPPED, 0);
-                        return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-                    }
 
                     modifyResult = keyOperations.modifyKeyRingPassword(secRing, cryptoInput, passphraseParcel);
 
@@ -119,23 +110,4 @@ public class PassphraseChangeOperation extends BaseOperation<PassphraseChangePar
 
     }
 
-    private static Long getFirstValidKeyId (CanonicalizedSecretKeyRing secRing, CachedPublicKeyRing cachedRing) {
-
-        Iterator<CanonicalizedSecretKey> secretKeyIterator = secRing.secretKeyIterator().iterator();
-
-        while(secretKeyIterator.hasNext()) {
-            try {
-                long keyId = secretKeyIterator.next().getKeyId();
-                CanonicalizedSecretKey.SecretKeyType keyType = cachedRing.getSecretKeyType(keyId);
-                if( keyType == CanonicalizedSecretKey.SecretKeyType.PASSPHRASE
-                        || keyType == CanonicalizedSecretKey.SecretKeyType.PASSPHRASE_EMPTY) {
-                    return keyId;
-                }
-            } catch (ProviderHelper.NotFoundException e) {
-                ;
-            }
-        }
-
-        return null;
-    }
 }
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 abfdf0966..cd4d9e5bb 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -72,6 +72,7 @@ import org.sufficientlysecure.keychain.operations.results.OperationResult;
 import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType;
 import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
 import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
+import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.service.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.service.PassphraseChangeParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
@@ -376,6 +377,16 @@ public class PgpKeyOperation {
             return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
         }
 
+        if (passphraseParcel.mValidSubkeyId == null) {
+            PGPSecretKey nonDummy = firstNonDummySecretKeyID(sKR);
+            if(nonDummy== null) {
+                log.add(OperationResult.LogType.MSG_MF_ERROR_ALL_KEYS_STRIPPED, 0);
+                return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
+            } else {
+                passphraseParcel.mValidSubkeyId = nonDummy.getKeyID();
+            }
+        }
+
         if (!cryptoInput.hasPassphrase()) {
             log.add(LogType.MSG_MF_REQUIRE_PASSPHRASE, indent);
 
@@ -405,6 +416,18 @@ public class PgpKeyOperation {
         }
     }
 
+    private static PGPSecretKey firstNonDummySecretKeyID(PGPSecretKeyRing secRing) {
+        Iterator<PGPSecretKey> secretKeyIterator = secRing.getSecretKeys();
+
+        while(secretKeyIterator.hasNext()) {
+            PGPSecretKey secretKey = secretKeyIterator.next();
+            if(!isDummy(secretKey)){
+                return secretKey;
+            }
+        }
+        return null;
+    }
+
     /** This method introduces a list of modifications specified by a SaveKeyringParcel to a
      * WrappedSecretKeyRing.
      *
@@ -1297,6 +1320,12 @@ public class PgpKeyOperation {
                 ok = true;
             } catch (PGPException e) {
 
+                // if this is the master key, error!
+                if (sKey.getKeyID() == masterPublicKey.getKeyID() && !isDummy(sKey)) {
+                    log.add(LogType.MSG_MF_ERROR_PASSPHRASE_MASTER, indent+1);
+                    return null;
+                }
+
                 // being in here means decrypt failed, likely due to a bad passphrase try
                 // again with an empty passphrase, maybe we can salvage this
                 try {