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
This commit is contained in:
gbrodman 2020-03-20 14:12:00 -04:00 committed by GitHub
parent dd7e7c73d8
commit 2b554e76de
22 changed files with 118 additions and 26 deletions

View file

@ -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;

View file

@ -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));

View file

@ -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(

View file

@ -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,

View file

@ -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. */

View file

@ -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")

View file

@ -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,13 +397,31 @@ 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);
assertThat(newContact.isAllowedToSetRegistryLockPassword()).isTrue();
@ -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());

View file

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

View file

@ -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());

View file

@ -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",

View file

@ -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

View file

@ -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());

View file

@ -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;

View file

@ -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}

Binary file not shown.

Before

Width:  |  Height:  |  Size: 49 KiB

After

Width:  |  Height:  |  Size: 50 KiB

Before After
Before After

Binary file not shown.

Before

Width:  |  Height:  |  Size: 50 KiB

After

Width:  |  Height:  |  Size: 51 KiB

Before After
Before After

Binary file not shown.

Before

Width:  |  Height:  |  Size: 69 KiB

After

Width:  |  Height:  |  Size: 70 KiB

Before After
Before After

Binary file not shown.

Before

Width:  |  Height:  |  Size: 54 KiB

After

Width:  |  Height:  |  Size: 54 KiB

Before After
Before After

Binary file not shown.

Before

Width:  |  Height:  |  Size: 52 KiB

After

Width:  |  Height:  |  Size: 52 KiB

Before After
Before After

View file

@ -0,0 +1,15 @@
-- Copyright 2020 The Nomulus Authors. All Rights Reserved.
--
-- Licensed under the Apache License, Version 2.0 (the "License");
-- you may not use this file except in compliance with the License.
-- You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-- See the License for the specific language governing permissions and
-- limitations under the License.
ALTER TABLE "RegistrarPoc" ADD COLUMN registry_lock_email_address text;

View file

@ -158,6 +158,7 @@
gae_user_id text,
name text,
phone_number text,
registry_lock_email_address text,
registry_lock_password_hash text,
registry_lock_password_salt text,
types text[],

View file

@ -241,7 +241,8 @@ CREATE TABLE public."RegistrarPoc" (
types text[],
visible_in_domain_whois_as_abuse boolean NOT NULL,
visible_in_whois_as_admin boolean NOT NULL,
visible_in_whois_as_tech boolean NOT NULL
visible_in_whois_as_tech boolean NOT NULL,
registry_lock_email_address text
);