From bc3aa44b22546fb5b12c5ca9ab438c979a5449f1 Mon Sep 17 00:00:00 2001
From: Alex Fong <alexfongg@gmail.com>
Date: Tue, 19 Apr 2016 09:00:15 +0800
Subject: Refactored to reduce coupling between ChangeUnlockParcel and
 SaveKeyringParcel

---
 .../keychain/service/ChangeUnlockParcel.java       |  4 ++++
 .../keychain/service/SaveKeyringParcel.java        |  8 ++++++--
 .../keychain/ui/CreateKeyFinalFragment.java        |  5 +++--
 .../keychain/ui/EditKeyFragment.java               |  5 +++--
 .../keychain/operations/CertifyOperationTest.java  |  4 ++--
 .../keychain/operations/ExportTest.java            |  4 ++--
 .../operations/PromoteKeyOperationTest.java        |  2 +-
 .../keychain/pgp/PgpEncryptDecryptTest.java        |  4 ++--
 .../keychain/pgp/PgpKeyOperationTest.java          | 22 +++++++++++-----------
 .../pgp/UncachedKeyringCanonicalizeTest.java       |  2 +-
 .../keychain/pgp/UncachedKeyringMergeTest.java     |  4 ++--
 .../keychain/pgp/UncachedKeyringTest.java          |  2 +-
 12 files changed, 38 insertions(+), 28 deletions(-)

(limited to 'OpenKeychain')

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ChangeUnlockParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ChangeUnlockParcel.java
index fa8fd0841..974bb2413 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ChangeUnlockParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ChangeUnlockParcel.java
@@ -33,6 +33,10 @@ public class ChangeUnlockParcel implements Parcelable {
     // The new passphrase to use
     public final Passphrase mNewPassphrase;
 
+    public ChangeUnlockParcel(Passphrase newPassphrase) {
+        mNewPassphrase = newPassphrase;
+    }
+
     public ChangeUnlockParcel(Long masterKeyId, byte[] fingerprint, Passphrase newPassphrase) {
         if (newPassphrase == null) {
             throw new AssertionError("newPassphrase must be non-null. THIS IS A BUG!");
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 32ec48343..db6bbcbdb 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
@@ -103,11 +103,15 @@ public class SaveKeyringParcel implements Parcelable {
         mKeyserver = keysever;
     }
 
-    public void setNewUnlock(Passphrase passphrase) {
-        mNewUnlock = new ChangeUnlockParcel(mMasterKeyId, mFingerprint, passphrase);
+    public void setNewUnlock(ChangeUnlockParcel parcel) {
+        mNewUnlock = parcel;
     }
 
     public ChangeUnlockParcel getChangeUnlockParcel() {
+        if(mNewUnlock != null) {
+            mNewUnlock.mMasterKeyId = mMasterKeyId;
+            mNewUnlock.mFingerprint = mFingerprint;
+        }
         return mNewUnlock;
     }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java
index 97fdd49d4..8f486f437 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java
@@ -44,6 +44,7 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException;
 import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
+import org.sufficientlysecure.keychain.service.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
 import org.sufficientlysecure.keychain.service.UploadKeyringParcel;
@@ -288,7 +289,7 @@ public class CreateKeyFinalFragment extends Fragment {
                     2048, null, KeyFlags.AUTHENTICATION, 0L));
 
             // use empty passphrase
-            saveKeyringParcel.setNewUnlock(new Passphrase());
+            saveKeyringParcel.setNewUnlock(new ChangeUnlockParcel(new Passphrase()));
         } else {
             saveKeyringParcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(Algorithm.RSA,
                     3072, null, KeyFlags.CERTIFY_OTHER, 0L));
@@ -298,7 +299,7 @@ public class CreateKeyFinalFragment extends Fragment {
                     3072, null, KeyFlags.ENCRYPT_COMMS | KeyFlags.ENCRYPT_STORAGE, 0L));
 
             if(createKeyActivity.mPassphrase != null) {
-                saveKeyringParcel.setNewUnlock(createKeyActivity.mPassphrase);
+                saveKeyringParcel.setNewUnlock(new ChangeUnlockParcel(createKeyActivity.mPassphrase));
             }
         }
         String userId = KeyRing.createUserId(
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 8d5edb3e3..80fea7b23 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
@@ -50,6 +50,7 @@ import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.provider.KeychainContract.UserPackets;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException;
+import org.sufficientlysecure.keychain.service.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange;
 import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
@@ -338,8 +339,8 @@ public class EditKeyFragment extends QueueingCryptoOperationFragment<SaveKeyring
                     Bundle data = message.getData();
 
                     // cache new returned passphrase!
-                    mSaveKeyringParcel.setNewUnlock(
-                            (Passphrase) data.getParcelable(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE));
+                    mSaveKeyringParcel.setNewUnlock(new ChangeUnlockParcel(
+                            (Passphrase) data.getParcelable(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE)));
                 }
             }
         };
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java
index 3c487256e..94d26a7d1 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java
@@ -84,7 +84,7 @@ public class CertifyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("derp");
-            parcel.setNewUnlock(mKeyPhrase1);
+            parcel.setNewUnlock(new ChangeUnlockParcel(mKeyPhrase1));
 
             PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
@@ -108,7 +108,7 @@ public class CertifyOperationTest {
             parcel.mAddUserAttribute.add(
                     WrappedUserAttribute.fromSubpacket(random.nextInt(100)+1, uatdata));
 
-            parcel.setNewUnlock(mKeyPhrase2);
+            parcel.setNewUnlock(new ChangeUnlockParcel(mKeyPhrase2));
 
             PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
index 45be0861b..abf55b71b 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
@@ -106,7 +106,7 @@ public class ExportTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("snips");
-            parcel.setNewUnlock(mKeyPhrase1);
+            parcel.setNewUnlock(new ChangeUnlockParcel(mKeyPhrase1));
 
             PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             assertTrue("initial test key creation must succeed", result.success());
@@ -124,7 +124,7 @@ public class ExportTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("snails");
-            parcel.setNewUnlock(new Passphrase("1234"));
+            parcel.setNewUnlock(new ChangeUnlockParcel(new Passphrase("1234")));
 
             PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             assertTrue("initial test key creation must succeed", result.success());
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/PromoteKeyOperationTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/PromoteKeyOperationTest.java
index e99664d4e..98f9fe482 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/PromoteKeyOperationTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/PromoteKeyOperationTest.java
@@ -80,7 +80,7 @@ public class PromoteKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("derp");
-            parcel.setNewUnlock(mKeyPhrase1);
+            parcel.setNewUnlock(new ChangeUnlockParcel(mKeyPhrase1));
 
             PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java
index 916c54793..3770b825c 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java
@@ -103,7 +103,7 @@ public class PgpEncryptDecryptTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("bloom");
-            parcel.setNewUnlock(mKeyPhrase1);
+            parcel.setNewUnlock(new ChangeUnlockParcel(mKeyPhrase1));
 
             PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
@@ -121,7 +121,7 @@ public class PgpEncryptDecryptTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("belle");
-            parcel.setNewUnlock(mKeyPhrase2);
+            parcel.setNewUnlock(new ChangeUnlockParcel(mKeyPhrase2));
 
             PgpEditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
index 77a64d634..03b923d90 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
@@ -109,7 +109,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserAttribute.add(uat);
         }
 
-        parcel.setNewUnlock(passphrase);
+        parcel.setNewUnlock(new ChangeUnlockParcel(passphrase));
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         PgpEditKeyResult result = op.createSecretKeyRing(parcel);
@@ -149,7 +149,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, new Random().nextInt(256)+255, null, KeyFlags.CERTIFY_OTHER, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.setNewUnlock(passphrase);
+            parcel.setNewUnlock(new ChangeUnlockParcel(passphrase));
 
             assertFailure("creating ring with < 2048 bit keysize should fail", parcel,
                     LogType.MSG_CR_ERROR_KEYSIZE_2048);
@@ -160,7 +160,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ELGAMAL, 2048, null, KeyFlags.CERTIFY_OTHER, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.setNewUnlock(passphrase);
+            parcel.setNewUnlock(new ChangeUnlockParcel(passphrase));
 
             assertFailure("creating ring with ElGamal master key should fail", parcel,
                     LogType.MSG_CR_ERROR_FLAGS_ELGAMAL);
@@ -171,7 +171,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, null));
             parcel.mAddUserIds.add("lotus");
-            parcel.setNewUnlock(passphrase);
+            parcel.setNewUnlock(new ChangeUnlockParcel(passphrase));
 
             assertFailure("creating master key with null expiry should fail", parcel,
                     LogType.MSG_CR_ERROR_NULL_EXPIRY);
@@ -182,7 +182,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.SIGN_DATA, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.setNewUnlock(passphrase);
+            parcel.setNewUnlock(new ChangeUnlockParcel(passphrase));
 
             assertFailure("creating ring with non-certifying master key should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_CERTIFY);
@@ -192,7 +192,7 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ECDSA, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.CERTIFY_OTHER, 0L));
-            parcel.setNewUnlock(passphrase);
+            parcel.setNewUnlock(new ChangeUnlockParcel(passphrase));
 
             assertFailure("creating ring without user ids should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_USER_ID);
@@ -201,7 +201,7 @@ public class PgpKeyOperationTest {
         {
             parcel.reset();
             parcel.mAddUserIds.add("shy");
-            parcel.setNewUnlock(passphrase);
+            parcel.setNewUnlock(new ChangeUnlockParcel(passphrase));
 
             assertFailure("creating ring with no master key should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_MASTER);
@@ -841,7 +841,7 @@ public class PgpKeyOperationTest {
 
         parcelKey.mAddUserIds.add("yubikey");
 
-        parcelKey.setNewUnlock(passphrase);
+        parcelKey.setNewUnlock(new ChangeUnlockParcel(passphrase));
         PgpKeyOperation opSecurityToken = new PgpKeyOperation(null);
 
         PgpEditKeyResult resultSecurityToken = opSecurityToken.createSecretKeyRing(parcelKey);
@@ -1156,7 +1156,7 @@ public class PgpKeyOperationTest {
     public void testPassphraseChange() throws Exception {
 
         // change passphrase to empty
-        parcel.setNewUnlock(new Passphrase());
+        parcel.setNewUnlock(new ChangeUnlockParcel(new Passphrase()));
         // note that canonicalization here necessarily strips the empty notation packet
         UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, cryptoInput);
 
@@ -1171,7 +1171,7 @@ public class PgpKeyOperationTest {
         // modify keyring, change to non-empty passphrase
         Passphrase otherPassphrase = TestingUtils.genPassphrase(true);
         CryptoInputParcel otherCryptoInput = new CryptoInputParcel(otherPassphrase);
-        parcel.setNewUnlock(otherPassphrase);
+        parcel.setNewUnlock(new ChangeUnlockParcel(otherPassphrase));
         modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB,
                 new CryptoInputParcel(new Date(), new Passphrase()));
 
@@ -1197,7 +1197,7 @@ public class PgpKeyOperationTest {
                 PacketTags.SECRET_SUBKEY, sKeyNoPassphrase.tag);
 
         Passphrase otherPassphrase2 = TestingUtils.genPassphrase(true);
-        parcel.setNewUnlock(otherPassphrase2);
+        parcel.setNewUnlock(new ChangeUnlockParcel(otherPassphrase2));
         {
             // if we replace a secret key with one without passphrase
             modified = KeyringTestingHelper.removePacket(modified, sKeyNoPassphrase.position);
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
index cfaa6d7f7..de3ae2941 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
@@ -117,7 +117,7 @@ public class UncachedKeyringCanonicalizeTest {
         }
 
         // passphrase is tested in PgpKeyOperationTest, just use empty here
-        parcel.setNewUnlock(new Passphrase());
+        parcel.setNewUnlock(new ChangeUnlockParcel(new Passphrase()));
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         PgpEditKeyResult result = op.createSecretKeyRing(parcel);
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
index 2e99f8bf6..a8e28a7c6 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java
@@ -114,7 +114,7 @@ public class UncachedKeyringMergeTest {
             }
 
             // passphrase is tested in PgpKeyOperationTest, just use empty here
-            parcel.setNewUnlock(new Passphrase());
+            parcel.setNewUnlock(new ChangeUnlockParcel(new Passphrase()));
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResult.OperationLog log = new OperationResult.OperationLog();
@@ -131,7 +131,7 @@ public class UncachedKeyringMergeTest {
 
             parcel.mAddUserIds.add("shy");
             // passphrase is tested in PgpKeyOperationTest, just use empty here
-            parcel.setNewUnlock(new Passphrase());
+            parcel.setNewUnlock(new ChangeUnlockParcel(new Passphrase()));
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResult.OperationLog log = new OperationResult.OperationLog();
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
index 2d33f6a57..ef82aa527 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
@@ -78,7 +78,7 @@ public class UncachedKeyringTest {
             parcel.mAddUserAttribute.add(uat);
         }
         // passphrase is tested in PgpKeyOperationTest, just use empty here
-        parcel.setNewUnlock(new Passphrase());
+        parcel.setNewUnlock(new ChangeUnlockParcel(new Passphrase()));
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         PgpEditKeyResult result = op.createSecretKeyRing(parcel);
-- 
cgit v1.2.3