PublicKeyChecker: Convert to a builder pattern
Instead of overloading the constructor and check methods with optional
arguments, expose optional arguments via setters. We are about to add
another one, and this was getting unwieldy.
We still use a Factory to create GerritPublicKeyChecker, which parses
the config once in a @Singleton and subsequently configures all
checkers with that config. Since the checker is initially created when
a store might not be available (because the repo hasn't been opened),
we need to separate the trusted key config setter from the store
setter, and check state at use time.
Change-Id: Ide6bc76a8180f91caa6f03f1a0eb25ec6d20ba45
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java
index 6fd8bac..54c12c6 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java
@@ -19,6 +19,9 @@
import org.eclipse.jgit.util.NB;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
public class Fingerprint {
private final byte[] fp;
@@ -33,6 +36,14 @@
NB.decodeUInt16(fp, 16), NB.decodeUInt16(fp, 18));
}
+ public static Map<Long, Fingerprint> byId(Iterable<Fingerprint> fps) {
+ Map<Long, Fingerprint> result = new HashMap<>();
+ for (Fingerprint fp : fps) {
+ result.put(fp.getId(), fp);
+ }
+ return Collections.unmodifiableMap(result);
+ }
+
private static byte[] checkLength(byte[] fp) {
checkArgument(fp.length == 20,
"fingerprint must be 20 bytes, got %s", fp.length);
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
index d942c75..88ebc8d 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
@@ -14,14 +14,14 @@
package com.google.gerrit.gpg;
-import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString;
import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_GPGKEY;
import com.google.common.base.CharMatcher;
import com.google.common.base.MoreObjects;
import com.google.common.collect.FluentIterable;
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
import com.google.common.io.BaseEncoding;
import com.google.gerrit.common.PageLinks;
@@ -44,11 +44,10 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
@@ -61,18 +60,13 @@
private static final Logger log =
LoggerFactory.getLogger(GerritPublicKeyChecker.class);
- private final Provider<ReviewDb> db;
- private final String webUrl;
- private final IdentifiedUser.GenericFactory userFactory;
- private final IdentifiedUser expectedUser;
-
@Singleton
public static class Factory {
private final Provider<ReviewDb> db;
private final String webUrl;
private final IdentifiedUser.GenericFactory userFactory;
private final int maxTrustDepth;
- private final ImmutableList<Fingerprint> trusted;
+ private final ImmutableMap<Long, Fingerprint> trusted;
@Inject
Factory(@GerritServerConfig Config cfg,
@@ -86,52 +80,50 @@
String[] strs = cfg.getStringList("receive", null, "trustedKey");
if (strs.length != 0) {
- List<Fingerprint> fps = new ArrayList<>(strs.length);
+ Map<Long, Fingerprint> fps =
+ Maps.newHashMapWithExpectedSize(strs.length);
for (String str : strs) {
str = CharMatcher.WHITESPACE.removeFrom(str).toUpperCase();
- fps.add(new Fingerprint(BaseEncoding.base16().decode(str)));
+ Fingerprint fp = new Fingerprint(BaseEncoding.base16().decode(str));
+ fps.put(fp.getId(), fp);
}
- trusted = ImmutableList.copyOf(fps);
+ trusted = ImmutableMap.copyOf(fps);
} else {
trusted = null;
}
}
- /**
- * Create a checker that can check arbitrary public keys.
- * <p>
- * Each key is checked against the set of identities in the database
- * belonging to the same user as the key.
- *
- * @return a new checker.
- */
public GerritPublicKeyChecker create() {
- return new GerritPublicKeyChecker(this, null);
- }
-
- /**
- * Create a checker for checking a single public key against a known user.
- * <p>
- * The top-level key passed to {@link #check(PGPPublicKey, PublicKeyStore)}
- * must belong to the given user. (Other keys checked in the course of
- * verifying the web of trust are checked against the set of identities in
- * the database belonging to the same user as the key.)
- *
- * @param expectedUser the user
- * @return a new checker.
- */
- public GerritPublicKeyChecker create(IdentifiedUser expectedUser) {
- checkNotNull(expectedUser);
- return new GerritPublicKeyChecker(this, expectedUser);
+ return new GerritPublicKeyChecker(this);
}
}
- private GerritPublicKeyChecker(Factory factory, IdentifiedUser expectedUser) {
- super(factory.maxTrustDepth, factory.trusted);
+ private final Provider<ReviewDb> db;
+ private final String webUrl;
+ private final IdentifiedUser.GenericFactory userFactory;
+
+ private IdentifiedUser expectedUser;
+
+ private GerritPublicKeyChecker(Factory factory) {
this.db = factory.db;
this.webUrl = factory.webUrl;
this.userFactory = factory.userFactory;
+ if (factory.trusted != null) {
+ enableTrust(factory.maxTrustDepth, factory.trusted);
+ }
+ }
+
+ /**
+ * Set the expected user for this checker.
+ * <p>
+ * If set, the top-level key passed to {@link #check(PGPPublicKey)} must
+ * belong to the given user. (Other keys checked in the course of verifying
+ * the web of trust are checked against the set of identities in the database
+ * belonging to the same user as the key.)
+ */
+ public GerritPublicKeyChecker setExpectedUser(IdentifiedUser expectedUser) {
this.expectedUser = expectedUser;
+ return this;
}
@Override
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java
index fbc3d44..6491ef1 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java
@@ -40,7 +40,7 @@
AllUsersName allUsers,
@Assisted IdentifiedUser expectedUser,
@Assisted boolean checkNonce) {
- super(keyCheckerFactory.create(expectedUser), checkNonce);
+ super(keyCheckerFactory.create().setExpectedUser(expectedUser), checkNonce);
this.repoManager = repoManager;
this.allUsers = allUsers;
}
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java
index 725a6e1..d19264a 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java
@@ -32,8 +32,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -45,66 +43,71 @@
// https://tools.ietf.org/html/rfc4880#section-5.2.3.13
private static final int COMPLETE_TRUST = 120;
- private final Map<Long, Fingerprint> trusted;
- private final int maxTrustDepth;
-
- /** Create a new checker that does not check the web of trust. */
- public PublicKeyChecker() {
- this(0, null);
- }
+ private PublicKeyStore store;
+ private Map<Long, Fingerprint> trusted;
+ private int maxTrustDepth;
/**
+ * Enable web-of-trust checks.
+ * <p>
+ * If enabled, a store must be set with {@link #setStore(PublicKeyStore)}.
+ * (These methods are separate since the store is a closeable resource that
+ * may not be available when reading trusted keys from a config.)
+ *
* @param maxTrustDepth maximum depth to search while looking for a trusted
* key.
- * @param trusted ultimately trusted key fingerprints; may not be empty. If
- * null, disable web-of-trust checks.
+ * @param trusted ultimately trusted key fingerprints, keyed by fingerprint;
+ * may not be empty. To construct a map, see {@link
+ * Fingerprint#byId(Iterable)}.
+ * @return a reference to this object.
*/
- public PublicKeyChecker(int maxTrustDepth, Collection<Fingerprint> trusted) {
- if (trusted != null) {
- if (maxTrustDepth <= 0) {
+ public PublicKeyChecker enableTrust(int maxTrustDepth,
+ Map<Long, Fingerprint> trusted) {
+ if (maxTrustDepth <= 0) {
+ throw new IllegalArgumentException(
+ "maxTrustDepth must be positive, got: " + maxTrustDepth);
+ }
+ if (trusted == null || trusted.isEmpty()) {
throw new IllegalArgumentException(
- "maxTrustDepth must be positive, got: " + maxTrustDepth);
- }
- if (trusted.isEmpty()) {
- throw new IllegalArgumentException("at least one trusted key required");
- }
- this.trusted = new HashMap<>();
- for (Fingerprint fp : trusted) {
- this.trusted.put(fp.getId(), fp);
- }
- } else {
- this.trusted = null;
+ "at least one trusted key is required");
}
this.maxTrustDepth = maxTrustDepth;
+ this.trusted = trusted;
+ return this;
+ }
+
+ /** Disable web-of-trust checks. */
+ public PublicKeyChecker disableTrust() {
+ store = null;
+ trusted = null;
+ return this;
}
/**
- * Check a public key, including its web of trust.
- *
- * @param key the public key.
- * @param store a store to read public keys from for trust checks. If this
- * store is not configured for web-of-trust checks, this argument is
- * ignored.
- * @return the result of the check.
+ * Set the public key store for web-of-trust checks.
+ * <p>
+ * If set, {@link #enableTrust(int, Map)} must also be called.
*/
- public final CheckResult check(PGPPublicKey key, PublicKeyStore store) {
- if (trusted == null) {
- return check(key);
- } else if (store == null) {
- throw new IllegalArgumentException(
- "PublicKeyStore required for web of trust checks");
+ public PublicKeyChecker setStore(PublicKeyStore store) {
+ if (store == null) {
+ throw new IllegalArgumentException("PublicKeyStore is required");
}
- return check(key, store, 0, true, new HashSet<Fingerprint>());
+ this.store = store;
+ return this;
}
/**
- * Check only a public key, not including its web of trust.
+ * Check a public key.
*
* @param key the public key.
* @return the result of the check.
*/
public final CheckResult check(PGPPublicKey key) {
- return check(key, null, 0, false, null);
+ if (store == null && trusted != null) {
+ throw new IllegalStateException("PublicKeyStore is required");
+ }
+ return check(key, 0, true,
+ trusted != null ? new HashSet<Fingerprint>() : null);
}
/**
@@ -115,16 +118,16 @@
*
* @param key the public key.
* @param depth the depth from the initial key passed to {@link #check(
- * PGPPublicKey, PublicKeyStore)}: 0 if this was the initial key, up to a
- * maximum of {@code maxTrustDepth}.
+ * PGPPublicKey)}: 0 if this was the initial key, up to a maximum of
+ * {@code maxTrustDepth}.
* @return the result of the custom check.
*/
public CheckResult checkCustom(PGPPublicKey key, int depth) {
return CheckResult.ok();
}
- private CheckResult check(PGPPublicKey key, PublicKeyStore store, int depth,
- boolean expand, Set<Fingerprint> seen) {
+ private CheckResult check(PGPPublicKey key, int depth, boolean expand,
+ Set<Fingerprint> seen) {
CheckResult basicResult = checkBasic(key);
CheckResult customResult = checkCustom(key, depth);
CheckResult trustResult = checkWebOfTrust(key, store, depth, seen);
@@ -181,7 +184,7 @@
private CheckResult checkWebOfTrust(PGPPublicKey key, PublicKeyStore store,
int depth, Set<Fingerprint> seen) {
- if (trusted == null || store == null) {
+ if (trusted == null) {
// Trust checking not configured, server trusts all OK keys.
return CheckResult.trusted();
}
@@ -222,8 +225,7 @@
}
String subpacketProblem = checkTrustSubpacket(sig, depth);
if (subpacketProblem == null) {
- CheckResult signerResult =
- check(signer, store, depth + 1, false, seen);
+ CheckResult signerResult = check(signer, depth + 1, false, seen);
if (signerResult.isTrusted()) {
return CheckResult.trusted();
}
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java
index 71068b8..1236aa2 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java
@@ -202,7 +202,7 @@
CheckResult.bad("Signature by " + keyIdToString(sig.getKeyID())
+ " is not valid"));
}
- CheckResult result = publicKeyChecker.check(signer, store);
+ CheckResult result = publicKeyChecker.check(signer);
if (!result.getProblems().isEmpty()) {
StringBuilder err = new StringBuilder("Invalid public key ")
.append(keyToString(signer))
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java
index 900bcaf..63d4fe8 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java
@@ -163,7 +163,7 @@
found = true;
GpgKeyInfo info = toJson(
keyRing.getPublicKey(),
- checkerFactory.create(rsrc.getUser()),
+ checkerFactory.create().setExpectedUser(rsrc.getUser()),
store);
keys.put(info.id, info);
info.id = null;
@@ -197,7 +197,7 @@
try (PublicKeyStore store = storeProvider.get()) {
return toJson(
rsrc.getKeyRing().getPublicKey(),
- checkerFactory.create(rsrc.getUser()),
+ checkerFactory.create().setExpectedUser(rsrc.getUser()),
store);
}
}
@@ -261,7 +261,7 @@
static GpgKeyInfo toJson(PGPPublicKey key, PublicKeyChecker checker,
PublicKeyStore store) throws IOException {
- return toJson(key, checker.check(key, store));
+ return toJson(key, checker.setStore(store).check(key));
}
public static void toJson(GpgKeyInfo info, CheckResult checkResult) {
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
index 9b18ea2..0ed1f7b 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
@@ -36,6 +36,7 @@
import com.google.gerrit.gpg.CheckResult;
import com.google.gerrit.gpg.Fingerprint;
import com.google.gerrit.gpg.GerritPublicKeyChecker;
+import com.google.gerrit.gpg.PublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyStore;
import com.google.gerrit.gpg.server.PostGpgKeys.Input;
import com.google.gerrit.reviewdb.client.AccountExternalId;
@@ -193,7 +194,10 @@
for (PGPPublicKeyRing keyRing : keyRings) {
PGPPublicKey key = keyRing.getPublicKey();
// Don't check web of trust; admins can fill in certifications later.
- CheckResult result = checkerFactory.create(rsrc.getUser()).check(key);
+ CheckResult result = checkerFactory.create()
+ .setExpectedUser(rsrc.getUser())
+ .disableTrust()
+ .check(key);
if (!result.isOk()) {
throw new BadRequestException(String.format(
"Problems with public key %s:\n%s",
@@ -245,12 +249,14 @@
throws IOException {
// Unlike when storing keys, include web-of-trust checks when producing
// result JSON, so the user at least knows of any issues.
- GerritPublicKeyChecker checker = checkerFactory.create(user);
+ PublicKeyChecker checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .setStore(store);
Map<String, GpgKeyInfo> infos =
Maps.newHashMapWithExpectedSize(keys.size() + deleted.size());
for (PGPPublicKeyRing keyRing : keys) {
PGPPublicKey key = keyRing.getPublicKey();
- CheckResult result = checker.check(key, store);
+ CheckResult result = checker.check(key);
GpgKeyInfo info = GpgKeys.toJson(key, result);
infos.put(info.id, info);
info.id = null;
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
index ce0c912..edc0559 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
@@ -172,7 +172,9 @@
@Test
public void defaultGpgCertificationMatchesEmail() throws Exception {
TestKey key = validKeyWithSecondUserId();
- GerritPublicKeyChecker checker = checkerFactory.create(user);
+ PublicKeyChecker checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .disableTrust();
assertProblems(
checker.check(key.getPublicKey()), Status.BAD,
"Key must contain a valid certification for one of the following "
@@ -181,14 +183,18 @@
+ " username:user");
addExternalId("test", "test", "test5@example.com");
- checker = checkerFactory.create(user);
+ checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .disableTrust();
assertNoProblems(checker.check(key.getPublicKey()));
}
@Test
public void defaultGpgCertificationDoesNotMatchEmail() throws Exception {
addExternalId("test", "test", "nobody@example.com");
- GerritPublicKeyChecker checker = checkerFactory.create(user);
+ PublicKeyChecker checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .disableTrust();
assertProblems(
checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD,
"Key must contain a valid certification for one of the following "
@@ -202,14 +208,18 @@
@Test
public void manualCertificationMatchesExternalId() throws Exception {
addExternalId("foo", "myId", null);
- GerritPublicKeyChecker checker = checkerFactory.create(user);
+ PublicKeyChecker checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .disableTrust();
assertNoProblems(checker.check(validKeyWithSecondUserId().getPublicKey()));
}
@Test
public void manualCertificationDoesNotMatchExternalId() throws Exception {
addExternalId("foo", "otherId", null);
- GerritPublicKeyChecker checker = checkerFactory.create(user);
+ PublicKeyChecker checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .disableTrust();
assertProblems(
checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD,
"Key must contain a valid certification for one of the following "
@@ -226,13 +236,16 @@
reloadUser();
TestKey key = validKeyWithSecondUserId();
- GerritPublicKeyChecker checker = checkerFactory.create(user);
+ PublicKeyChecker checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .disableTrust();
assertProblems(
checker.check(key.getPublicKey()), Status.BAD,
"No identities found for user; check"
+ " http://test/#/settings/web-identities");
- checker = checkerFactory.create();
+ checker = checkerFactory.create()
+ .disableTrust();
assertProblems(
checker.check(key.getPublicKey()), Status.BAD,
"Key is not associated with any users");
@@ -264,14 +277,18 @@
add(keyE(), addUser("userE"));
// Checker for A, checking A.
- GerritPublicKeyChecker checkerA = checkerFactory.create(user);
- assertNoProblems(checkerA.check(keyA.getPublicKey(), store));
+ PublicKeyChecker checkerA = checkerFactory.create()
+ .setExpectedUser(user)
+ .setStore(store);
+ assertNoProblems(checkerA.check(keyA.getPublicKey()));
// Checker for B, checking B. Trust chain and IDs are correct, so the only
// problem is with the key itself.
- GerritPublicKeyChecker checkerB = checkerFactory.create(userB);
+ PublicKeyChecker checkerB = checkerFactory.create()
+ .setExpectedUser(userB)
+ .setStore(store);
assertProblems(
- checkerB.check(keyB.getPublicKey(), store), Status.BAD,
+ checkerB.check(keyB.getPublicKey()), Status.BAD,
"Key is expired");
}
@@ -294,9 +311,11 @@
add(keyE(), addUser("userE"));
// Checker for A, checking B.
- GerritPublicKeyChecker checkerA = checkerFactory.create(user);
+ PublicKeyChecker checkerA = checkerFactory.create()
+ .setExpectedUser(user)
+ .setStore(store);
assertProblems(
- checkerA.check(keyB.getPublicKey(), store), Status.BAD,
+ checkerA.check(keyB.getPublicKey()), Status.BAD,
"Key is expired",
"Key must contain a valid certification for one of the following"
+ " identities:\n"
@@ -306,9 +325,11 @@
+ " username:user");
// Checker for B, checking A.
- GerritPublicKeyChecker checkerB = checkerFactory.create(userB);
+ PublicKeyChecker checkerB = checkerFactory.create()
+ .setExpectedUser(userB)
+ .setStore(store);
assertProblems(
- checkerB.check(keyA.getPublicKey(), store), Status.BAD,
+ checkerB.check(keyA.getPublicKey()), Status.BAD,
"Key must contain a valid certification for one of the following"
+ " identities:\n"
+ " gerrit:userB\n"
@@ -325,9 +346,11 @@
TestKey keyA = add(keyA(), user);
TestKey keyB = add(keyB(), addUser("userB"));
- GerritPublicKeyChecker checker = checkerFactory.create(user);
+ PublicKeyChecker checker = checkerFactory.create()
+ .setExpectedUser(user)
+ .setStore(store);
assertProblems(
- checker.check(keyA.getPublicKey(), store), Status.OK,
+ checker.check(keyA.getPublicKey()), Status.OK,
"No path to a trusted key",
"Certification by " + keyToString(keyB.getPublicKey())
+ " is valid, but key is not trusted",
@@ -352,15 +375,16 @@
// This checker can check any key, so the only problems come from issues
// with the keys themselves, not having invalid user IDs.
- GerritPublicKeyChecker checker = checkerFactory.create();
- assertNoProblems(checker.check(keyA.getPublicKey(), store));
+ PublicKeyChecker checker = checkerFactory.create()
+ .setStore(store);
+ assertNoProblems(checker.check(keyA.getPublicKey()));
assertProblems(
- checker.check(keyB.getPublicKey(), store), Status.BAD,
+ checker.check(keyB.getPublicKey()), Status.BAD,
"Key is expired");
- assertNoProblems(checker.check(keyC.getPublicKey(), store));
- assertNoProblems(checker.check(keyD.getPublicKey(), store));
+ assertNoProblems(checker.check(keyC.getPublicKey()));
+ assertNoProblems(checker.check(keyD.getPublicKey()));
assertProblems(
- checker.check(keyE.getPublicKey(), store), Status.BAD,
+ checker.check(keyE.getPublicKey()), Status.BAD,
"Key is expired",
"No path to a trusted key");
}
@@ -382,8 +406,10 @@
keyRingB = PGPPublicKeyRing.insertPublicKey(keyRingB, keyB);
add(keyRingB, addUser("userB"));
- GerritPublicKeyChecker checkerA = checkerFactory.create(user);
- assertProblems(checkerA.check(keyA.getPublicKey(), store), Status.OK,
+ PublicKeyChecker checkerA = checkerFactory.create()
+ .setExpectedUser(user)
+ .setStore(store);
+ assertProblems(checkerA.check(keyA.getPublicKey()), Status.OK,
"No path to a trusted key",
"Certification by " + keyToString(keyB)
+ " is valid, but key is not trusted",
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
index 5c47cfe..6f69b38 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
@@ -44,9 +44,9 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;
-import java.util.ArrayList;
import java.util.Arrays;
-import java.util.List;
+import java.util.HashMap;
+import java.util.Map;
public class PublicKeyCheckerTest {
@Rule
@@ -175,11 +175,14 @@
}
private PublicKeyChecker newChecker(int maxTrustDepth, TestKey... trusted) {
- List<Fingerprint> fps = new ArrayList<>(trusted.length);
+ Map<Long, Fingerprint> fps = new HashMap<>();
for (TestKey k : trusted) {
- fps.add(new Fingerprint(k.getPublicKey().getFingerprint()));
+ Fingerprint fp = new Fingerprint(k.getPublicKey().getFingerprint());
+ fps.put(fp.getId(), fp);
}
- return new PublicKeyChecker(maxTrustDepth, fps);
+ return new PublicKeyChecker()
+ .enableTrust(maxTrustDepth, fps)
+ .setStore(store);
}
private TestKey add(TestKey k) {
@@ -205,12 +208,12 @@
private void assertProblems(PublicKeyChecker checker, TestKey k,
String... expected) {
- CheckResult result = checker.check(k.getPublicKey(), store);
+ CheckResult result = checker.check(k.getPublicKey());
assertEquals(Arrays.asList(expected), result.getProblems());
}
private void assertProblems(TestKey tk, String... expected) throws Exception {
- CheckResult result = new PublicKeyChecker().check(tk.getPublicKey(), store);
+ CheckResult result = new PublicKeyChecker().check(tk.getPublicKey());
assertEquals(Arrays.asList(expected), result.getProblems());
}
