Don't allow setting reserved lists with conflicting auth codes

This is an error condition that will soon throw an exception when
attempting to register the domain name, so it's good to let the registry
operator know of the error when it is first introduced.

Unfortunately there's still a backdoor that allows duplicate labels
that's harder to protect against (that this commit doesn't cover): the
case where reserved lists are already applied to a TLD, then one of the
reserved lists is updated to add another auth code, which then conflicts
with one on a different reserved list.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=149443007

java/google/registry/model/registry/Registry.java

@@ -35,9 +35,12 @@ import com.google.common.base.Predicate;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
 import com.google.common.collect.Ordering;
 import com.google.common.collect.Range;
 import com.google.common.net.InternetDomainName;
@@ -61,7 +64,10 @@ import google.registry.model.domain.fee.BaseFee.FeeType;
 import google.registry.model.domain.fee.Fee;
 import google.registry.model.registry.label.PremiumList;
 import google.registry.model.registry.label.ReservedList;
+import google.registry.model.registry.label.ReservedList.ReservedListEntry;
 import google.registry.util.Idn;
+import java.util.Collection;
+import java.util.Map.Entry;
 import java.util.Set;
 import javax.annotation.Nullable;
 import org.joda.money.CurrencyUnit;
@@ -723,18 +729,17 @@ public class Registry extends ImmutableObject implements Buildable {
 
     public Builder setReservedListsByName(Set<String> reservedListNames) {
       checkArgument(reservedListNames != null, "reservedListNames must not be null");
-      ImmutableSet.Builder<Key<ReservedList>> builder = new ImmutableSet.Builder<>();
+      ImmutableSet.Builder<ReservedList> builder = new ImmutableSet.Builder<>();
       for (String reservedListName : reservedListNames) {
         // Check for existence of the reserved list and throw an exception if it doesn't exist.
         Optional<ReservedList> reservedList = ReservedList.get(reservedListName);
-        if (!reservedList.isPresent()) {
+        checkArgument(
-          throw new IllegalStateException(
+            reservedList.isPresent(),
-              "Could not find reserved list " + reservedListName + " to add to the tld");
+            "Could not find reserved list %s to add to the tld",
+            reservedListName);
+        builder.add(reservedList.get());
       }
-        builder.add(Key.create(reservedList.get()));
+      return setReservedLists(builder.build());
-      }
-      getInstance().reservedLists = builder.build();
-      return this;
     }
 
     public Builder setReservedLists(ReservedList... reservedLists) {
@@ -743,6 +748,7 @@ public class Registry extends ImmutableObject implements Buildable {
 
     public Builder setReservedLists(Set<ReservedList> reservedLists) {
       checkArgumentNotNull(reservedLists, "reservedLists must not be null");
+      checkAuthCodeConflicts(reservedLists);
       ImmutableSet.Builder<Key<ReservedList>> builder = new ImmutableSet.Builder<>();
       for (ReservedList reservedList : reservedLists) {
         builder.add(Key.create(reservedList));
@@ -751,6 +757,32 @@ public class Registry extends ImmutableObject implements Buildable {
       return this;
     }
 
+    /**
+     * Checks that domain names don't have conflicting auth codes across different reserved lists.
+     */
+    private static void checkAuthCodeConflicts(Set<ReservedList> reservedLists) {
+      Multimap<String, String> allAuthCodes = HashMultimap.create();
+      for (ReservedList list : reservedLists) {
+        for (ReservedListEntry entry : list.getReservedListEntries().values()) {
+          if (entry.getAuthCode() != null) {
+            allAuthCodes.put(entry.getLabel(), entry.getAuthCode());
+          }
+        }
+      }
+      ImmutableSet<Entry<String, Collection<String>>> conflicts =
+          FluentIterable.from(allAuthCodes.asMap().entrySet())
+              .filter(new Predicate<Entry<String, Collection<String>>>() {
+                @Override
+                public boolean apply(Entry<String, Collection<String>> entry) {
+                  return entry.getValue().size() > 1;
+                }})
+              .toSet();
+      checkArgument(
+          conflicts.isEmpty(),
+          "Cannot set reserved lists because of auth code conflicts for labels: %s",
+          conflicts);
+    }
+
     public Builder setPremiumList(PremiumList premiumList) {
       getInstance().premiumList = (premiumList == null) ? null : Key.create(premiumList);
       return this;

javatests/google/registry/model/registry/RegistryTest.java

@@ -48,8 +48,7 @@ import org.junit.Test;
 /** Unit tests for {@link Registry}. */
 public class RegistryTest extends EntityTestCase {
 
-  @Rule
+  @Rule public final ExceptionRule thrown = new ExceptionRule();
-  public final ExceptionRule thrown = new ExceptionRule();
 
   Registry registry;
 
@@ -173,6 +172,37 @@ public class RegistryTest extends EntityTestCase {
     assertThat(r.getReservedLists()).isEmpty();
   }
 
+  @Test
+  public void testSetReservedLists_succeedsWithDuplicateIdenticalAuthCodes() {
+    ReservedList rl1 = persistReservedList(
+        "tld-reserved007",
+        "lol,RESERVED_FOR_ANCHOR_TENANT,identical",
+        "cat,FULLY_BLOCKED");
+    ReservedList rl2 = persistReservedList(
+        "tld-reserved008",
+        "lol,RESERVED_FOR_ANCHOR_TENANT,identical",
+        "tim,FULLY_BLOCKED");
+    Registry registry = Registry.get("tld").asBuilder().setReservedLists(rl1, rl2).build();
+    assertThat(registry.getReservedLists()).containsExactly(Key.create(rl1), Key.create(rl2));
+  }
+
+  @Test
+  public void testSetReservedLists_failsForConflictingAuthCodes() {
+    ReservedList rl1 = persistReservedList(
+        "tld-reserved055",
+        "lol,RESERVED_FOR_ANCHOR_TENANT,conflict1",
+        "cat,FULLY_BLOCKED");
+    ReservedList rl2 = persistReservedList(
+        "tld-reserved056",
+        "lol,RESERVED_FOR_ANCHOR_TENANT,conflict2",
+        "tim,FULLY_BLOCKED");
+    thrown.expect(
+        IllegalArgumentException.class,
+        "auth code conflicts for labels: [lol=[conflict1, conflict2]]");
+    @SuppressWarnings("unused")
+    Registry unused = Registry.get("tld").asBuilder().setReservedLists(rl1, rl2).build();
+  }
+
   @Test
   public void testSetPremiumList() {
     PremiumList pl2 = persistPremiumList("tld2", "lol,USD 50", "cat,USD 700");

javatests/google/registry/tools/CreateTldCommandTest.java

@@ -388,7 +388,7 @@ public class CreateTldCommandTest extends CommandTestCase<CreateTldCommand> {
   public void testFailure_setNonExistentReservedLists() throws Exception {
     runFailureReservedListsTest(
         "xn--q9jyb4c_asdf,common_asdsdgh",
-        IllegalStateException.class,
+        IllegalArgumentException.class,
         "Could not find reserved list xn--q9jyb4c_asdf to add to the tld");
   }
 

javatests/google/registry/tools/UpdateTldCommandTest.java

@@ -606,7 +606,7 @@ public class UpdateTldCommandTest extends CommandTestCase<UpdateTldCommand> {
   @Test
   public void testFailure_setNonExistentReservedLists() throws Exception {
     thrown.expect(
-        IllegalStateException.class,
+        IllegalArgumentException.class,
         "Could not find reserved list xn--q9jyb4c_ZZZ to add to the tld");
     runCommandForced("--reserved_lists", "xn--q9jyb4c_ZZZ", "xn--q9jyb4c");
   }