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 040e4a2e1..5d177672a 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 @@ -391,7 +391,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA * @throws FormException if the checks fail. */ void checkContactRequirements( - Set existingContacts, Set updatedContacts) { + ImmutableSet existingContacts, + ImmutableSet updatedContacts) { // Check that no two contacts use the same email address. Set emails = new HashSet<>(); for (RegistrarContact contact : updatedContacts) { @@ -435,7 +436,12 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA throw new ContactRequirementException( "An abuse contact visible in domain WHOIS query must be designated"); } + checkContactRegistryLockRequirements(existingContacts, updatedContacts); + } + private static void checkContactRegistryLockRequirements( + ImmutableSet existingContacts, + ImmutableSet updatedContacts) { // Any contact(s) with new passwords must be allowed to set them for (RegistrarContact updatedContact : updatedContacts) { if (updatedContact.isRegistryLockAllowed() @@ -463,6 +469,31 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA } } } + + // Any previously-existing contacts with registry lock enabled cannot be deleted + existingContacts.stream() + .filter(RegistrarContact::isRegistryLockAllowed) + .forEach( + contact -> { + Optional updatedContactOptional = + updatedContacts.stream() + .filter( + updatedContact -> + updatedContact.getEmailAddress().equals(contact.getEmailAddress())) + .findFirst(); + if (!updatedContactOptional.isPresent()) { + throw new FormException( + String.format( + "Cannot delete the contact %s that has registry lock enabled", + contact.getEmailAddress())); + } + if (!updatedContactOptional.get().isRegistryLockAllowed()) { + throw new FormException( + String.format( + "Cannot remove the ability to use registry lock on the contact %s", + contact.getEmailAddress())); + } + }); } /** diff --git a/core/src/main/resources/google/registry/ui/soy/registrar/ContactSettings.soy b/core/src/main/resources/google/registry/ui/soy/registrar/ContactSettings.soy index 4d8f32f79..ac0f6412c 100644 --- a/core/src/main/resources/google/registry/ui/soy/registrar/ContactSettings.soy +++ b/core/src/main/resources/google/registry/ui/soy/registrar/ContactSettings.soy @@ -145,6 +145,7 @@ {param label: 'Email' /} {param namePrefix: $namePrefix /} {param name: 'emailAddress' /} + {param disabled: not $readonly and $item['emailAddress'] != null /} {/call} {call registry.soy.forms.inputFieldRow data="all"} {param label: 'Phone' /} 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 5b363e411..9a43b05b2 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 @@ -66,13 +66,12 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { @Test public void testPost_updateContacts_success() throws Exception { - // Remove all the contacts but the first by updating with list of - // just it. + // Remove all the contacts but one by updating with a list of just it ImmutableMap adminContact1 = ImmutableMap.of( - "name", "contact1", - "emailAddress", "contact1@email.com", - "phoneNumber", "+1.2125650001", + "name", "Marla Singer", + "emailAddress", "Marla.Singer@crr.com", + "phoneNumber", "+1.2128675309", // Have to keep ADMIN or else expect FormException for at-least-one. "types", "ADMIN,TECH"); @@ -83,15 +82,12 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", regMap)); assertThat(response).containsEntry("status", "SUCCESS"); - RegistrarContact newContact = - new RegistrarContact.Builder() - .setParent(registrar) - .setName(adminContact1.get("name")) - .setEmailAddress(adminContact1.get("emailAddress")) - .setPhoneNumber(adminContact1.get("phoneNumber")) - .setTypes(ImmutableList.of(Type.ADMIN, Type.TECH)) - .build(); - assertThat(loadRegistrar(CLIENT_ID).getContacts()).containsExactly(newContact); + RegistrarContact foundContact = + Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContacts()); + assertThat(foundContact.getName()).isEqualTo(adminContact1.get("name")); + assertThat(foundContact.getEmailAddress()).isEqualTo(adminContact1.get("emailAddress")); + assertThat(foundContact.getPhoneNumber()).isEqualTo(adminContact1.get("phoneNumber")); + assertThat(foundContact.getTypes()).containsExactly(Type.ADMIN, Type.TECH); assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS"); verifyNotificationEmailsSent(); } @@ -188,41 +184,65 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { @Test public void testSuccess_setRegistryLockPassword() { - addPasswordToTechContact(); - techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH)); - assertThat(techContact.verifyRegistryLockPassword("hellothere")).isTrue(); + addPasswordToContactTwo(); + String emailAddress = AppEngineRule.makeRegistrarContact2().getEmailAddress(); + RegistrarContact newContactWithPassword = + loadRegistrar(CLIENT_ID).getContacts().stream() + .filter(rc -> rc.getEmailAddress().equals(emailAddress)) + .findFirst() + .get(); + assertThat(newContactWithPassword.verifyRegistryLockPassword("hellothere")).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("hellothere")).isTrue(); + addPasswordToContactTwo(); + String emailAddress = AppEngineRule.makeRegistrarContact2().getEmailAddress(); + RegistrarContact newContactWithPassword = + loadRegistrar(CLIENT_ID).getContacts().stream() + .filter(rc -> rc.getEmailAddress().equals(emailAddress)) + .findFirst() + .get(); + assertThat(newContactWithPassword.verifyRegistryLockPassword("hellothere")).isTrue(); - techContact = Iterables.getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(Type.TECH)); - Map techContactMap = techContact.toJsonMap(); - techContactMap.put("name", "Some Other Name"); + Map newContactMap = newContactWithPassword.toJsonMap(); + newContactMap.put("name", "Some Other Name"); Map reqJson = loadRegistrar(CLIENT_ID).toJsonMap(); reqJson.put( "contacts", - ImmutableList.of(AppEngineRule.makeRegistrarContact2().toJsonMap(), techContactMap)); + ImmutableList.of( + AppEngineRule.makeRegistrarContact1().toJsonMap(), + newContactMap, + AppEngineRule.makeRegistrarContact3().toJsonMap())); 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("hellothere")).isTrue(); + newContactWithPassword = + loadRegistrar(CLIENT_ID).getContacts().stream() + .filter(rc -> rc.getEmailAddress().equals(emailAddress)) + .findFirst() + .get(); + assertThat(newContactWithPassword.verifyRegistryLockPassword("hellothere")).isTrue(); } - private void addPasswordToTechContact() { - techContact = - persistResource(techContact.asBuilder().setAllowedToSetRegistryLockPassword(true).build()); - Map contactMap = techContact.toJsonMap(); + private void addPasswordToContactTwo() { + RegistrarContact contact = + persistResource( + AppEngineRule.makeRegistrarContact2() + .asBuilder() + .setRegistryLockEmailAddress("johndoe@theregistrar.com") + .setAllowedToSetRegistryLockPassword(true) + .build()); + Map contactMap = contact.toJsonMap(); contactMap.put("registryLockPassword", "hellothere"); Map reqJson = loadRegistrar(CLIENT_ID).toJsonMap(); reqJson.put( "contacts", - ImmutableList.of(AppEngineRule.makeRegistrarContact2().toJsonMap(), contactMap)); + ImmutableList.of( + AppEngineRule.makeRegistrarContact1().toJsonMap(), + contactMap, + AppEngineRule.makeRegistrarContact3().toJsonMap())); Map response = action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson)); assertThat(response).containsAtLeastEntriesIn(ImmutableMap.of("status", "SUCCESS")); @@ -260,13 +280,16 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { public void testPost_failure_setRegistryLockPassword_notAllowed() { // "allowedToSetRegistryLockPassword" must be set through the back end first // before we can set a password through the UI - Map contactMap = - techContact.asBuilder().setAllowedToSetRegistryLockPassword(true).build().toJsonMap(); + Map contactMap = AppEngineRule.makeRegistrarContact2().toJsonMap(); + contactMap.put("allowedToSetRegistryLockPassword", true); contactMap.put("registryLockPassword", "hellothere"); Map reqJson = loadRegistrar(CLIENT_ID).toJsonMap(); reqJson.put( "contacts", - ImmutableList.of(AppEngineRule.makeRegistrarContact2().toJsonMap(), contactMap)); + ImmutableList.of( + AppEngineRule.makeRegistrarContact1().toJsonMap(), + contactMap, + AppEngineRule.makeRegistrarContact3().toJsonMap())); Map response = action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson)); @@ -304,6 +327,30 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormException"); } + @Test + public void testPost_failure_removingRegistryLockContact() { + ImmutableMap contact = + ImmutableMap.of( + "name", "contact1", + "emailAddress", "contact1@email.com", + "phoneNumber", "+1.2125650001", + // Have to keep ADMIN or else expect FormException for at-least-one. + "types", "ADMIN,TECH"); + Map regMap = loadRegistrar(CLIENT_ID).toJsonMap(); + regMap.put("contacts", ImmutableList.of(contact)); + Map response = + action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", regMap)); + assertThat(response) + .containsExactly( + "status", + "ERROR", + "results", + ImmutableList.of(), + "message", + "Cannot delete the contact Marla.Singer@crr.com that has registry lock enabled"); + assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormException"); + } + @Test public void testPost_failure_setRegistryLock_passwordTooShort() { techContact = diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index f4e8c23ac..08447bcc9 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -14,6 +14,7 @@ package google.registry.ui.server.registrar; +import static com.google.appengine.repackaged.com.google.common.collect.Iterables.getOnlyElement; import static com.google.monitoring.metrics.contrib.LongMetricSubject.assertThat; import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailAddress; import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDisplayName; @@ -23,14 +24,12 @@ import static google.registry.security.JsonHttpTestUtils.createJsonPayload; import static google.registry.testing.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.disallowRegistrarAccess; import static google.registry.testing.DatastoreHelper.loadRegistrar; -import static google.registry.testing.DatastoreHelper.persistResource; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.truth.Truth; import google.registry.model.ofy.Ofy; @@ -95,21 +94,13 @@ public abstract class RegistrarSettingsActionTestCase { // Add a technical contact to the registrar (in addition to the default admin contact created by // AppEngineRule). techContact = - persistResource( - new RegistrarContact.Builder() - .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()); + getOnlyElement(loadRegistrar(CLIENT_ID).getContactsOfType(RegistrarContact.Type.TECH)); action.registrarAccessor = null; action.appEngineServiceUtils = appEngineServiceUtils; when(appEngineServiceUtils.getCurrentVersionHostname("backend")).thenReturn("backend.hostname"); - action.jsonActionRunner = new JsonActionRunner( - ImmutableMap.of(), new JsonResponse(new ResponseImpl(rsp))); + action.jsonActionRunner = + new JsonActionRunner(ImmutableMap.of(), new JsonResponse(new ResponseImpl(rsp))); action.sendEmailUtils = new SendEmailUtils( getGSuiteOutgoingEmailAddress(), diff --git a/core/src/test/resources/google/registry/ui/server/registrar/update_registrar.json b/core/src/test/resources/google/registry/ui/server/registrar/update_registrar.json index c72f2dbcc..dc6002610 100644 --- a/core/src/test/resources/google/registry/ui/server/registrar/update_registrar.json +++ b/core/src/test/resources/google/registry/ui/server/registrar/update_registrar.json @@ -18,6 +18,17 @@ "emailAddress": "etphonehome@example.com", "gaeUserId": null, "types": "ADMIN,BILLING,TECH,WHOIS" + }, + { + "visibleInWhoisAsAdmin": false, + "faxNumber": null, + "phoneNumber": "+1.2128675309", + "name": "Marla Singer", + "visibleInWhoisAsTech": false, + "emailAddress": "Marla.Singer@crr.com", + "registryLockEmailAddress": "Marla.Singer.RegistryLock@crr.com", + "gaeUserId": "12345", + "types": "TECH" } ], "allowedTlds": [ diff --git a/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt b/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt index 3a78b4b7f..265fc0361 100644 --- a/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt +++ b/core/src/test/resources/google/registry/ui/server/registrar/update_registrar_email.txt @@ -13,8 +13,7 @@ contacts: ADDED: {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, 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} + {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} FINAL CONTENTS: - {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} + {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}, + {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} diff --git a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_page.png b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_page.png index 477b79275..bc4d36aa6 100644 Binary files a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_page.png and b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_page.png differ diff --git a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_alreadySet_page.png b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_alreadySet_page.png index 903d77657..5bcde0314 100644 Binary files a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_alreadySet_page.png and b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_alreadySet_page.png differ diff --git a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_notAllowedForContact_page.png b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_notAllowedForContact_page.png index 477b79275..bc4d36aa6 100644 Binary files a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_notAllowedForContact_page.png and b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_notAllowedForContact_page.png differ diff --git a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_page.png b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_page.png index 4d74c0f30..588e32144 100644 Binary files a/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_page.png and b/core/src/test/resources/google/registry/webdriver/goldens/chrome-linux/RegistrarConsoleScreenshotTest_settingsContactEdit_setRegistryLockPassword_page.png differ