fix unit tests (for real)

7 files changed, 39 insertions, 20 deletions

diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
index 2ecc304e7..e6ada202b 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
@@ -75,7 +75,6 @@ public class PgpKeyOperationTest {
 
     static UncachedKeyRing staticRing;
     final static Passphrase passphrase = TestingUtils.genPassphrase();
-    final static CryptoInputParcel cryptoInput = new CryptoInputParcel(new Date(), passphrase);
 
     UncachedKeyRing ring;
     PgpKeyOperation op;
@@ -83,6 +82,8 @@ public class PgpKeyOperationTest {
     ArrayList<RawPacket> onlyA = new ArrayList<RawPacket>();
     ArrayList<RawPacket> onlyB = new ArrayList<RawPacket>();
 
+    static CryptoInputParcel cryptoInput;
+
     @BeforeClass
     public static void setUpOnce() throws Exception {
         Security.insertProviderAt(new BouncyCastleProvider(), 1);
@@ -110,6 +111,9 @@ public class PgpKeyOperationTest {
 
         // we sleep here for a second, to make sure all new certificates have different timestamps
         Thread.sleep(1000);
+
+        cryptoInput = new CryptoInputParcel(new Date(), passphrase);
+
     }
 
     @Before public void setUp() throws Exception {
@@ -302,6 +306,7 @@ public class PgpKeyOperationTest {
             if (badphrase.equals(passphrase)) {
                 badphrase = new Passphrase("a");
             }
+            parcel.mAddUserIds.add("allure");
 
             assertModifyFailure("keyring modification with bad passphrase should fail",
                     ring, parcel, new CryptoInputParcel(badphrase), LogType.MSG_MF_UNLOCK_ERROR);
@@ -659,7 +664,8 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mRevokeSubKeys.add(keyId);
 
-            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
+            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB,
+                    new CryptoInputParcel(new Date(), passphrase));
 
             Assert.assertEquals("no extra packets in original", 0, onlyA.size());
             Assert.assertEquals("exactly one extra packet in modified", 1, onlyB.size());
@@ -779,7 +785,7 @@ public class PgpKeyOperationTest {
         { // we should be able to change the stripped/divert status of subkeys without passphrase
             parcel.reset();
             parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null));
-            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, null);
+            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, new CryptoInputParcel());
             Assert.assertEquals("one extra packet in modified", 1, onlyB.size());
             Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket();
             Assert.assertEquals("new packet should have GNU_DUMMY S2K type",
@@ -795,7 +801,7 @@ public class PgpKeyOperationTest {
                     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
             };
             parcel.mChangeSubKeys.add(new SubkeyChange(keyId, false, serial));
-            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, null);
+            modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, new CryptoInputParcel());
             Assert.assertEquals("one extra packet in modified", 1, onlyB.size());
             Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket();
             Assert.assertEquals("new packet should have GNU_DUMMY S2K type",
@@ -972,12 +978,12 @@ public class PgpKeyOperationTest {
         Assert.assertEquals("signature type must be positive certification",
                 PGPSignature.POSITIVE_CERTIFICATION, ((SignaturePacket) p).getSignatureType());
 
-        // make sure packets can be distinguished by timestamp
         Thread.sleep(1000);
 
         // applying the same modification AGAIN should not add more certifications but drop those
         // as duplicates
-        modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, cryptoInput, true, false);
+        modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB,
+                new CryptoInputParcel(new Date(), passphrase), true, false);
 
         Assert.assertEquals("duplicate modification: one extra packet in original", 1, onlyA.size());
         Assert.assertEquals("duplicate modification: one extra packet in modified", 1, onlyB.size());
@@ -1055,7 +1061,8 @@ public class PgpKeyOperationTest {
         Passphrase otherPassphrase = TestingUtils.genPassphrase(true);
         CryptoInputParcel otherCryptoInput = new CryptoInputParcel(otherPassphrase);
         parcel.mNewUnlock = new ChangeUnlockParcel(otherPassphrase);
-        modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, new CryptoInputParcel(new Date()));
+        modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB,
+                new CryptoInputParcel(new Date(), new Passphrase()));
 
         Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
                 3, onlyB.size());
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
index 732c13f62..755a0b00d 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
@@ -190,11 +190,11 @@ public class UncachedKeyringMergeTest {
 
             parcel.reset();
             parcel.mAddUserIds.add("flim");
-            modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing();
+            modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
 
             parcel.reset();
             parcel.mAddUserIds.add("flam");
-            modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing();
+            modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
         }
 
         { // merge A into base
@@ -231,8 +231,8 @@ public class UncachedKeyringMergeTest {
             parcel.reset();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
-            modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing();
-            modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing();
+            modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
+            modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
 
             subKeyIdA = KeyringTestingHelper.getSubkeyId(modifiedA, 2);
             subKeyIdB = KeyringTestingHelper.getSubkeyId(modifiedB, 2);
@@ -273,7 +273,7 @@ public class UncachedKeyringMergeTest {
             parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(ringA, 1));
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
                     ringA.getEncoded(), false, 0);
-            modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing();
+            modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
         }
 
         {
@@ -372,7 +372,7 @@ public class UncachedKeyringMergeTest {
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
                     ringA.getEncoded(), false, 0);
-            modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing();
+            modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
         }
 
         {
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 25abab25b..4646aef8a 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
@@ -530,7 +530,6 @@ public abstract class OperationResult implements Parcelable {
         MSG_MF_ERROR_REVOKED_PRIMARY (LogLevel.ERROR, R.string.msg_mf_error_revoked_primary),
         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_INPUT_REQUIRED (LogLevel.OK, R.string.msg_mf_input_required),
         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),
@@ -540,6 +539,9 @@ public abstract class OperationResult implements Parcelable {
         MSG_MF_PASSPHRASE_FAIL (LogLevel.WARN, R.string.msg_mf_passphrase_fail),
         MSG_MF_PRIMARY_REPLACE_OLD (LogLevel.DEBUG, R.string.msg_mf_primary_replace_old),
         MSG_MF_PRIMARY_NEW (LogLevel.DEBUG, R.string.msg_mf_primary_new),
+        MSG_MF_RESTRICTED_MODE (LogLevel.INFO, R.string.msg_mf_restricted_mode),
+        MSG_MF_REQUIRE_DIVERT (LogLevel.OK, R.string.msg_mf_require_divert),
+        MSG_MF_REQUIRE_PASSPHRASE (LogLevel.OK, R.string.msg_mf_require_passphrase),
         MSG_MF_SUBKEY_CHANGE (LogLevel.INFO, R.string.msg_mf_subkey_change),
         MSG_MF_SUBKEY_NEW_ID (LogLevel.DEBUG, R.string.msg_mf_subkey_new_id),
         MSG_MF_SUBKEY_NEW (LogLevel.INFO, R.string.msg_mf_subkey_new),
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 f739cfb69..454a35cd6 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -395,12 +395,14 @@ public class PgpKeyOperation {
             return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
         }
 
-        if (saveParcel.isRestrictedOnly()) {
+        if (isDummy(masterSecretKey) || saveParcel.isRestrictedOnly()) {
+            log.add(LogType.MSG_MF_RESTRICTED_MODE, indent);
             return internalRestricted(sKR, saveParcel, log);
         }
 
         // Do we require a passphrase? If so, pass it along
         if (!isDivertToCard(masterSecretKey) && !cryptoInput.hasPassphrase()) {
+            log.add(LogType.MSG_MF_REQUIRE_PASSPHRASE, indent);
             return new PgpEditKeyResult(log, RequiredInputParcel.createRequiredPassphrase(
                     masterSecretKey.getKeyID(), cryptoInput.getSignatureTime()));
         }
@@ -971,7 +973,7 @@ public class PgpKeyOperation {
         progress(R.string.progress_done, 100);
 
         if (!nfcSignOps.isEmpty()) {
-            log.add(LogType.MSG_MF_INPUT_REQUIRED, indent);
+            log.add(LogType.MSG_MF_REQUIRE_DIVERT, indent);
             return new PgpEditKeyResult(log, nfcSignOps.build());
         }
 
@@ -1459,6 +1461,12 @@ public class PgpKeyOperation {
         return flags;
     }
 
+    private static boolean isDummy(PGPSecretKey secretKey) {
+        S2K s2k = secretKey.getS2K();
+        return s2k.getType() == S2K.GNU_DUMMY_S2K
+                && s2k.getProtectionMode() == S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY;
+    }
+
     private static boolean isDivertToCard(PGPSecretKey secretKey) {
         S2K s2k = secretKey.getS2K();
         return s2k.getType() == S2K.GNU_DUMMY_S2K
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
index 9fd278c13..88f258c58 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
@@ -85,7 +85,7 @@ public class SaveKeyringParcel implements Parcelable {
     /** Returns true iff this parcel does not contain any operations which require a passphrase. */
     public boolean isRestrictedOnly() {
         if (mNewUnlock != null || !mAddUserIds.isEmpty() || !mAddUserAttribute.isEmpty()
-                || !mAddSubKeys.isEmpty() || mChangePrimaryUserId != null || !mRevokeSubKeys .isEmpty()
+                || !mAddSubKeys.isEmpty() || mChangePrimaryUserId != null || !mRevokeUserIds.isEmpty()
                 || !mRevokeSubKeys.isEmpty()) {
             return false;
         }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java
index 6edc48b09..21aacd1f0 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java
@@ -36,7 +36,7 @@ public class CryptoInputParcel implements Parcelable {
 
     public CryptoInputParcel(Passphrase passphrase) {
         mSignatureTime = new Date();
-        mPassphrase = null;
+        mPassphrase = passphrase;
     }
 
     public CryptoInputParcel(Date signatureTime) {
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index a7e9cdf7e..76514575c 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -913,7 +913,7 @@
     <string name="msg_mf_error_passphrase_master">"Fatal error decrypting master key! This is likely a programming error, please file a bug report!"</string>
     <string name="msg_mf_error_pgp">"Internal OpenPGP error!"</string>
     <string name="msg_mf_error_sig">"Signature exception!"</string>
-    <string name="msg_mf_input_required">"Diverting to card/nfc for crypto operations"</string>
+    <string name="msg_mf_error_subkey_missing">"Tried to operate on missing subkey %s!"</string>
     <string name="msg_mf_master">"Modifying master certifications"</string>
     <string name="msg_mf_notation_empty">"Adding empty notation packet"</string>
     <string name="msg_mf_notation_pin">"Adding PIN notation packet"</string>
@@ -923,8 +923,10 @@
     <string name="msg_mf_passphrase_fail">"Passphrase for subkey could not be changed! (Does it have a different one from the other keys?)"</string>
     <string name="msg_mf_primary_replace_old">"Replacing certificate of previous primary user ID"</string>
     <string name="msg_mf_primary_new">"Generating new certificate for new primary user ID"</string>
+    <string name="msg_mf_restricted_mode">"Changing to restricted operation mode"</string>
     <string name="msg_mf_subkey_change">"Modifying subkey %s"</string>
-    <string name="msg_mf_error_subkey_missing">"Tried to operate on missing subkey %s!"</string>
+    <string name="msg_mf_require_divert">"Diverting to card/nfc for crypto operations"</string>
+    <string name="msg_mf_require_passphrase">"Passphrase required for operations"</string>
     <string name="msg_mf_subkey_new">"Adding new subkey of type %s"</string>
     <string name="msg_mf_subkey_new_id">"New subkey ID: %s"</string>
     <string name="msg_mf_error_past_expiry">"Expiry date cannot be in the past!"</string>