From 338b8edb975f55ad51a7af0393357489386a361c Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Tue, 29 Jul 2025 16:54:58 -0400 Subject: [PATCH] Make the contacts prohibited feature flag for min reg data set more lenient (#2787) 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 --- .../flows/domain/DomainFlowUtils.java | 47 +++++++++++++++++-- .../flows/domain/DomainUpdateFlow.java | 13 +++-- .../flows/domain/DomainUpdateFlowTest.java | 38 +++++++++++++-- 3 files changed, 85 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java index 0740dda5e..d72bce3c6 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java @@ -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> registrant, Set 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> existingRegistrant, + Optional> newRegistrant, + Set existingContacts, + Set 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 registrantContactId) throws RegistrantNotAllowedException { ImmutableSet 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 hostNames = command.getNameserverHostNames(); validateNameserversCountForTld(tldStr, domainName, hostNames.size()); validateNameserversAllowedOnTld(tldStr, hostNames); 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 c7b3600a4..2c475bdae 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -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(), 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 ed6618942..dfd080eea 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java @@ -346,18 +346,18 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase