Add a registryLockEmailAddress field to RegistrarConctact objects (#523)

* Add a registryLockEmailAddress field to RegistrarConctact objects

Because we need to manage the login email, it should be on an account
that we manage. However, for registry lock, we would want to send the
verification emails to a separate email address that the user can use.

As a result, we will use a second field for a user-accessible registry
lock email address. This must be set on the contact when enabling
registry lock for this contact.

* Responses to CR

* derp

core/src/main/java/google/registry/model/registrar/RegistrarContact.java

@@ -44,7 +44,9 @@ import google.registry.model.Jsonifiable;
 import google.registry.model.annotations.ReportedOn;
 import java.util.Arrays;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
+import javax.annotation.Nullable;
 import javax.persistence.Column;
 import javax.persistence.Table;
 import javax.persistence.Transient;
@@ -112,6 +114,9 @@ public class RegistrarContact extends ImmutableObject implements Jsonifiable {
   @Column(nullable = false)
   String emailAddress;
 
+  /** External email address of this contact used for registry lock confirmations. */
+  String registryLockEmailAddress;
+
   /** The voice number of the contact. */
   String phoneNumber;
 
@@ -212,6 +217,10 @@ public class RegistrarContact extends ImmutableObject implements Jsonifiable {
     return emailAddress;
   }
 
+  public Optional<String> getRegistryLockEmailAddress() {
+    return Optional.ofNullable(registryLockEmailAddress);
+  }
+
   public String getPhoneNumber() {
     return phoneNumber;
   }
@@ -318,6 +327,7 @@ public class RegistrarContact extends ImmutableObject implements Jsonifiable {
     return new JsonMapBuilder()
         .put("name", name)
         .put("emailAddress", emailAddress)
+        .put("registryLockEmailAddress", registryLockEmailAddress)
         .put("phoneNumber", phoneNumber)
         .put("faxNumber", faxNumber)
         .put("types", getTypes().stream().map(Object::toString).collect(joining(",")))
@@ -352,6 +362,14 @@ public class RegistrarContact extends ImmutableObject implements Jsonifiable {
     public RegistrarContact build() {
       checkNotNull(getInstance().parent, "Registrar parent cannot be null");
       checkValidEmail(getInstance().emailAddress);
+      // Check allowedToSetRegistryLockPassword here because if we want to allow the user to set
+      // a registry lock password, we must also set up the correct registry lock email concurrently
+      // or beforehand.
+      if (getInstance().allowedToSetRegistryLockPassword) {
+        checkArgument(
+            !isNullOrEmpty(getInstance().registryLockEmailAddress),
+            "Registry lock email must not be null if allowing registry lock access");
+      }
       return cloneEmptyToNull(super.build());
     }
 
@@ -365,6 +383,11 @@ public class RegistrarContact extends ImmutableObject implements Jsonifiable {
       return this;
     }
 
+    public Builder setRegistryLockEmailAddress(@Nullable String registryLockEmailAddress) {
+      getInstance().registryLockEmailAddress = registryLockEmailAddress;
+      return this;
+    }
+
     public Builder setPhoneNumber(String phoneNumber) {
       getInstance().phoneNumber = phoneNumber;
       return this;

core/src/main/java/google/registry/tools/RegistrarContactCommand.java

@@ -78,11 +78,15 @@ final class RegistrarContactCommand extends MutatingCommand {
   private List<String> contactTypeNames;
 
   @Nullable
-  @Parameter(
-      names = "--email",
-      description = "Contact email address.")
+  @Parameter(names = "--email", description = "Contact email address.")
   String email;
 
+  @Nullable
+  @Parameter(
+      names = "--registry_lock_email",
+      description = "Email address used for registry lock confirmation emails")
+  String registryLockEmail;
+
   @Nullable
   @Parameter(
       names = "--phone",
@@ -247,6 +251,9 @@ final class RegistrarContactCommand extends MutatingCommand {
     builder.setParent(registrar);
     builder.setName(name);
     builder.setEmailAddress(email);
+    if (!isNullOrEmpty(registryLockEmail)) {
+      builder.setRegistryLockEmailAddress(registryLockEmail);
+    }
     if (phone != null) {
       builder.setPhoneNumber(phone.orElse(null));
     }
@@ -277,14 +284,14 @@ final class RegistrarContactCommand extends MutatingCommand {
 
   private RegistrarContact updateContact(RegistrarContact contact, Registrar registrar) {
     checkNotNull(registrar);
-    checkNotNull(email, "--email is required when --mode=UPDATE");
-    RegistrarContact.Builder builder = contact.asBuilder();
-    builder.setParent(registrar);
+    checkArgument(!isNullOrEmpty(email), "--email is required when --mode=UPDATE");
+    RegistrarContact.Builder builder =
+        contact.asBuilder().setEmailAddress(email).setParent(registrar);
     if (!isNullOrEmpty(name)) {
       builder.setName(name);
     }
-    if (!isNullOrEmpty(email)) {
-      builder.setEmailAddress(email);
+    if (!isNullOrEmpty(registryLockEmail)) {
+      builder.setRegistryLockEmailAddress(registryLockEmail);
     }
     if (phone != null) {
       builder.setPhoneNumber(phone.orElse(null));

core/src/main/java/google/registry/ui/server/RegistrarFormFields.java

@@ -184,6 +184,11 @@ public final class RegistrarFormFields {
           .required()
           .build();
 
+  public static final FormField<String, String> REGISTRY_LOCK_EMAIL_ADDRESS_FIELD =
+      FormFields.EMAIL
+          .asBuilderNamed("registryLockEmailAddress")
+          .build();
+
   public static final FormField<Boolean, Boolean> CONTACT_VISIBLE_IN_WHOIS_AS_ADMIN_FIELD =
       FormField.named("visibleInWhoisAsAdmin", Boolean.class)
           .build();
@@ -377,6 +382,8 @@ public final class RegistrarFormFields {
       RegistrarContact.Builder builder, Map<String, ?> args) {
     builder.setName(CONTACT_NAME_FIELD.extractUntyped(args).orElse(null));
     builder.setEmailAddress(CONTACT_EMAIL_ADDRESS_FIELD.extractUntyped(args).orElse(null));
+    builder.setRegistryLockEmailAddress(
+        REGISTRY_LOCK_EMAIL_ADDRESS_FIELD.extractUntyped(args).orElse(null));
     builder.setVisibleInWhoisAsAdmin(
         CONTACT_VISIBLE_IN_WHOIS_AS_ADMIN_FIELD.extractUntyped(args).orElse(false));
     builder.setVisibleInWhoisAsTech(

core/src/main/java/google/registry/ui/server/registrar/RegistryLockGetAction.java

@@ -157,9 +157,11 @@ public final class RegistryLockGetAction implements JsonGetAction {
     Optional<RegistrarContact> contactOptional = getContactMatchingLogin(user, registrar);
     boolean isRegistryLockAllowed =
         isAdmin || contactOptional.map(RegistrarContact::isRegistryLockAllowed).orElse(false);
-    // Use the contact email if it's present, else use the login email
+    // Use the contact's registry lock email if it's present, else use the login email (for admins)
     String relevantEmail =
-        contactOptional.map(RegistrarContact::getEmailAddress).orElse(user.getEmail());
+        contactOptional
+            .map(contact -> contact.getRegistryLockEmailAddress().get())
+            .orElse(user.getEmail());
     return ImmutableMap.of(
         LOCK_ENABLED_FOR_CONTACT_PARAM,
         isRegistryLockAllowed,

core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java

@@ -202,7 +202,14 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
     checkArgument(
         registrarContact.verifyRegistryLockPassword(postInput.password),
         "Incorrect registry lock password for contact");
-    return registrarContact.getEmailAddress();
+    return registrarContact
+        .getRegistryLockEmailAddress()
+        .orElseThrow(
+            () ->
+                new IllegalStateException(
+                    String.format(
+                        "Contact %s had no registry lock email address",
+                        registrarContact.getEmailAddress())));
   }
 
   /** Value class that represents the expected input body from the UI request. */

core/src/test/java/google/registry/testing/AppEngineRule.java

@@ -247,6 +247,7 @@ public final class AppEngineRule extends ExternalResource {
         .setParent(makeRegistrar2())
         .setName("Marla Singer")
         .setEmailAddress("Marla.Singer@crr.com")
+        .setRegistryLockEmailAddress("Marla.Singer.RegistryLock@crr.com")
         .setPhoneNumber("+1.2128675309")
         .setTypes(ImmutableSet.of(RegistrarContact.Type.TECH))
         .setGaeUserId("12345")

core/src/test/java/google/registry/tools/RegistrarContactCommandTest.java

@@ -89,6 +89,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--mode=UPDATE",
         "--name=Judith Registrar",
         "--email=judith.doe@example.com",
+        "--registry_lock_email=judith.doe@external.com",
         "--phone=+1.2125650000",
         "--fax=+1.2125650001",
         "--contact_type=WHOIS",
@@ -102,6 +103,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
             .setParent(registrar)
             .setName("Judith Registrar")
             .setEmailAddress("judith.doe@example.com")
+            .setRegistryLockEmailAddress("judith.doe@external.com")
             .setPhoneNumber("+1.2125650000")
             .setFaxNumber("+1.2125650001")
             .setTypes(ImmutableSet.of(WHOIS))
@@ -292,6 +294,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--mode=CREATE",
         "--name=Jim Doe",
         "--email=jim.doe@example.com",
+        "--registry_lock_email=jim.doe@external.com",
         "--contact_type=ADMIN,ABUSE",
         "--visible_in_whois_as_admin=true",
         "--visible_in_whois_as_tech=false",
@@ -303,6 +306,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
             .setParent(registrar)
             .setName("Jim Doe")
             .setEmailAddress("jim.doe@example.com")
+            .setRegistryLockEmailAddress("jim.doe@external.com")
             .setTypes(ImmutableSet.of(ADMIN, ABUSE))
             .setVisibleInWhoisAsAdmin(true)
             .setVisibleInWhoisAsTech(false)
@@ -374,6 +378,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
         "--mode=CREATE",
         "--name=Jim Doe",
         "--email=jim.doe@example.com",
+        "--registry_lock_email=jim.doe.registry.lock@example.com",
         "--allowed_to_set_registry_lock_password=true",
         "NewRegistrar");
     RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
@@ -392,12 +397,30 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
                 .setEmailAddress("jim.doe@example.com")
                 .build());
     assertThat(registrarContact.isAllowedToSetRegistryLockPassword()).isFalse();
+
+    // First, try (and fail) to set the password directly
     assertThrows(
         IllegalArgumentException.class,
         () -> registrarContact.asBuilder().setRegistryLockPassword("foo"));
+
+    // Next, try (and fail) to allow registry lock without a registry lock email
+    assertThat(
+            assertThrows(
+                IllegalArgumentException.class,
+                () ->
+                    runCommandForced(
+                        "--mode=UPDATE",
+                        "--email=jim.doe@example.com",
+                        "--allowed_to_set_registry_lock_password=true",
+                        "NewRegistrar")))
+        .hasMessageThat()
+        .isEqualTo("Registry lock email must not be null if allowing registry lock access");
+
+    // Next, include the email and it should succeed
     runCommandForced(
         "--mode=UPDATE",
         "--email=jim.doe@example.com",
+        "--registry_lock_email=jim.doe.registry.lock@example.com",
         "--allowed_to_set_registry_lock_password=true",
         "NewRegistrar");
     RegistrarContact newContact = reloadResource(registrarContact);
@@ -415,6 +438,7 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
                 .setParent(registrar)
                 .setName("Jim Doe")
                 .setEmailAddress("jim.doe@example.com")
+                .setRegistryLockEmailAddress("jim.doe.registry.lock@example.com")
                 .setAllowedToSetRegistryLockPassword(true)
                 .setRegistryLockPassword("hi")
                 .build());

core/src/test/java/google/registry/ui/server/registrar/ContactSettingsTest.java

@@ -237,6 +237,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
             AppEngineRule.makeRegistrarContact2()
                 .asBuilder()
                 .setEmailAddress("someotheremail@example.com")
+                .setRegistryLockEmailAddress("someotherexample@example.com")
                 .setAllowedToSetRegistryLockPassword(true)
                 .build()
                 .toJsonMap(),

core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java

@@ -100,6 +100,7 @@ public abstract class RegistrarSettingsActionTestCase {
                 .setParent(loadRegistrar(CLIENT_ID))
                 .setName("Jian-Yang")
                 .setEmailAddress("jyang@bachman.accelerator")
+                .setRegistryLockEmailAddress("jyang.registrylock@bachman.accelerator")
                 .setPhoneNumber("+1.1234567890")
                 .setTypes(ImmutableSet.of(RegistrarContact.Type.TECH))
                 .build());

core/src/test/java/google/registry/ui/server/registrar/RegistryLockGetActionTest.java

@@ -182,7 +182,7 @@ public final class RegistryLockGetActionTest {
                         "lockEnabledForContact",
                         true,
                         "email",
-                        "Marla.Singer@crr.com",
+                        "Marla.Singer.RegistryLock@crr.com",
                         "clientId",
                         "TheRegistrar",
                         "locks",
@@ -276,7 +276,7 @@ public final class RegistryLockGetActionTest {
                     "lockEnabledForContact",
                     false,
                     "email",
-                    "Marla.Singer@crr.com",
+                    "Marla.Singer.RegistryLock@crr.com",
                     "clientId",
                     "TheRegistrar",
                     "locks",
@@ -304,7 +304,7 @@ public final class RegistryLockGetActionTest {
                     "lockEnabledForContact",
                     true,
                     "email",
-                    "Marla.Singer@crr.com",
+                    "Marla.Singer.RegistryLock@crr.com",
                     "clientId",
                     "TheRegistrar",
                     "locks",
@@ -327,7 +327,7 @@ public final class RegistryLockGetActionTest {
                     "lockEnabledForContact",
                     true,
                     "email",
-                    "Marla.Singer@crr.com",
+                    "Marla.Singer.RegistryLock@crr.com",
                     "clientId",
                     "TheRegistrar",
                     "locks",

core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java

@@ -105,7 +105,7 @@ public final class RegistryLockPostActionTest {
   @Test
   public void testSuccess_lock() throws Exception {
     Map<String, ?> response = action.handleJsonRequest(lockRequest());
-    assertSuccess(response, "lock", "Marla.Singer@crr.com");
+    assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com");
   }
 
   @Test
@@ -113,7 +113,7 @@ public final class RegistryLockPostActionTest {
     saveRegistryLock(createLock().asBuilder().setLockCompletionTimestamp(clock.nowUtc()).build());
     persistResource(domain.asBuilder().setStatusValues(REGISTRY_LOCK_STATUSES).build());
     Map<String, ?> response = action.handleJsonRequest(unlockRequest());
-    assertSuccess(response, "unlock", "Marla.Singer@crr.com");
+    assertSuccess(response, "unlock", "Marla.Singer.RegistryLock@crr.com");
   }
 
   @Test
@@ -142,7 +142,7 @@ public final class RegistryLockPostActionTest {
         createAction(
             AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithLockPermission, false)));
     Map<String, ?> response = action.handleJsonRequest(lockRequest());
-    assertSuccess(response, "lock", "Marla.Singer@crr.com");
+    assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com");
   }
 
   @Test
@@ -309,7 +309,7 @@ public final class RegistryLockPostActionTest {
             .build());
 
     Map<String, ?> response = action.handleJsonRequest(lockRequest());
-    assertSuccess(response, "lock", "Marla.Singer@crr.com");
+    assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com");
   }
 
   @Test
@@ -319,7 +319,7 @@ public final class RegistryLockPostActionTest {
     previousLock = getRegistryLockByVerificationCode(verificationCode).get();
     clock.setTo(previousLock.getLockRequestTimestamp().plusHours(2));
     Map<String, ?> response = action.handleJsonRequest(lockRequest());
-    assertSuccess(response, "lock", "Marla.Singer@crr.com");
+    assertSuccess(response, "lock", "Marla.Singer.RegistryLock@crr.com");
   }
 
   @Test

core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java

@@ -153,6 +153,7 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
           persistResource(
               makeRegistrarContact2()
                   .asBuilder()
+                  .setRegistryLockEmailAddress("johndoe.registrylock@example.com")
                   .setAllowedToSetRegistryLockPassword(true)
                   .build());
           persistResource(makeRegistrar2().asBuilder().setRegistryLockAllowed(true).build());

core/src/test/resources/google/registry/model/schema.txt

@@ -477,6 +477,7 @@ class google.registry.model.registrar.RegistrarContact {
   java.lang.String gaeUserId;
   java.lang.String name;
   java.lang.String phoneNumber;
+  java.lang.String registryLockEmailAddress;
   java.lang.String registryLockPasswordHash;
   java.lang.String registryLockPasswordSalt;
   java.util.Set<google.registry.model.registrar.RegistrarContact$Type> types;

core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt

@@ -11,10 +11,10 @@ emailAddress: the.registrar@example.com -> thase@the.registrar
 url: http://my.fake.url -> http://my.new.url
 contacts:
     ADDED:
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, registryLockEmailAddress=null, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}
     REMOVED:
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Marla Singer, emailAddress=Marla.Singer@crr.com, phoneNumber=+1.2128675309, faxNumber=null, types=[TECH], gaeUserId=12345, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false},
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=John Doe, emailAddress=johndoe@theregistrar.com, phoneNumber=+1.1234567890, faxNumber=null, types=[ADMIN], gaeUserId=31337, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false},
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Jian-Yang, emailAddress=jyang@bachman.accelerator, phoneNumber=+1.1234567890, faxNumber=null, types=[TECH], gaeUserId=null, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Marla Singer, emailAddress=Marla.Singer@crr.com, registryLockEmailAddress=Marla.Singer.RegistryLock@crr.com, phoneNumber=+1.2128675309, faxNumber=null, types=[TECH], gaeUserId=12345, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false},
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=John Doe, emailAddress=johndoe@theregistrar.com, registryLockEmailAddress=null, phoneNumber=+1.1234567890, faxNumber=null, types=[ADMIN], gaeUserId=31337, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false},
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Jian-Yang, emailAddress=jyang@bachman.accelerator, registryLockEmailAddress=jyang.registrylock@bachman.accelerator, phoneNumber=+1.1234567890, faxNumber=null, types=[TECH], gaeUserId=null, visibleInWhoisAsAdmin=false, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}
     FINAL CONTENTS:
-        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}
+        {parent=Key<?>(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, registryLockEmailAddress=null, phoneNumber=+1.2345678901, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false, allowedToSetRegistryLockPassword=false}