mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2025-12-23 06:15:42 +00:00
Code Issues Releases Wiki Activity

Make the contacts prohibited feature flag for min reg data set more lenient (#2787)

Browse Source 
It will now only throw errors on domain updates if a new contact/registrant has
been specified where none was previously present. This means that domain updates
on unrelated fields (e.g. nameserver changes) will succeed even if there is
existing contact data that the update is not removing.

This is a follow-up to #2781.

BUG=http://b/434958659
This commit is contained in:
Ben McIlwain
2025-07-29 16:54:58 -04:00
committed by GitHub
parent 9f191e9392
commit 338b8edb97
3 changed files with 85 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java
View File

@@ -481,10 +481,10 @@ public class DomainFlowUtils {
}
/**
* Enforces the presence/absence of contact data depending on the minimum data set migration
* schedule.
* Enforces the presence/absence of contact data on domain creates depending on the minimum data
* set migration schedule.
*/
static void validateContactDataPresence(
static void validateCreateContactData(
Optional<VKey<Contact>> registrant, Set<DesignatedContact> contacts)
throws RequiredParameterMissingException, ParameterValuePolicyErrorException {
// TODO(b/353347632): Change these flag checks to a registry config check once minimum data set
@@ -514,6 +514,45 @@ public class DomainFlowUtils {
}
}
/**
* Enforces the presence/absence of contact data on domain updates depending on the minimum data
* set migration schedule.
*/
static void validateUpdateContactData(
Optional<VKey<Contact>> existingRegistrant,
Optional<VKey<Contact>> newRegistrant,
Set<DesignatedContact> existingContacts,
Set<DesignatedContact> newContacts)
throws RequiredParameterMissingException, ParameterValuePolicyErrorException {
// TODO(b/353347632): Change these flag checks to a registry config check once minimum data set
// migration is completed.
if (FeatureFlag.isActiveNow(MINIMUM_DATASET_CONTACTS_PROHIBITED)) {
// Throw if the update specifies a new registrant that is different from the existing one.
if (newRegistrant.isPresent() && !newRegistrant.equals(existingRegistrant)) {
throw new RegistrantProhibitedException();
}
// Throw if the update specifies any new contacts that weren't already present on the domain.
if (!Sets.difference(newContacts, existingContacts).isEmpty()) {
throw new ContactsProhibitedException();
}
} else if (!FeatureFlag.isActiveNow(MINIMUM_DATASET_CONTACTS_OPTIONAL)) {
// Throw if the update empties out a registrant that had been present.
if (newRegistrant.isEmpty() && existingRegistrant.isPresent()) {
throw new MissingRegistrantException();
}
// Throw if the update contains no admin contact when one had been present.
if (existingContacts.stream().anyMatch(c -> c.getType().equals(Type.ADMIN))
&& newContacts.stream().noneMatch(c -> c.getType().equals(Type.ADMIN))) {
throw new MissingAdminContactException();
}
// Throw if the update contains no tech contact when one had been present.
if (existingContacts.stream().anyMatch(c -> c.getType().equals(Type.TECH))
&& newContacts.stream().noneMatch(c -> c.getType().equals(Type.TECH))) {
throw new MissingTechnicalContactException();
}
}
}
static void validateRegistrantAllowedOnTld(String tld, Optional<String> registrantContactId)
throws RegistrantNotAllowedException {
ImmutableSet<String> allowedRegistrants = Tld.get(tld).getAllowedRegistrantContactIds();
@@ -1054,7 +1093,7 @@ public class DomainFlowUtils {
String tldStr = tld.getTldStr();
validateRegistrantAllowedOnTld(tldStr, command.getRegistrantContactId());
validateNoDuplicateContacts(command.getContacts());
validateContactDataPresence(command.getRegistrant(), command.getContacts());
validateCreateContactData(command.getRegistrant(), command.getContacts());
ImmutableSet<String> hostNames = command.getNameserverHostNames();
validateNameserversCountForTld(tldStr, domainName, hostNames.size());
validateNameserversAllowedOnTld(tldStr, hostNames);

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

@@ -30,7 +30,6 @@ import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
import static google.registry.flows.domain.DomainFlowUtils.checkAllowedAccessToTld;
import static google.registry.flows.domain.DomainFlowUtils.cloneAndLinkReferences;
import static google.registry.flows.domain.DomainFlowUtils.updateDsData;
import static google.registry.flows.domain.DomainFlowUtils.validateContactDataPresence;
import static google.registry.flows.domain.DomainFlowUtils.validateContactsHaveTypes;
import static google.registry.flows.domain.DomainFlowUtils.validateDsData;
import static google.registry.flows.domain.DomainFlowUtils.validateFeesAckedIfPresent;
@@ -38,6 +37,7 @@ import static google.registry.flows.domain.DomainFlowUtils.validateNameserversAl
import static google.registry.flows.domain.DomainFlowUtils.validateNameserversCountForTld;
import static google.registry.flows.domain.DomainFlowUtils.validateNoDuplicateContacts;
import static google.registry.flows.domain.DomainFlowUtils.validateRegistrantAllowedOnTld;
import static google.registry.flows.domain.DomainFlowUtils.validateUpdateContactData;
import static google.registry.flows.domain.DomainFlowUtils.verifyClientUpdateNotProhibited;
import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPendingDelete;
import static google.registry.model.common.FeatureFlag.FeatureName.MINIMUM_DATASET_CONTACTS_OPTIONAL;
@@ -186,7 +186,7 @@ public final class DomainUpdateFlow implements MutatingFlow {
Domain newDomain = performUpdate(command, existingDomain, now);
DomainHistory domainHistory =
historyBuilder.setType(DOMAIN_UPDATE).setDomain(newDomain).build();
validateNewState(newDomain);
validateNewState(existingDomain, newDomain);
if (requiresDnsUpdate(existingDomain, newDomain)) {
requestDomainDnsRefresh(targetId);
}
@@ -328,8 +328,13 @@ public final class DomainUpdateFlow implements MutatingFlow {
* compliant with the additions or amendments, otherwise existing data can become invalid and
* cause Domain update failure.
*/
private static void validateNewState(Domain newDomain) throws EppException {
validateContactDataPresence(newDomain.getRegistrant(), newDomain.getContacts());
private static void validateNewState(Domain existingDomain, Domain newDomain)
throws EppException {
validateUpdateContactData(
existingDomain.getRegistrant(),
newDomain.getRegistrant(),
existingDomain.getContacts(),
newDomain.getContacts());
validateDsData(newDomain.getDsData());
validateNameserversCountForTld(
newDomain.getTld(),

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

@@ -346,18 +346,18 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
}
@Test
void testFailure_minimumDatasetPhase2_nonRegistrantContactsStillExist() throws Exception {
void testFailure_minimumDatasetPhase2_whenAddingNewContacts() throws Exception {
persistResource(
new FeatureFlag.Builder()
.setFeatureName(MINIMUM_DATASET_CONTACTS_PROHIBITED)
.setStatusMap(
ImmutableSortedMap.of(START_OF_TIME, INACTIVE, clock.nowUtc().minusDays(5), ACTIVE))
.build());
// This EPP adds a new technical contact mak21 that wasn't already present.
setEppInput("domain_update_empty_registrant.xml");
persistReferencedEntities();
persistDomain();
// Fails because after the update the domain would still have some contacts on it even though
// the registrant has been removed.
// Fails because the update adds some new contacts, although the registrant has been removed.
ContactsProhibitedException thrown =
assertThrows(ContactsProhibitedException.class, this::runFlow);
assertAboutEppExceptions().that(thrown).marshalsToXml();
@@ -1574,14 +1574,13 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
}
@Test
void testFailure_minimumDatasetPhase2_registrantStillExists() throws Exception {
void testFailure_minimumDatasetPhase2_addingNewRegistrantFails() throws Exception {
persistResource(
new FeatureFlag.Builder()
.setFeatureName(MINIMUM_DATASET_CONTACTS_PROHIBITED)
.setStatusMap(
ImmutableSortedMap.of(START_OF_TIME, INACTIVE, clock.nowUtc().minusDays(5), ACTIVE))
.build());
setEppInput("domain_update_remove_admin.xml");
persistReferencedEntities();
persistResource(
DatabaseHelper.newDomain(getUniqueIdFromCommand())
@@ -1590,7 +1589,10 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
ImmutableSet.of(
DesignatedContact.create(Type.ADMIN, sh8013Contact.createVKey()),
DesignatedContact.create(Type.TECH, sh8013Contact.createVKey())))
.setRegistrant(Optional.empty())
.build());
// This EPP sets the registrant to sh8013, whereas in our test setup it is absent.
setEppInput("domain_update_registrant.xml");
RegistrantProhibitedException thrown =
assertThrows(RegistrantProhibitedException.class, this::runFlow);
assertAboutEppExceptions().that(thrown).marshalsToXml();
@@ -1657,6 +1659,32 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
assertThat(updatedDomain.getContacts()).isEmpty();
}
@Test
void testSuccess_minimumDatasetPhase2_removeOneContact() throws Exception {
persistResource(
new FeatureFlag.Builder()
.setFeatureName(MINIMUM_DATASET_CONTACTS_PROHIBITED)
.setStatusMap(
ImmutableSortedMap.of(START_OF_TIME, INACTIVE, clock.nowUtc().minusDays(5), ACTIVE))
.build());
setEppInput("domain_update_remove_admin.xml");
persistReferencedEntities();
persistResource(
DatabaseHelper.newDomain(getUniqueIdFromCommand())
.asBuilder()
.setContacts(
ImmutableSet.of(
DesignatedContact.create(Type.ADMIN, sh8013Contact.createVKey()),
DesignatedContact.create(Type.TECH, sh8013Contact.createVKey())))
.build());
assertThat(reloadResourceByForeignKey().getRegistrant()).isPresent();
assertThat(reloadResourceByForeignKey().getContacts()).hasSize(2);
runFlowAssertResponse(loadFile("generic_success_response.xml"));
Domain updatedDomain = reloadResourceByForeignKey();
assertThat(updatedDomain.getRegistrant()).isPresent();
assertThat(updatedDomain.getContacts()).hasSize(1);
}
@Test
void testFailure_addPendingDeleteContact() throws Exception {
persistReferencedEntities();