diff --git a/core/src/main/java/google/registry/ui/server/RegistrarFormFields.java b/core/src/main/java/google/registry/ui/server/RegistrarFormFields.java index 65130f5d4..292d3442a 100644 --- a/core/src/main/java/google/registry/ui/server/RegistrarFormFields.java +++ b/core/src/main/java/google/registry/ui/server/RegistrarFormFields.java @@ -21,7 +21,9 @@ import static google.registry.util.DomainNameUtils.canonicalizeDomainName; import com.google.common.base.Ascii; import com.google.common.base.Splitter; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.net.InternetDomainName; import com.google.re2j.Pattern; import google.registry.model.registrar.Registrar; @@ -194,12 +196,10 @@ public final class RegistrarFormFields { FormField.named("visibleInDomainWhoisAsAbuse", Boolean.class).build(); public static final FormField CONTACT_PHONE_NUMBER_FIELD = - FormFields.PHONE_NUMBER.asBuilder() - .build(); + FormFields.PHONE_NUMBER.asBuilder().build(); public static final FormField CONTACT_FAX_NUMBER_FIELD = - FormFields.PHONE_NUMBER.asBuilderNamed("faxNumber") - .build(); + FormFields.PHONE_NUMBER.asBuilderNamed("faxNumber").build(); public static final FormField CONTACT_GAE_USER_ID_FIELD = FormFields.NAME.asBuilderNamed("gaeUserId").build(); @@ -217,11 +217,8 @@ public final class RegistrarFormFields { .asSet(Splitter.on(',').omitEmptyStrings().trimResults()) .build(); - public static final FormField>, List> - CONTACTS_FIELD = FormField.mapNamed("contacts") - .transform(RegistrarContact.Builder.class, RegistrarFormFields::toRegistrarContactBuilder) - .asList() - .build(); + public static final FormField>, List>> CONTACTS_AS_MAPS = + FormField.mapNamed("contacts").asList().build(); public static final FormField, List> I18N_STREET_FIELD = FormFields.XS_NORMALIZED_STRING.asBuilderNamed("street") @@ -344,33 +341,60 @@ public final class RegistrarFormFields { } } - private static @Nullable RegistrarContact.Builder toRegistrarContactBuilder( - @Nullable Map args) { + public static ImmutableList getRegistrarContactBuilders( + ImmutableSet existingContacts, @Nullable Map args) { if (args == null) { - return null; + return ImmutableList.of(); } - RegistrarContact.Builder builder = new RegistrarContact.Builder(); - CONTACT_NAME_FIELD.extractUntyped(args).ifPresent(builder::setName); - CONTACT_EMAIL_ADDRESS_FIELD.extractUntyped(args).ifPresent(builder::setEmailAddress); - CONTACT_VISIBLE_IN_WHOIS_AS_ADMIN_FIELD - .extractUntyped(args) - .ifPresent(builder::setVisibleInWhoisAsAdmin); - CONTACT_VISIBLE_IN_WHOIS_AS_TECH_FIELD - .extractUntyped(args) - .ifPresent(builder::setVisibleInWhoisAsTech); - PHONE_AND_EMAIL_VISIBLE_IN_DOMAIN_WHOIS_AS_ABUSE_FIELD - .extractUntyped(args) - .ifPresent(builder::setVisibleInDomainWhoisAsAbuse); - CONTACT_PHONE_NUMBER_FIELD.extractUntyped(args).ifPresent(builder::setPhoneNumber); - CONTACT_FAX_NUMBER_FIELD.extractUntyped(args).ifPresent(builder::setFaxNumber); - CONTACT_TYPES.extractUntyped(args).ifPresent(builder::setTypes); - CONTACT_GAE_USER_ID_FIELD.extractUntyped(args).ifPresent(builder::setGaeUserId); - CONTACT_ALLOWED_TO_SET_REGISTRY_LOCK_PASSWORD - .extractUntyped(args) - .ifPresent(builder::setAllowedToSetRegistryLockPassword); + Optional>> contactsAsMaps = CONTACTS_AS_MAPS.extractUntyped(args); + if (!contactsAsMaps.isPresent()) { + return ImmutableList.of(); + } + ImmutableList.Builder result = new ImmutableList.Builder<>(); + for (Map contactAsMap : contactsAsMaps.get()) { + String emailAddress = + CONTACT_EMAIL_ADDRESS_FIELD + .extractUntyped(contactAsMap) + .orElseThrow( + () -> new IllegalArgumentException("Contacts from UI must have email addresses")); + // Start with a new builder if the contact didn't previously exist + RegistrarContact.Builder contactBuilder = + existingContacts.stream() + .filter(rc -> rc.getEmailAddress().equals(emailAddress)) + .findFirst() + .map(RegistrarContact::asBuilder) + .orElse(new RegistrarContact.Builder()); + applyRegistrarContactArgs(contactBuilder, contactAsMap); + result.add(contactBuilder); + } + return result.build(); + } + + private static void applyRegistrarContactArgs( + RegistrarContact.Builder builder, Map args) { + builder.setName(CONTACT_NAME_FIELD.extractUntyped(args).orElse(null)); + builder.setEmailAddress(CONTACT_EMAIL_ADDRESS_FIELD.extractUntyped(args).orElse(null)); + builder.setVisibleInWhoisAsAdmin( + CONTACT_VISIBLE_IN_WHOIS_AS_ADMIN_FIELD.extractUntyped(args).orElse(false)); + builder.setVisibleInWhoisAsTech( + CONTACT_VISIBLE_IN_WHOIS_AS_TECH_FIELD.extractUntyped(args).orElse(false)); + builder.setVisibleInDomainWhoisAsAbuse( + PHONE_AND_EMAIL_VISIBLE_IN_DOMAIN_WHOIS_AS_ABUSE_FIELD.extractUntyped(args).orElse(false)); + builder.setPhoneNumber(CONTACT_PHONE_NUMBER_FIELD.extractUntyped(args).orElse(null)); + builder.setFaxNumber(CONTACT_FAX_NUMBER_FIELD.extractUntyped(args).orElse(null)); + builder.setTypes(CONTACT_TYPES.extractUntyped(args).orElse(ImmutableSet.of())); + builder.setGaeUserId(CONTACT_GAE_USER_ID_FIELD.extractUntyped(args).orElse(null)); + builder.setAllowedToSetRegistryLockPassword( + CONTACT_ALLOWED_TO_SET_REGISTRY_LOCK_PASSWORD.extractUntyped(args).orElse(false)); + + // Registry lock password does not need to be set every time CONTACT_REGISTRY_LOCK_PASSWORD_FIELD .extractUntyped(args) - .ifPresent(builder::setRegistryLockPassword); - return builder; + .ifPresent( + password -> { + if (!Strings.isNullOrEmpty(password)) { + builder.setRegistryLockPassword(password); + } + }); } } diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 285e9692c..3237830a9 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -60,7 +60,6 @@ import google.registry.util.CollectionUtils; import google.registry.util.DiffUtils; import java.util.HashSet; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -175,8 +174,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA } private RegistrarResult update(final Map args, String clientId) { - return tm() - .transact( + return tm().transact( () -> { // We load the registrar here rather than outside of the transaction - to make // sure we have the latest version. This one is loaded inside the transaction, so it's @@ -215,7 +213,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA updatedRegistrar = checkAndUpdateAdminControlledFields(updatedRegistrar, args); // read the contacts from the request. - ImmutableSet updatedContacts = readContacts(registrar, args); + ImmutableSet updatedContacts = + readContacts(registrar, contacts, args); // Save the updated contacts if (!updatedContacts.equals(contacts)) { @@ -384,16 +383,10 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA /** Reads the contacts from the supplied args. */ public static ImmutableSet readContacts( - Registrar registrar, Map args) { - - ImmutableSet.Builder contacts = new ImmutableSet.Builder<>(); - Optional> builders = - RegistrarFormFields.CONTACTS_FIELD.extractUntyped(args); - if (builders.isPresent()) { - builders.get().forEach(c -> contacts.add(c.setParent(registrar).build())); - } - - return contacts.build(); + Registrar registrar, ImmutableSet existingContacts, Map args) { + return RegistrarFormFields.getRegistrarContactBuilders(existingContacts, args).stream() + .map(builder -> builder.setParent(registrar).build()) + .collect(toImmutableSet()); } /** @@ -459,8 +452,17 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA () -> new FormException( "Not allowed to set registry lock password directly on new contact")); - if (!existingContact.isAllowedToSetRegistryLockPassword()) { - throw new FormException("Registrar contact not allowed to set registry lock password"); + if (updatedContact.isRegistryLockAllowed()) { + // the password must have been set before or the user was allowed to set it now + if (!existingContact.isAllowedToSetRegistryLockPassword() + && !existingContact.isRegistryLockAllowed()) { + throw new FormException("Registrar contact not allowed to set registry lock password"); + } + } + if (updatedContact.isAllowedToSetRegistryLockPassword()) { + if (!existingContact.isAllowedToSetRegistryLockPassword()) { + throw new FormException("Cannot set isAllowedToSetRegistryLockPassword through UI"); + } } } } diff --git a/core/src/test/java/google/registry/ui/server/registrar/ContactSettingsTest.java b/core/src/test/java/google/registry/ui/server/registrar/ContactSettingsTest.java index 95b8b9ccf..5e7329717 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/ContactSettingsTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/ContactSettingsTest.java @@ -188,6 +188,33 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { @Test public void testSuccess_setRegistryLockPassword() { + addPasswordToTechContact(); + techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH)); + assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue(); + assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS"); + } + + @Test + public void testSuccess_setRegistryLockPassword_notOverriddenLater() { + addPasswordToTechContact(); + techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH)); + assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue(); + + techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH)); + Map techContactMap = techContact.toJsonMap(); + techContactMap.put("name", "Some Other Name"); + Map reqJson = loadRegistrar(CLIENT_ID).toJsonMap(); + reqJson.put( + "contacts", + ImmutableList.of(AppEngineRule.makeRegistrarContact2().toJsonMap(), techContactMap)); + Map response = + action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson)); + assertThat(response).containsAtLeastEntriesIn(ImmutableMap.of("status", "SUCCESS")); + techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH)); + assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue(); + } + + private void addPasswordToTechContact() { techContact = persistResource(techContact.asBuilder().setAllowedToSetRegistryLockPassword(true).build()); Map contactMap = techContact.toJsonMap(); @@ -199,9 +226,6 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { Map response = action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson)); assertThat(response).containsAtLeastEntriesIn(ImmutableMap.of("status", "SUCCESS")); - techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH)); - assertThat(techContact.verifyRegistryLockPassword("hi")).isTrue(); - assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS"); } @Test @@ -275,7 +299,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { "results", ImmutableList.of(), "message", - "Registrar contact not allowed to set registry lock password"); + "Cannot set isAllowedToSetRegistryLockPassword through UI"); assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormException"); } }