From ce4f3c0d569937fff607ba72294853a568b1ce70 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Tue, 7 Mar 2017 11:28:41 -0800 Subject: [PATCH] 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 --- .../registry/model/registry/Registry.java | 48 +++++++++++++++---- .../registry/model/registry/RegistryTest.java | 34 ++++++++++++- .../registry/tools/CreateTldCommandTest.java | 2 +- .../registry/tools/UpdateTldCommandTest.java | 2 +- 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/java/google/registry/model/registry/Registry.java b/java/google/registry/model/registry/Registry.java index ce1b21316..200bc3381 100644 --- a/java/google/registry/model/registry/Registry.java +++ b/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 reservedListNames) { checkArgument(reservedListNames != null, "reservedListNames must not be null"); - ImmutableSet.Builder> builder = new ImmutableSet.Builder<>(); + ImmutableSet.Builder 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.get(reservedListName); - if (!reservedList.isPresent()) { - throw new IllegalStateException( - "Could not find reserved list " + reservedListName + " to add to the tld"); - } - builder.add(Key.create(reservedList.get())); + checkArgument( + reservedList.isPresent(), + "Could not find reserved list %s to add to the tld", + reservedListName); + builder.add(reservedList.get()); } - getInstance().reservedLists = builder.build(); - return this; + return setReservedLists(builder.build()); } public Builder setReservedLists(ReservedList... reservedLists) { @@ -743,6 +748,7 @@ public class Registry extends ImmutableObject implements Buildable { public Builder setReservedLists(Set reservedLists) { checkArgumentNotNull(reservedLists, "reservedLists must not be null"); + checkAuthCodeConflicts(reservedLists); ImmutableSet.Builder> 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 reservedLists) { + Multimap allAuthCodes = HashMultimap.create(); + for (ReservedList list : reservedLists) { + for (ReservedListEntry entry : list.getReservedListEntries().values()) { + if (entry.getAuthCode() != null) { + allAuthCodes.put(entry.getLabel(), entry.getAuthCode()); + } + } + } + ImmutableSet>> conflicts = + FluentIterable.from(allAuthCodes.asMap().entrySet()) + .filter(new Predicate>>() { + @Override + public boolean apply(Entry> 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; diff --git a/javatests/google/registry/model/registry/RegistryTest.java b/javatests/google/registry/model/registry/RegistryTest.java index cb60e31dc..c3190c8be 100644 --- a/javatests/google/registry/model/registry/RegistryTest.java +++ b/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 - public final ExceptionRule thrown = new ExceptionRule(); + @Rule 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"); diff --git a/javatests/google/registry/tools/CreateTldCommandTest.java b/javatests/google/registry/tools/CreateTldCommandTest.java index 1c45469c8..8a5e72329 100644 --- a/javatests/google/registry/tools/CreateTldCommandTest.java +++ b/javatests/google/registry/tools/CreateTldCommandTest.java @@ -388,7 +388,7 @@ public class CreateTldCommandTest extends CommandTestCase { 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"); } diff --git a/javatests/google/registry/tools/UpdateTldCommandTest.java b/javatests/google/registry/tools/UpdateTldCommandTest.java index 26257e765..04814716f 100644 --- a/javatests/google/registry/tools/UpdateTldCommandTest.java +++ b/javatests/google/registry/tools/UpdateTldCommandTest.java @@ -606,7 +606,7 @@ public class UpdateTldCommandTest extends CommandTestCase { @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"); }