diff --git a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java index 890fe8676..e27127a5d 100644 --- a/core/src/main/java/google/registry/flows/ResourceFlowUtils.java +++ b/core/src/main/java/google/registry/flows/ResourceFlowUtils.java @@ -195,14 +195,31 @@ public final class ResourceFlowUtils { } /** Check that the same values aren't being added and removed in an update command. */ - public static void checkSameValuesNotAddedAndRemoved( - ImmutableSet fieldsToAdd, ImmutableSet fieldsToRemove) + public static void checkSameValuesNotAddedAndRemoved( + ImmutableSet fieldsToAdd, ImmutableSet fieldsToRemove) throws AddRemoveSameValueException { if (!intersection(fieldsToAdd, fieldsToRemove).isEmpty()) { throw new AddRemoveSameValueException(); } } + /** Check that we aren't adding a value that is already present. */ + public static void checkExistingValueNotAdded( + ImmutableSet fieldsToAdd, ImmutableSet existingFields) + throws AddExistingValueException { + if (!intersection(fieldsToAdd, existingFields).isEmpty()) { + throw new AddExistingValueException(); + } + } + + public static void checkNonexistentValueNotRemoved( + ImmutableSet fieldsToRemove, ImmutableSet existingFields) + throws RemoveNonexistentValueException { + if (intersection(fieldsToRemove, existingFields).size() != fieldsToRemove.size()) { + throw new RemoveNonexistentValueException(); + } + } + /** Check that all {@link StatusValue} objects in a set are client-settable. */ public static void verifyAllStatusesAreClientSettable(Set statusValues) throws StatusNotClientSettableException { @@ -266,6 +283,20 @@ public final class ResourceFlowUtils { } } + /** Cannot add a value that is already present. */ + public static class AddExistingValueException extends ParameterValuePolicyErrorException { + public AddExistingValueException() { + super("Cannot add a value that is already present"); + } + } + + /** Cannot remove a value that does not exist. */ + public static class RemoveNonexistentValueException extends ParameterValuePolicyErrorException { + public RemoveNonexistentValueException() { + super("Cannot remove a value that does not exist"); + } + } + /** The specified status value cannot be set by clients. */ public static class StatusNotClientSettableException extends ParameterValueRangeErrorException { public StatusNotClientSettableException(String statusValue) { diff --git a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java index d4b541c70..886d6aba7 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -21,6 +21,8 @@ import static com.google.common.collect.Sets.union; import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.flows.FlowUtils.persistEntityChanges; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; +import static google.registry.flows.ResourceFlowUtils.checkExistingValueNotAdded; +import static google.registry.flows.ResourceFlowUtils.checkNonexistentValueNotRemoved; import static google.registry.flows.ResourceFlowUtils.checkSameValuesNotAddedAndRemoved; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyAllStatusesAreClientSettable; @@ -248,6 +250,8 @@ public final class DomainUpdateFlow implements MutatingFlow { checkSameValuesNotAddedAndRemoved(add.getNameservers(), remove.getNameservers()); checkSameValuesNotAddedAndRemoved(add.getContacts(), remove.getContacts()); checkSameValuesNotAddedAndRemoved(add.getStatusValues(), remove.getStatusValues()); + checkExistingValueNotAdded(add.getNameservers(), domain.getNameservers()); + checkNonexistentValueNotRemoved(remove.getNameservers(), domain.getNameservers()); Change change = command.getInnerChange(); Optional secDnsUpdate = eppInput.getSingleExtension(SecDnsUpdateExtension.class); diff --git a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java index 0c1fc51be..a0d2c79dd 100644 --- a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java @@ -20,6 +20,8 @@ import static google.registry.dns.DnsUtils.requestHostDnsRefresh; import static google.registry.dns.RefreshDnsOnHostRenameAction.PARAM_HOST_KEY; import static google.registry.dns.RefreshDnsOnHostRenameAction.QUEUE_HOST_RENAME; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; +import static google.registry.flows.ResourceFlowUtils.checkExistingValueNotAdded; +import static google.registry.flows.ResourceFlowUtils.checkNonexistentValueNotRemoved; import static google.registry.flows.ResourceFlowUtils.checkSameValuesNotAddedAndRemoved; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyAllStatusesAreClientSettable; @@ -161,6 +163,8 @@ public final class HostUpdateFlow implements MutatingFlow { AddRemove remove = command.getInnerRemove(); checkSameValuesNotAddedAndRemoved(add.getStatusValues(), remove.getStatusValues()); checkSameValuesNotAddedAndRemoved(add.getInetAddresses(), remove.getInetAddresses()); + checkExistingValueNotAdded(add.getInetAddresses(), existingHost.getInetAddresses()); + checkNonexistentValueNotRemoved(remove.getInetAddresses(), existingHost.getInetAddresses()); HostFlowUtils.validateInetAddresses(add.getInetAddresses()); VKey newSuperordinateDomainKey = newSuperordinateDomain.map(Domain::createVKey).orElse(null); diff --git a/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java index fb073a0c6..e60ef8da8 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java @@ -67,7 +67,9 @@ import google.registry.flows.FlowTestCase.CommitMode; import google.registry.flows.FlowTestCase.UserPrivileges; import google.registry.flows.FlowUtils.NotLoggedInException; import google.registry.flows.ResourceFlowTestCase; +import google.registry.flows.ResourceFlowUtils.AddExistingValueException; import google.registry.flows.ResourceFlowUtils.AddRemoveSameValueException; +import google.registry.flows.ResourceFlowUtils.RemoveNonexistentValueException; import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException; import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException; import google.registry.flows.ResourceFlowUtils.StatusNotClientSettableException; @@ -1373,6 +1375,30 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase { .asBuilder() .setSuperordinateDomain(foo.createVKey()) .setLastTransferTime(null) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) .build()); persistResource(foo.asBuilder().setSubordinateHosts(ImmutableSet.of(oldHostName())).build()); clock.advanceOneMilli(); @@ -561,6 +565,8 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .setSuperordinateDomain(domain.createVKey()) .setLastTransferTime(clock.nowUtc().minusDays(20)) .setLastSuperordinateChange(clock.nowUtc().minusDays(3)) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) .build()); DateTime lastTransferTime = host.getLastTransferTime(); persistResource(domain.asBuilder().setSubordinateHosts(ImmutableSet.of(oldHostName())).build()); @@ -598,6 +604,8 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .setSuperordinateDomain(foo.createVKey()) .setLastTransferTime(lastTransferTime) .setLastSuperordinateChange(clock.nowUtc().minusDays(3)) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) .build()); persistResource(foo.asBuilder().setSubordinateHosts(ImmutableSet.of(oldHostName())).build()); clock.advanceOneMilli(); @@ -632,6 +640,8 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .setSuperordinateDomain(foo.createVKey()) .setLastTransferTime(lastTransferTime) .setLastSuperordinateChange(clock.nowUtc().minusDays(10)) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) .build()); persistResource(foo.asBuilder().setSubordinateHosts(ImmutableSet.of(oldHostName())).build()); clock.advanceOneMilli(); @@ -666,6 +676,8 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .setSuperordinateDomain(foo.createVKey()) .setLastTransferTime(null) .setLastSuperordinateChange(clock.nowUtc().minusDays(3)) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) .build()); persistResource(foo.asBuilder().setSubordinateHosts(ImmutableSet.of(oldHostName())).build()); Host renamedHost = doSuccessfulTest(); @@ -687,7 +699,12 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { createTld("foo"); Domain domain = persistActiveDomain("example.foo"); persistResource( - newHost(oldHostName()).asBuilder().setSuperordinateDomain(domain.createVKey()).build()); + newHost(oldHostName()) + .asBuilder() + .setSuperordinateDomain(domain.createVKey()) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) + .build()); DateTime lastTransferTime = clock.nowUtc().minusDays(2); persistResource( domain @@ -728,6 +745,8 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .setSuperordinateDomain(domain.createVKey()) .setLastTransferTime(lastTransferTime) .setLastSuperordinateChange(clock.nowUtc().minusDays(4)) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) .build()); persistResource( domain @@ -763,6 +782,8 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .setSuperordinateDomain(domain.createVKey()) .setLastTransferTime(clock.nowUtc().minusDays(12)) .setLastSuperordinateChange(clock.nowUtc().minusDays(4)) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) .build()); domain = persistResource( @@ -1007,6 +1028,50 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { assertAboutEppExceptions().that(thrown).marshalsToXml(); } + @Test + void testFailure_addExistingInetAddress() throws Exception { + createTld("tld"); + Domain domain = persistActiveDomain("example.tld"); + persistResource( + newHost(oldHostName()) + .asBuilder() + .setSuperordinateDomain(domain.createVKey()) + .setLastTransferTime(clock.nowUtc().minusDays(12)) + .setLastSuperordinateChange(clock.nowUtc().minusDays(4)) + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) + .build()); + setEppHostUpdateInput( + "ns1.example.tld", + "ns2.example.tld", + "1080:0:0:0:8:800:200C:417A", + null); + assertAboutEppExceptions() + .that(assertThrows(AddExistingValueException.class, this::runFlow)) + .marshalsToXml(); + } + + @Test + void testFailure_removeNonexistentInetAddress() throws Exception { + createTld("tld"); + Domain domain = persistActiveDomain("example.tld"); + persistResource( + newHost(oldHostName()) + .asBuilder() + .setSuperordinateDomain(domain.createVKey()) + .setLastTransferTime(clock.nowUtc().minusDays(12)) + .setLastSuperordinateChange(clock.nowUtc().minusDays(4)) + .build()); + setEppHostUpdateInput( + "ns1.example.tld", + "ns2.example.tld", + null, + "1080:0:0:0:8:800:200C:417A"); + assertAboutEppExceptions() + .that(assertThrows(RemoveNonexistentValueException.class, this::runFlow)) + .marshalsToXml(); + } + @Test void testSuccess_clientUpdateProhibited_removed() throws Exception { setEppInput("host_update_remove_client_update_prohibited.xml"); @@ -1078,7 +1143,12 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { setEppInput("host_update_prohibited_status.xml"); createTld("tld"); persistActiveDomain("example.tld"); - persistActiveHost("ns1.example.tld"); + persistResource( + persistActiveHost("ns1.example.tld") + .asBuilder() + .setInetAddresses( + ImmutableSet.of(InetAddresses.forString("1080:0:0:0:8:800:200C:417A"))) + .build()); clock.advanceOneMilli(); runFlowAssertResponse(