diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java index a21e36771..a7b2bf59e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java @@ -76,7 +76,7 @@ public class CertifyOperation extends BaseReadWriteOperation userIds = action.getUserIds(); + if (userIds != null && !userIds.isEmpty()) { + log.add(LogType.MSG_CRT_CERTIFY_UIDS, 2, userIds.size(), + KeyFormattingUtils.convertKeyIdToHex(action.getMasterKeyId())); // fetch public key ring, add the certification and return it - for (String userId : action.mUserIds) { + for (String userId : userIds) { try { PGPSignature sig = signatureGenerator.generateCertification(userId, publicKey); publicKey = PGPPublicKey.addCertification(publicKey, userId, sig); @@ -96,12 +98,13 @@ public class PgpCertifyOperation { } - if (action.mUserAttributes != null) { - log.add(LogType.MSG_CRT_CERTIFY_UATS, 2, action.mUserAttributes.size(), - KeyFormattingUtils.convertKeyIdToHex(action.mMasterKeyId)); + ArrayList userAttributes = action.getUserAttributes(); + if (userAttributes != null && !userAttributes.isEmpty()) { + log.add(LogType.MSG_CRT_CERTIFY_UATS, 2, userAttributes.size(), + KeyFormattingUtils.convertKeyIdToHex(action.getMasterKeyId())); // fetch public key ring, add the certification and return it - for (WrappedUserAttribute userAttribute : action.mUserAttributes) { + for (WrappedUserAttribute userAttribute : userAttributes) { PGPUserAttributeSubpacketVector vector = userAttribute.getVector(); try { PGPSignature sig = signatureGenerator.generateCertification(vector, publicKey); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/CertifyActionsParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/CertifyActionsParcel.java index b7cc470e3..54a2c5fd5 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/CertifyActionsParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/CertifyActionsParcel.java @@ -18,99 +18,80 @@ package org.sufficientlysecure.keychain.service; -import android.os.Parcel; -import android.os.Parcelable; -import java.io.Serializable; import java.util.ArrayList; +import java.util.Collection; import java.util.List; -import org.sufficientlysecure.keychain.pgp.WrappedUserAttribute; +import android.os.Parcelable; +import android.support.annotation.CheckResult; +import android.support.annotation.Nullable; + +import com.google.auto.value.AutoValue; import org.sufficientlysecure.keychain.keyimport.ParcelableHkpKeyserver; +import org.sufficientlysecure.keychain.pgp.WrappedUserAttribute; -/** - * This class is a a transferable representation for a number of keyrings to - * be certified. - */ -public class CertifyActionsParcel implements Parcelable { +@AutoValue +public abstract class CertifyActionsParcel implements Parcelable { + public abstract long getMasterKeyId(); + public abstract ArrayList getCertifyActions(); + @Nullable + public abstract ParcelableHkpKeyserver getParcelableKeyServer(); - // the master key id to certify with - final public long mMasterKeyId; - public CertifyLevel mLevel; - - public ArrayList mCertifyActions = new ArrayList<>(); - - public ParcelableHkpKeyserver keyServerUri; - - public CertifyActionsParcel(long masterKeyId) { - mMasterKeyId = masterKeyId; - mLevel = CertifyLevel.DEFAULT; + public static Builder builder(long masterKeyId) { + return new AutoValue_CertifyActionsParcel.Builder() + .setMasterKeyId(masterKeyId) + .setCertifyActions(new ArrayList()); } - public CertifyActionsParcel(Parcel source) { - mMasterKeyId = source.readLong(); - // just like parcelables, this is meant for ad-hoc IPC only and is NOT portable! - mLevel = CertifyLevel.values()[source.readInt()]; - keyServerUri = source.readParcelable(ParcelableHkpKeyserver.class.getClassLoader()); + @AutoValue.Builder + public abstract static class Builder { + abstract Builder setMasterKeyId(long masterKeyId); + public abstract Builder setCertifyActions(ArrayList certifyActions); + public abstract Builder setParcelableKeyServer(ParcelableHkpKeyserver uri); - mCertifyActions = (ArrayList) source.readSerializable(); - } + abstract ArrayList getCertifyActions(); - public void add(CertifyAction action) { - mCertifyActions.add(action); - } - - @Override - public void writeToParcel(Parcel destination, int flags) { - destination.writeLong(mMasterKeyId); - destination.writeInt(mLevel.ordinal()); - destination.writeParcelable(keyServerUri, flags); - - destination.writeSerializable(mCertifyActions); - } - - public static final Creator CREATOR = new Creator() { - public CertifyActionsParcel createFromParcel(final Parcel source) { - return new CertifyActionsParcel(source); + public void addAction(CertifyAction action) { + getCertifyActions().add(action); + } + public void addActions(Collection certifyActions) { + getCertifyActions().addAll(certifyActions); } - public CertifyActionsParcel[] newArray(final int size) { - return new CertifyActionsParcel[size]; + public abstract CertifyActionsParcel build(); + } + + @AutoValue + public abstract static class CertifyAction implements Parcelable { + public abstract long getMasterKeyId(); + @Nullable + public abstract ArrayList getUserIds(); + @Nullable + public abstract ArrayList getUserAttributes(); + + public static CertifyAction createForUserIds(long masterKeyId, List userIds) { + return new AutoValue_CertifyActionsParcel_CertifyAction(masterKeyId, new ArrayList<>(userIds), null); } - }; - // TODO make this parcelable - public static class CertifyAction implements Serializable { - final public long mMasterKeyId; + public static CertifyAction createForUserAttributes(long masterKeyId, List attributes) { + return new AutoValue_CertifyActionsParcel_CertifyAction(masterKeyId, null, new ArrayList<>(attributes)); + } - final public ArrayList mUserIds; - final public ArrayList mUserAttributes; + @CheckResult + public CertifyAction withAddedUserIds(ArrayList addedUserIds) { + if (getUserAttributes() != null) { + throw new IllegalStateException("Can't add user ids to user attribute certification parcel!"); + } + ArrayList prevUserIds = getUserIds(); + if (prevUserIds == null) { + throw new IllegalStateException("Can't add user ids to user attribute certification parcel!"); + } - public CertifyAction(long masterKeyId, List userIds, List attributes) { - mMasterKeyId = masterKeyId; - mUserIds = userIds == null ? null : new ArrayList<>(userIds); - mUserAttributes = attributes == null ? null : new ArrayList<>(attributes); + ArrayList userIds = new ArrayList<>(prevUserIds); + userIds.addAll(addedUserIds); + return new AutoValue_CertifyActionsParcel_CertifyAction(getMasterKeyId(), userIds, null); } } - - @Override - public int describeContents() { - return 0; - } - - @Override - public String toString() { - String out = "mMasterKeyId: " + mMasterKeyId + "\n"; - out += "mLevel: " + mLevel + "\n"; - out += "mCertifyActions: " + mCertifyActions + "\n"; - - return out; - } - - // All supported algorithms - public enum CertifyLevel { - DEFAULT, NONE, CASUAL, POSITIVE - } - } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java index c343ab4a3..f08ffe8ad 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyKeyFragment.java @@ -140,18 +140,17 @@ public class CertifyKeyFragment long selectedKeyId = mCertifyKeySpinner.getSelectedKeyId(); // fill values for this action - CertifyActionsParcel actionsParcel = new CertifyActionsParcel(selectedKeyId); - actionsParcel.mCertifyActions.addAll(certifyActions); + CertifyActionsParcel.Builder actionsParcel = CertifyActionsParcel.builder(selectedKeyId); + actionsParcel.addActions(certifyActions); if (mUploadKeyCheckbox.isChecked()) { - actionsParcel.keyServerUri = Preferences.getPreferences(getActivity()) - .getPreferredKeyserver(); + actionsParcel.setParcelableKeyServer(Preferences.getPreferences(getActivity()).getPreferredKeyserver()); } - // cached for next cryptoOperation loop - cacheActionsParcel(actionsParcel); - - return actionsParcel; + // cache for next cryptoOperation loop + CertifyActionsParcel certifyActionsParcel = actionsParcel.build(); + cacheActionsParcel(certifyActionsParcel); + return certifyActionsParcel; } @Override diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/MultiUserIdsAdapter.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/MultiUserIdsAdapter.java index b6da1eddf..304b0d72f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/MultiUserIdsAdapter.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/MultiUserIdsAdapter.java @@ -17,6 +17,9 @@ package org.sufficientlysecure.keychain.ui.adapter; + +import java.util.ArrayList; + import android.content.Context; import android.database.Cursor; import android.os.Parcel; @@ -35,8 +38,6 @@ import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.pgp.KeyRing; import org.sufficientlysecure.keychain.service.CertifyActionsParcel.CertifyAction; -import java.util.ArrayList; - public class MultiUserIdsAdapter extends CursorAdapter { private LayoutInflater mInflater; private final ArrayList mCheckStates; @@ -178,11 +179,12 @@ public class MultiUserIdsAdapter extends CursorAdapter { p.recycle(); CertifyAction action = actions.get(keyId); - if (actions.get(keyId) == null) { - actions.put(keyId, new CertifyAction(keyId, uids, null)); + if (action == null) { + action = CertifyAction.createForUserIds(keyId, uids); } else { - action.mUserIds.addAll(uids); + action = action.withAddedUserIds(uids); } + actions.put(keyId, action); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/LinkedIdViewFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/LinkedIdViewFragment.java index 5cb0bc42f..12f54b238 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/LinkedIdViewFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/LinkedIdViewFragment.java @@ -542,14 +542,14 @@ public class LinkedIdViewFragment extends CryptoOperationFragment implements @Nullable @Override public Parcelable createOperationInput() { - CertifyAction action = new CertifyAction(mMasterKeyId, null, + CertifyAction action = CertifyAction.createForUserAttributes(mMasterKeyId, Collections.singletonList(mLinkedId.toUserAttribute())); // fill values for this action - CertifyActionsParcel parcel = new CertifyActionsParcel(mCertifyKeyId); - parcel.mCertifyActions.addAll(Collections.singletonList(action)); + CertifyActionsParcel.Builder builder = CertifyActionsParcel.builder(mCertifyKeyId); + builder.addActions(Collections.singletonList(action)); - return parcel; + return builder.build(); } @Override 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 0423a83b9..46d95188c 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/CertifyOperationTest.java @@ -153,10 +153,10 @@ public class CertifyOperationTest { Certs.UNVERIFIED, ring.getVerified()); } - CertifyActionsParcel actions = new CertifyActionsParcel(mStaticRing1.getMasterKeyId()); - actions.add(new CertifyAction(mStaticRing2.getMasterKeyId(), - mStaticRing2.getPublicKey().getUnorderedUserIds(), null)); - CertifyResult result = op.execute(actions, CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); + CertifyActionsParcel.Builder actions = CertifyActionsParcel.builder(mStaticRing1.getMasterKeyId()); + actions.addAction(CertifyAction.createForUserIds(mStaticRing2.getMasterKeyId(), + mStaticRing2.getPublicKey().getUnorderedUserIds())); + CertifyResult result = op.execute(actions.build(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); Assert.assertTrue("certification must succeed", result.success()); @@ -181,10 +181,10 @@ public class CertifyOperationTest { Certs.UNVERIFIED, ring.getVerified()); } - CertifyActionsParcel actions = new CertifyActionsParcel(mStaticRing1.getMasterKeyId()); - actions.add(new CertifyAction(mStaticRing2.getMasterKeyId(), null, + CertifyActionsParcel.Builder actions = CertifyActionsParcel.builder(mStaticRing1.getMasterKeyId()); + actions.addAction(CertifyAction.createForUserAttributes(mStaticRing2.getMasterKeyId(), mStaticRing2.getPublicKey().getUnorderedUserAttributes())); - CertifyResult result = op.execute(actions, CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); + CertifyResult result = op.execute(actions.build(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); Assert.assertTrue("certification must succeed", result.success()); @@ -203,11 +203,11 @@ public class CertifyOperationTest { CertifyOperation op = new CertifyOperation(RuntimeEnvironment.application, KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application), null, null); - CertifyActionsParcel actions = new CertifyActionsParcel(mStaticRing1.getMasterKeyId()); - actions.add(new CertifyAction(mStaticRing1.getMasterKeyId(), - mStaticRing2.getPublicKey().getUnorderedUserIds(), null)); + CertifyActionsParcel.Builder actions = CertifyActionsParcel.builder(mStaticRing1.getMasterKeyId()); + actions.addAction(CertifyAction.createForUserIds(mStaticRing1.getMasterKeyId(), + mStaticRing2.getPublicKey().getUnorderedUserIds())); - CertifyResult result = op.execute(actions, CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); + CertifyResult result = op.execute(actions.build(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); Assert.assertFalse("certification with itself must fail!", result.success()); Assert.assertTrue("error msg must be about self certification", @@ -221,12 +221,12 @@ public class CertifyOperationTest { KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application), null, null); { - CertifyActionsParcel actions = new CertifyActionsParcel(mStaticRing1.getMasterKeyId()); + CertifyActionsParcel.Builder actions = CertifyActionsParcel.builder(mStaticRing1.getMasterKeyId()); ArrayList uids = new ArrayList(); uids.add("nonexistent"); - actions.add(new CertifyAction(1234L, uids, null)); + actions.addAction(CertifyAction.createForUserIds(1234L, uids)); - CertifyResult result = op.execute(actions, CryptoInputParcel.createCryptoInputParcel(new Date(), + CertifyResult result = op.execute(actions.build(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); Assert.assertFalse("certification of nonexistent key must fail", result.success()); @@ -235,11 +235,11 @@ public class CertifyOperationTest { } { - CertifyActionsParcel actions = new CertifyActionsParcel(1234L); - actions.add(new CertifyAction(mStaticRing1.getMasterKeyId(), - mStaticRing2.getPublicKey().getUnorderedUserIds(), null)); + CertifyActionsParcel.Builder actions = CertifyActionsParcel.builder(1234L); + actions.addAction(CertifyAction.createForUserIds(mStaticRing1.getMasterKeyId(), + mStaticRing2.getPublicKey().getUnorderedUserIds())); - CertifyResult result = op.execute(actions, CryptoInputParcel.createCryptoInputParcel(new Date(), + CertifyResult result = op.execute(actions.build(), CryptoInputParcel.createCryptoInputParcel(new Date(), mKeyPhrase1)); Assert.assertFalse("certification of nonexistent key must fail", result.success()); 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 b8a1e3525..0a7a1c20d 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java @@ -18,6 +18,14 @@ package org.sufficientlysecure.keychain.pgp; + +import java.io.ByteArrayInputStream; +import java.security.Security; +import java.util.ArrayList; +import java.util.Date; +import java.util.Iterator; +import java.util.Random; + import org.bouncycastle.bcpg.BCPGInputStream; import org.bouncycastle.bcpg.PacketTags; import org.bouncycastle.bcpg.S2K; @@ -304,7 +312,8 @@ public class UncachedKeyringMergeTest { ringB.getEncoded(), 0).getSecretKey(); secretKey.unlock(new Passphrase()); PgpCertifyOperation op = new PgpCertifyOperation(); - CertifyAction action = new CertifyAction(pubRing.getMasterKeyId(), publicRing.getPublicKey().getUnorderedUserIds(), null); + CertifyAction action = CertifyAction.createForUserIds( + pubRing.getMasterKeyId(), publicRing.getPublicKey().getUnorderedUserIds()); // sign all user ids PgpCertifyResult result = op.certify(secretKey, publicRing, new OperationLog(), 0, action, null, new Date()); Assert.assertTrue("certification must succeed", result.success()); diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java index deb94f439..98623da74 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/remote/KeychainExternalProviderTest.java @@ -194,11 +194,12 @@ public class KeychainExternalProviderTest { } private void certifyKey(long secretMasterKeyId, long publicMasterKeyId, String userId) { - CertifyActionsParcel certifyActionsParcel = new CertifyActionsParcel(secretMasterKeyId); - certifyActionsParcel.add(new CertifyAction(publicMasterKeyId, Collections.singletonList(userId), null)); + CertifyActionsParcel.Builder certifyActionsParcel = CertifyActionsParcel.builder(secretMasterKeyId); + certifyActionsParcel.addAction( + CertifyAction.createForUserIds(publicMasterKeyId, Collections.singletonList(userId))); CertifyOperation op = new CertifyOperation( RuntimeEnvironment.application, databaseInteractor, new ProgressScaler(), null); - CertifyResult certifyResult = op.execute(certifyActionsParcel, CryptoInputParcel.createCryptoInputParcel()); + CertifyResult certifyResult = op.execute(certifyActionsParcel.build(), CryptoInputParcel.createCryptoInputParcel()); assertTrue(certifyResult.success()); }