From c4e5bc913e7761fd6e13081bc891cc0fa011e2b2 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Thu, 18 Jul 2024 15:33:52 -0400 Subject: [PATCH] Remove contact entities from RDAP entirely when they don't exist in DB (#2497) This is consistent with how other registries are handling RDAP and is also consistent with overall behavior in WHOIS and domain info flows as implemented in my previous PRs #2477 and #2490. --- .../registry/rdap/RdapEntityAction.java | 2 +- .../registry/rdap/RdapEntitySearchAction.java | 2 +- .../registry/rdap/RdapJsonFormatter.java | 45 +--- .../registry/rdap/RdapDomainActionTest.java | 38 +-- .../rdap/RdapDomainSearchActionTest.java | 14 +- .../registry/rdap/RdapJsonFormatterTest.java | 23 +- ..._domain_no_contacts_exist_with_remark.json | 111 --------- ...rdap_domain_no_registrant_with_remark.json | 225 ++++++++++++++++++ ...domain_redacted_contacts_with_remark.json} | 0 9 files changed, 274 insertions(+), 186 deletions(-) create mode 100644 core/src/test/resources/google/registry/rdap/rdap_domain_no_registrant_with_remark.json rename core/src/test/resources/google/registry/rdap/{rdap_domain_no_contacts_with_remark.json => rdap_domain_redacted_contacts_with_remark.json} (100%) diff --git a/core/src/main/java/google/registry/rdap/RdapEntityAction.java b/core/src/main/java/google/registry/rdap/RdapEntityAction.java index 2a24ca90b..f4c24adb6 100644 --- a/core/src/main/java/google/registry/rdap/RdapEntityAction.java +++ b/core/src/main/java/google/registry/rdap/RdapEntityAction.java @@ -76,7 +76,7 @@ public class RdapEntityAction extends RdapActionBase { // they are global, and might have different roles for different domains. if (contact.isPresent() && isAuthorized(contact.get())) { return rdapJsonFormatter.createRdapContactEntity( - contact, ImmutableSet.of(), OutputDataType.FULL); + contact.get(), ImmutableSet.of(), OutputDataType.FULL); } } diff --git a/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java b/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java index e338df6ec..7314a37d1 100644 --- a/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java +++ b/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java @@ -461,7 +461,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { .entitySearchResultsBuilder() .add( rdapJsonFormatter.createRdapContactEntity( - Optional.of(contact), ImmutableSet.of(), outputDataType)); + contact, ImmutableSet.of(), outputDataType)); newCursor = Optional.of( CONTACT_CURSOR_PREFIX diff --git a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java index 29ebca3ea..41ab4a908 100644 --- a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java +++ b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java @@ -75,7 +75,6 @@ import java.net.InetAddress; import java.net.URI; import java.nio.file.Paths; import java.util.HashMap; -import java.util.LinkedHashSet; import java.util.Locale; import java.util.Optional; import java.util.Set; @@ -251,16 +250,6 @@ public class RdapJsonFormatter { private static final Ordering DESIGNATED_CONTACT_ORDERING = Ordering.natural().onResultOf(DesignatedContact::getType); - /** - * The list of RDAP contact roles that are required to be present on each domain. - * - *

Per RDAP Response Profile 2.7.3, A domain MUST have the REGISTRANT, ADMIN, TECH roles and - * MAY have others. We also have the BILLING role in our system but it isn't required and is only - * listed if actually present. - */ - private static final ImmutableSet REQUIRED_CONTACT_ROLES = - ImmutableSet.of(Role.REGISTRANT, Role.ADMIN, Role.TECH); - /** Creates the TOS notice that is added to every reply. */ Notice createTosNotice() { String linkValue = makeRdapServletRelativeUrl("help", RdapHelpAction.TOS_PATH); @@ -403,7 +392,6 @@ public class RdapJsonFormatter { // Convert the contact entities to RDAP output contacts (this also converts the contact types // to RDAP roles). - Set rdapContacts = new LinkedHashSet<>(); for (VKey contactKey : contactsToRoles.keySet()) { Set roles = contactsToRoles.get(contactKey).stream() @@ -412,22 +400,13 @@ public class RdapJsonFormatter { if (roles.isEmpty()) { continue; } - rdapContacts.add( - createRdapContactEntity( - Optional.ofNullable(loadedContacts.get(contactKey)), roles, OutputDataType.INTERNAL)); + builder + .entitiesBuilder() + .add( + createRdapContactEntity( + loadedContacts.get(contactKey), roles, OutputDataType.INTERNAL)); } - // Loop through all required contact roles and fill in placeholder REDACTED info for any - // required ones that are missing, i.e. because of minimum registration data set. - for (Role role : REQUIRED_CONTACT_ROLES) { - if (rdapContacts.stream().noneMatch(c -> c.roles().contains(role))) { - rdapContacts.add( - createRdapContactEntity( - Optional.empty(), ImmutableSet.of(role), OutputDataType.INTERNAL)); - } - } - builder.entitiesBuilder().addAll(rdapContacts); - // Add the nameservers to the data; the load was kicked off above for efficiency. // RDAP Response Profile 2.9: we MUST have the nameservers for (Host host : HOST_RESOURCE_ORDERING.immutableSortedCopy(loadedHosts)) { @@ -537,28 +516,20 @@ public class RdapJsonFormatter { * @param outputDataType whether to generate full or summary data */ RdapContactEntity createRdapContactEntity( - Optional contact, Iterable roles, OutputDataType outputDataType) { + Contact contact, Iterable roles, OutputDataType outputDataType) { RdapContactEntity.Builder contactBuilder = RdapContactEntity.builder(); // RDAP Response Profile 2.7.1, 2.7.3 - we MUST have the contacts. 2.7.4 discusses censoring of // fields we don't want to show (as opposed to not having contacts at all) because of GDPR etc. // // 2.8 allows for unredacted output for authorized people. - // TODO(mcilwain): Once the RDAP profile is fully updated for minimum registration data set, - // we will want to not include non-existent contacts at all, rather than - // pretending they exist and just showing REDACTED info. This is especially - // important for authorized flows, where you wouldn't expect to see redaction - // (although no one actually has access to authorized flows yet). boolean isAuthorized = - contact.isPresent() - && rdapAuthorization.isAuthorizedForRegistrar( - contact.get().getCurrentSponsorRegistrarId()); + rdapAuthorization.isAuthorizedForRegistrar(contact.getCurrentSponsorRegistrarId()); VcardArray.Builder vcardBuilder = VcardArray.builder(); if (isAuthorized) { - fillRdapContactEntityWhenAuthorized( - contactBuilder, vcardBuilder, contact.get(), outputDataType); + fillRdapContactEntityWhenAuthorized(contactBuilder, vcardBuilder, contact, outputDataType); } else { // GTLD Registration Data Temp Spec 17may18, Appendix A, 2.3, 2.4 and RDAP Response Profile // 2.7.4.1, 2.7.4.2 - the following fields must be redacted: diff --git a/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java b/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java index 5ad7b262e..7ad0f350f 100644 --- a/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java @@ -276,27 +276,23 @@ class RdapDomainActionTest extends RdapActionBaseTestCase { } @Test - void testValidDomain_notLoggedIn_noContacts() { - assertProperResponseForCatLol("cat.lol", "rdap_domain_no_contacts_with_remark.json"); + void testValidDomain_notLoggedIn_redactsAllContactInfo() { + assertProperResponseForCatLol("cat.lol", "rdap_domain_redacted_contacts_with_remark.json"); } @Test - void testValidDomain_notLoggedIn_contactsShowRedacted_evenWhenRegistrantDoesntExist() { - // Even though the registrant is empty on this domain, it still shows a full set of REDACTED - // fields through RDAP. + void testValidDomain_notLoggedIn_showsNoRegistrant_whenRegistrantDoesntExist() { persistResource( loadByForeignKey(Domain.class, "cat.lol", clock.nowUtc()) .get() .asBuilder() .setRegistrant(Optional.empty()) .build()); - assertProperResponseForCatLol("cat.lol", "rdap_domain_no_contacts_with_remark.json"); + assertProperResponseForCatLol("cat.lol", "rdap_domain_no_registrant_with_remark.json"); } @Test - void testValidDomain_notLoggedIn_contactsShowRedacted_whenNoContactsExist() { - // Even though the domain has no contacts, it still shows a full set of REDACTED fields through - // RDAP. + void testValidDomain_notLoggedIn_containsNoContactEntities_whenNoContactsExist() { persistResource( loadByForeignKey(Domain.class, "cat.lol", clock.nowUtc()) .get() @@ -308,24 +304,38 @@ class RdapDomainActionTest extends RdapActionBaseTestCase { } @Test - void testValidDomain_loggedInAsOtherRegistrar_noContacts() { + void testValidDomain_loggedIn_containsNoContactEntities_whenNoContactsExist() { + login("evilregistrar"); + persistResource( + loadByForeignKey(Domain.class, "cat.lol", clock.nowUtc()) + .get() + .asBuilder() + .setRegistrant(Optional.empty()) + .setContacts(ImmutableSet.of()) + .build()); + assertProperResponseForCatLol("cat.lol", "rdap_domain_no_contacts_exist_with_remark.json"); + } + + @Test + void testValidDomain_loggedInAsOtherRegistrar_redactsAllContactInfo() { login("idnregistrar"); - assertProperResponseForCatLol("cat.lol", "rdap_domain_no_contacts_with_remark.json"); + assertProperResponseForCatLol("cat.lol", "rdap_domain_redacted_contacts_with_remark.json"); } @Test void testUpperCase_ignored() { - assertProperResponseForCatLol("CaT.lOl", "rdap_domain_no_contacts_with_remark.json"); + assertProperResponseForCatLol("CaT.lOl", "rdap_domain_redacted_contacts_with_remark.json"); } @Test void testTrailingDot_ignored() { - assertProperResponseForCatLol("cat.lol.", "rdap_domain_no_contacts_with_remark.json"); + assertProperResponseForCatLol("cat.lol.", "rdap_domain_redacted_contacts_with_remark.json"); } @Test void testQueryParameter_ignored() { - assertProperResponseForCatLol("cat.lol?key=value", "rdap_domain_no_contacts_with_remark.json"); + assertProperResponseForCatLol( + "cat.lol?key=value", "rdap_domain_redacted_contacts_with_remark.json"); } @Test diff --git a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java index b5e8fbd7c..9ae1e4b78 100644 --- a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java @@ -750,7 +750,7 @@ class RdapDomainSearchActionTest extends RdapSearchActionTestCase