Enforce abuse WHOIS contact for REAL registrars when adding TLDs

We do not enforce this for non-REAL registrars or in any environment other than UNITTEST or PRODUCTION. This is similar but separate to [] since we can add allowed TLDs in either location.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=239510275

java/google/registry/model/registrar/Registrar.java

@@ -561,6 +561,16 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable
         .collect(toImmutableSortedSet(CONTACT_EMAIL_COMPARATOR));
   }
 
+  /**
+   * Returns the {@link RegistrarContact} that is the WHOIS abuse contact for this registrar, or
+   * empty if one does not exist.
+   */
+  public Optional<RegistrarContact> getWhoisAbuseContact() {
+    return getContacts().stream()
+        .filter(RegistrarContact::getVisibleInDomainWhoisAsAbuse)
+        .findFirst();
+  }
+
   private Iterable<RegistrarContact> getContactsIterable() {
     return ofy().load().type(RegistrarContact.class).ancestor(Registrar.this);
   }

java/google/registry/tools/CreateOrUpdateRegistrarCommand.java

@@ -258,6 +258,8 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
   @Nullable
   abstract Registrar getOldRegistrar(String clientId);
 
+  abstract void checkModifyAllowedTlds(@Nullable Registrar oldRegistrar);
+
   protected void initRegistrarCommand() {}
 
   @Override
@@ -300,9 +302,12 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
       if (driveFolderId != null) {
         builder.setDriveFolderId(driveFolderId.orElse(null));
       }
+      if (!allowedTlds.isEmpty() || !addAllowedTlds.isEmpty()) {
+        checkModifyAllowedTlds(oldRegistrar);
+      }
       if (!allowedTlds.isEmpty()) {
-        checkArgument(addAllowedTlds.isEmpty(),
+        checkArgument(
-            "Can't specify both --allowedTlds and --addAllowedTlds");
+            addAllowedTlds.isEmpty(), "Can't specify both --allowedTlds and --addAllowedTlds");
         ImmutableSet.Builder<String> allowedTldsBuilder = new ImmutableSet.Builder<>();
         for (String allowedTld : allowedTlds) {
           allowedTldsBuilder.add(canonicalizeDomainName(allowedTld));

java/google/registry/tools/CreateRegistrarCommand.java

@@ -30,6 +30,7 @@ import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Streams;
+import google.registry.config.RegistryEnvironment;
 import google.registry.model.registrar.Registrar;
 import java.util.ArrayList;
 import java.util.List;
@@ -86,14 +87,28 @@ final class CreateRegistrarCommand extends CreateOrUpdateRegistrarCommand
             .filter(registrar -> normalizeClientId(registrar.getClientId()).equals(clientId))
             .collect(toCollection(ArrayList::new));
     if (!collisions.isEmpty()) {
-      throw new IllegalArgumentException(String.format(
+      throw new IllegalArgumentException(
-          "The registrar client identifier %s normalizes identically to existing registrar %s",
+          String.format(
-          clientId,
+              "The registrar client identifier %s normalizes identically to existing registrar %s",
-          collisions.get(0).getClientId()));
+              clientId, collisions.get(0).getClientId()));
     }
     return null;
   }
 
+  @Override
+  void checkModifyAllowedTlds(@Nullable Registrar oldRegistrar) {
+    // When creating a registrar, only allow allowed-TLD modification if we're in a non-PRODUCTION
+    // environment and/or the registrar is not REAL
+    checkArgument(
+        !RegistryEnvironment.PRODUCTION.equals(RegistryEnvironment.get())
+            || !Registrar.Type.REAL.equals(registrarType),
+        "Cannot add allowed TLDs when creating a REAL registrar in a production environment."
+            + " Please create the registrar without allowed TLDs, then use `nomulus"
+            + " registrar_contact` to create a registrar contact for it that is visible as the"
+            + " abuse contact in WHOIS. Then use `nomulus update_registrar` to add the allowed"
+            + " TLDs.");
+  }
+
   @Override
   protected String postExecute() {
     if (!createGoogleGroups) {

java/google/registry/tools/UpdateRegistrarCommand.java

@@ -14,10 +14,13 @@
 
 package google.registry.tools;
 
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.beust.jcommander.Parameters;
+import google.registry.config.RegistryEnvironment;
 import google.registry.model.registrar.Registrar;
+import javax.annotation.Nullable;
 
 /** Command to update a Registrar. */
 @Parameters(separators = " =", commandDescription = "Update registrar account(s)")
@@ -28,4 +31,22 @@ final class UpdateRegistrarCommand extends CreateOrUpdateRegistrarCommand {
     return checkArgumentPresent(
         Registrar.loadByClientId(clientId), "Registrar %s not found", clientId);
   }
+
+  @Override
+  void checkModifyAllowedTlds(@Nullable Registrar oldRegistrar) {
+    // Only allow modifying allowed TLDs if we're in a non-PRODUCTION environment, if the registrar
+    // is not REAL, or the registrar has a WHOIS abuse contact set.
+    checkArgumentNotNull(oldRegistrar, "Old registrar was not present during modification");
+
+    boolean isRealRegistrar =
+        Registrar.Type.REAL.equals(registrarType)
+            || (Registrar.Type.REAL.equals(oldRegistrar.getType()) && registrarType == null);
+    if (RegistryEnvironment.PRODUCTION.equals(RegistryEnvironment.get()) && isRealRegistrar) {
+      checkArgumentPresent(
+          oldRegistrar.getWhoisAbuseContact(),
+          "Cannot modify allowed TLDs if there is no WHOIS abuse contact set. Please use the"
+              + " \"nomulus registrar_contact\" command on this registrar to set a WHOIS abuse"
+              + " contact.");
+    }
+  }
 }

javatests/google/registry/tools/CreateRegistrarCommandTest.java

@@ -163,10 +163,11 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
   }
 
   @Test
-  public void testSuccess_allowedTlds() throws Exception {
+  public void testSuccess_allowedTldsInNonProductionEnvironment() throws Exception {
     createTlds("xn--q9jyb4c", "foobar");
 
-    runCommandForced(
+    runCommandInEnvironment(
+        RegistryToolEnvironment.SANDBOX,
         "--name=blobio",
         "--password=some_password",
         "--registrar_type=REAL",
@@ -180,6 +181,34 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         "--state MA",
         "--zip 00351",
         "--cc US",
+        "--force",
+        "clientz");
+
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
+    assertThat(registrar).isPresent();
+    assertThat(registrar.get().getAllowedTlds()).containsExactly("xn--q9jyb4c", "foobar");
+  }
+
+  @Test
+  public void testSuccess_allowedTldsInPDT() throws Exception {
+    createTlds("xn--q9jyb4c", "foobar");
+
+    runCommandInEnvironment(
+        RegistryToolEnvironment.PRODUCTION,
+        "--name=blobio",
+        "--password=some_password",
+        "--registrar_type=PDT",
+        "--iana_id=9995",
+        "--allowed_tlds=xn--q9jyb4c,foobar",
+        "--billing_account_map=USD=123abc",
+        "--passcode=01234",
+        "--icann_referral_email=foo@bar.test",
+        "--street=\"123 Fake St\"",
+        "--city Fakington",
+        "--state MA",
+        "--zip 00351",
+        "--cc US",
+        "--force",
         "clientz");
 
     Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
@@ -468,7 +497,8 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
         assertThrows(
             IllegalArgumentException.class,
             () ->
-                runCommandForced(
+                runCommandInEnvironment(
+                    RegistryToolEnvironment.SANDBOX,
                     "--name=blobio",
                     "--password=some_password",
                     "--registrar_type=REAL",
@@ -482,6 +512,7 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
                     "--state MA",
                     "--zip 00351",
                     "--cc US",
+                    "--force",
                     "clientz"));
     assertThat(thrown).hasMessageThat().contains("USD");
   }
@@ -884,6 +915,32 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
                 "clientz"));
   }
 
+  @Test
+  public void testFailure_allowedTldsInRealWithoutAbuseContact() {
+    createTlds("xn--q9jyb4c", "foobar");
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommandInEnvironment(
+                    RegistryToolEnvironment.PRODUCTION,
+                    "--name=blobio",
+                    "--password=some_password",
+                    "--registrar_type=REAL",
+                    "--iana_id=8",
+                    "--allowed_tlds=foobar",
+                    "--passcode=01234",
+                    "--icann_referral_email=foo@bar.test",
+                    "--street=\"123 Fake St\"",
+                    "--city Fakington",
+                    "--state MA",
+                    "--zip 00351",
+                    "--cc US",
+                    "--force",
+                    "clientz"));
+    assertThat(thrown).hasMessageThat().startsWith("Cannot add allowed TLDs");
+  }
+
   @Test
   public void testFailure_invalidIpWhitelistFlag() {
     assertThrows(

javatests/google/registry/tools/UpdateRegistrarCommandTest.java

@@ -32,6 +32,7 @@ import com.google.common.collect.ImmutableSet;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.Registrar.Type;
+import google.registry.testing.AppEngineRule;
 import google.registry.util.CidrAddressBlock;
 import java.util.Optional;
 import org.joda.money.CurrencyUnit;
@@ -86,43 +87,94 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
 
   @Test
   public void testSuccess_allowedTlds() throws Exception {
+    persistWhoisAbuseContact();
     createTlds("xn--q9jyb4c", "foobar");
     persistResource(
         loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
             .build());
-    runCommand("--allowed_tlds=xn--q9jyb4c,foobar", "--force", "NewRegistrar");
+    runCommandInEnvironment(
+        RegistryToolEnvironment.PRODUCTION,
+        "--allowed_tlds=xn--q9jyb4c,foobar",
+        "--force",
+        "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
         .containsExactly("xn--q9jyb4c", "foobar");
   }
 
   @Test
   public void testSuccess_addAllowedTlds() throws Exception {
+    persistWhoisAbuseContact();
     createTlds("xn--q9jyb4c", "foo", "bar");
     persistResource(
         loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
             .build());
-    runCommand("--add_allowed_tlds=foo,bar", "--force", "NewRegistrar");
+    runCommandInEnvironment(
+        RegistryToolEnvironment.PRODUCTION,
+        "--add_allowed_tlds=foo,bar",
+        "--force",
+        "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
         .containsExactly("xn--q9jyb4c", "foo", "bar");
   }
 
   @Test
   public void testSuccess_addAllowedTldsWithDupes() throws Exception {
+    persistWhoisAbuseContact();
     createTlds("xn--q9jyb4c", "foo", "bar");
     persistResource(
         loadRegistrar("NewRegistrar")
             .asBuilder()
             .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
             .build());
-    runCommand("--add_allowed_tlds=xn--q9jyb4c,foo,bar", "--force", "NewRegistrar");
+    runCommandInEnvironment(
+        RegistryToolEnvironment.PRODUCTION,
+        "--add_allowed_tlds=xn--q9jyb4c,foo,bar",
+        "--force",
+        "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
         .isEqualTo(ImmutableSet.of("xn--q9jyb4c", "foo", "bar"));
   }
 
+  @Test
+  public void testSuccess_allowedTldsInNonProductionEnvironment() throws Exception {
+    createTlds("xn--q9jyb4c", "foobar");
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
+            .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
+            .build());
+    runCommandInEnvironment(
+        RegistryToolEnvironment.SANDBOX,
+        "--allowed_tlds=xn--q9jyb4c,foobar",
+        "--force",
+        "NewRegistrar");
+    assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
+        .containsExactly("xn--q9jyb4c", "foobar");
+  }
+
+  @Test
+  public void testSuccess_allowedTldsInPdtRegistrar() throws Exception {
+    createTlds("xn--q9jyb4c", "foobar");
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
+            .setType(Type.PDT)
+            .setIanaIdentifier(9995L)
+            .setAllowedTlds(ImmutableSet.of("xn--q9jyb4c"))
+            .build());
+    runCommandInEnvironment(
+        RegistryToolEnvironment.PRODUCTION,
+        "--allowed_tlds=xn--q9jyb4c,foobar",
+        "--force",
+        "NewRegistrar");
+    assertThat(loadRegistrar("NewRegistrar").getAllowedTlds())
+        .containsExactly("xn--q9jyb4c", "foobar");
+  }
+
   @Test
   public void testSuccess_ipWhitelist() throws Exception {
     assertThat(loadRegistrar("NewRegistrar").getIpAddressWhitelist()).isEmpty();
@@ -531,6 +583,36 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
             runCommand("--allowed_tlds=bar", "--add_allowed_tlds=foo", "--force", "NewRegistrar"));
   }
 
+  @Test
+  public void testFailure_setAllowedTldsWithoutAbuseContact() {
+    createTlds("bar");
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommandInEnvironment(
+                    RegistryToolEnvironment.PRODUCTION,
+                    "--allowed_tlds=bar",
+                    "--force",
+                    "TheRegistrar"));
+    assertThat(thrown).hasMessageThat().startsWith("Cannot modify allowed TLDs");
+  }
+
+  @Test
+  public void testFailure_addAllowedTldsWithoutAbuseContact() {
+    createTlds("bar");
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommandInEnvironment(
+                    RegistryToolEnvironment.PRODUCTION,
+                    "--add_allowed_tlds=bar",
+                    "--force",
+                    "TheRegistrar"));
+    assertThat(thrown).hasMessageThat().startsWith("Cannot modify allowed TLDs");
+  }
+
   @Test
   public void testFailure_invalidIpWhitelist() {
     assertThrows(
@@ -732,4 +814,12 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
     runCommand("--po_number=null", "--force", "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getPoNumber()).isEmpty();
   }
+
+  private void persistWhoisAbuseContact() {
+    persistResource(
+        AppEngineRule.makeRegistrarContact1()
+            .asBuilder()
+            .setVisibleInDomainWhoisAsAbuse(true)
+            .build());
+  }
 }