mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-02-09 06:20:29 +00:00
Code Issues Releases Wiki Activity

Forbid no-op domain-NS and host-IP adds/removes (#2928)

Browse Source 
The RST testing expects us to fail if they try to remove an IP from a
host that already doesn't that have that IP, or to add one that already
exists (ditto on both for a domain's nameservers). I don't really see an
issue with our previous no-op implementation, but we need to do this to
pass the tests.
This commit is contained in:
gbrodman
2026-01-09 12:55:12 -05:00
committed by GitHub
parent 64f6cd9af4
commit 69e5d40f04
5 changed files with 139 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
core/src/main/java/google/registry/flows/ResourceFlowUtils.java
View File

@@ -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 <T> void checkSameValuesNotAddedAndRemoved(
ImmutableSet<T> fieldsToAdd, ImmutableSet<T> 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 <T> void checkExistingValueNotAdded(
ImmutableSet<T> fieldsToAdd, ImmutableSet<T> existingFields)
throws AddExistingValueException {
if (!intersection(fieldsToAdd, existingFields).isEmpty()) {
throw new AddExistingValueException();
}
}
public static <T> void checkNonexistentValueNotRemoved(
ImmutableSet<T> fieldsToRemove, ImmutableSet<T> 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<StatusValue> 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) {

4
core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java
View File

@@ -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<SecDnsUpdateExtension> secDnsUpdate =
eppInput.getSingleExtension(SecDnsUpdateExtension.class);

4
core/src/main/java/google/registry/flows/host/HostUpdateFlow.java
View File

@@ -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<Domain> newSuperordinateDomainKey =
newSuperordinateDomain.map(Domain::createVKey).orElse(null);

26
core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java
View File

@@ -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<DomainUpdateFlow, Domain
assertAboutEppExceptions().that(thrown).marshalsToXml();
}
@Test
void testFailure_addExistingNameserver() throws Exception {
Host host = persistActiveHost("ns2.example.foo");
persistResource(
DatabaseHelper.newDomain(getUniqueIdFromCommand())
.asBuilder()
.setNameservers(ImmutableSet.of(host.createVKey()))
.build());
setEppInput("domain_update_add_nameserver.xml");
assertAboutEppExceptions()
.that(assertThrows(AddExistingValueException.class, this::runFlow))
.marshalsToXml();
}
@Test
void testFailure_removeNonexistentValue() throws Exception {
persistActiveDomain(getUniqueIdFromCommand());
persistActiveHost("ns1.example.foo");
setEppInput("domain_update_remove_nameserver.xml");
assertAboutEppExceptions()
.that(assertThrows(RemoveNonexistentValueException.class, this::runFlow))
.marshalsToXml();
}
@Test
void testFailure_minimumDataset_addingNewRegistrantFails() throws Exception {
persistReferencedEntities();

74
core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java
View File

@@ -48,7 +48,9 @@ import google.registry.flows.EppException;
import google.registry.flows.EppRequestSource;
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;
@@ -522,6 +524,8 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Host> {
.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<HostUpdateFlow, Host> {
.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<HostUpdateFlow, Host> {
.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<HostUpdateFlow, Host> {
.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<HostUpdateFlow, Host> {
.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<HostUpdateFlow, Host> {
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<HostUpdateFlow, Host> {
.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<HostUpdateFlow, Host> {
.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<HostUpdateFlow, Host> {
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",
"<host:addr ip=\"v6\">1080:0:0:0:8:800:200C:417A</host:addr>",
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,
"<host:addr ip=\"v6\">1080:0:0:0:8:800:200C:417A</host:addr>");
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<HostUpdateFlow, Host> {
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(