small improvements regarding pin tests and logging

4 files changed, 33 insertions, 6 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 6b53a93ce..103e2dc88 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
@@ -916,7 +916,7 @@ public class PgpKeyOperationTest {
         parcel.mNewUnlock = new ChangeUnlockParcel("");
         // note that canonicalization here necessarily strips the empty notation packet
         UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB,
-                passphrase, true, false);
+                passphrase);
 
         Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
                 3, onlyB.size());
@@ -929,7 +929,7 @@ public class PgpKeyOperationTest {
         // modify keyring, change to non-empty passphrase
         String otherPassphrase = TestingUtils.genPassphrase(true);
         parcel.mNewUnlock = new ChangeUnlockParcel(otherPassphrase);
-        modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, "", true, false);
+        modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, "");
 
         Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
                 3, onlyB.size());
@@ -989,11 +989,15 @@ public class PgpKeyOperationTest {
     @Test
     public void testUnlockPin() throws Exception {
 
+        String pin = "5235125";
+
         // change passphrase to a pin type
-        parcel.mNewUnlock = new ChangeUnlockParcel(null, "52351");
+        parcel.mNewUnlock = new ChangeUnlockParcel(null, pin);
         UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
 
-        Assert.assertEquals("exactly four packets should have been modified (the secret keys + notation packet)",
+        Assert.assertEquals("exactly three packets should have been added (the secret keys + notation packet)",
+                3, onlyA.size());
+        Assert.assertEquals("exactly four packets should have been added (the secret keys + notation packet)",
                 4, onlyB.size());
 
         RawPacket dkSig = onlyB.get(1);
@@ -1001,11 +1005,25 @@ public class PgpKeyOperationTest {
                 PacketTags.SIGNATURE, dkSig.tag);
 
         // check that notation data contains pin
-        CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), false, 0);
+        CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
+                modified.getEncoded(), false, 0);
         Assert.assertEquals("secret key type should be 'pin' after this",
                 SecretKeyType.PIN,
                 secretRing.getSecretKey().getSecretKeyType());
 
+        // need to sleep for a sec, so the timestamp changes for notation data
+        Thread.sleep(1000);
+
+        {
+            parcel.mNewUnlock = new ChangeUnlockParcel("phrayse", null);
+            applyModificationWithChecks(parcel, modified, onlyA, onlyB, pin, true, false);
+
+            Assert.assertEquals("exactly four packets should have been removed (the secret keys + notation packet)",
+                    4, onlyA.size());
+            Assert.assertEquals("exactly three packets should have been added (no more notation packet)",
+                    3, onlyB.size());
+        }
+
     }
 
     private static UncachedKeyRing applyModificationWithChecks(SaveKeyringParcel parcel,
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 606dd49d5..426b0827e 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
@@ -461,6 +461,8 @@ 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_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),
         MSG_MF_PASSPHRASE (LogLevel.INFO, R.string.msg_mf_passphrase),
         MSG_MF_PASSPHRASE_KEY (LogLevel.DEBUG, R.string.msg_mf_passphrase_key),
         MSG_MF_PASSPHRASE_EMPTY_RETRY (LogLevel.DEBUG, R.string.msg_mf_passphrase_empty_retry),
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 e39bde6b0..5ac5f7a9a 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -921,6 +921,9 @@ public class PgpKeyOperation {
 
             // if there is any old packet with notation data
             if (hasNotationData(sKR)) {
+
+                log.add(LogType.MSG_MF_NOTATION_EMPTY, indent);
+
                 // add packet with EMPTY notation data (updates old one, but will be stripped later)
                 PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
                         masterPrivateKey.getPublicKeyPacket().getAlgorithm(), HashAlgorithmTags.SHA512)
@@ -945,6 +948,8 @@ public class PgpKeyOperation {
         if (newUnlock.mNewPin != null) {
             sKR = applyNewPassphrase(sKR, masterPublicKey, passphrase, newUnlock.mNewPin, log, indent);
 
+            log.add(LogType.MSG_MF_NOTATION_PIN, indent);
+
             // add packet with "pin" notation data
             PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
                     masterPrivateKey.getPublicKeyPacket().getAlgorithm(), HashAlgorithmTags.SHA512)
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 05ea2a99a..be409078a 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -803,7 +803,9 @@
     <string name="msg_mf_error_pgp">"Internal PGP error!"</string>
     <string name="msg_mf_error_sig">"Signature exception!"</string>
     <string name="msg_mf_master">"Modifying master certifications"</string>
-    <string name="msg_mf_passphrase">"Changing passphrase for keyring…"</string>
+    <string name="msg_mf_notation_empty">"Adding empty notation packet"</string>
+    <string name="msg_mf_notation_pin">"Adding PIN notation packet"</string>
+    <string name="msg_mf_passphrase">"Changing passphrase for keyring"</string>
     <string name="msg_mf_passphrase_key">"Re-encrypting subkey %s with new passphrase"</string>
     <string name="msg_mf_passphrase_empty_retry">"Setting new passphrase failed, trying again with empty old passphrase"</string>
     <string name="msg_mf_passphrase_fail">"Passphrase for subkey could not be changed! (Does it have a different one from the other keys?)"</string>