Include the registry lock email in the JS object as a sensitive field (#658)

* Include the registry lock email in the JS object as a sensitive field

* Change wording of exceptions to be more consistent
This commit is contained in:
gbrodman 2020-07-01 13:05:21 -04:00 committed by GitHub
parent e20eb17458
commit 74d0cdce5b
6 changed files with 85 additions and 17 deletions

View file

@ -454,7 +454,13 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
.orElseThrow(
() ->
new FormException(
"Not allowed to set registry lock password directly on new contact"));
"Cannot set registry lock password directly on new contact"));
// Can't modify registry lock email address
if (!Objects.equals(
updatedContact.getRegistryLockEmailAddress(),
existingContact.getRegistryLockEmailAddress())) {
throw new FormException("Cannot modify registryLockEmailAddress through the UI");
}
if (updatedContact.isRegistryLockAllowed()) {
// the password must have been set before or the user was allowed to set it now
if (!existingContact.isAllowedToSetRegistryLockPassword()
@ -464,7 +470,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
}
if (updatedContact.isAllowedToSetRegistryLockPassword()) {
if (!existingContact.isAllowedToSetRegistryLockPassword()) {
throw new FormException("Cannot set isAllowedToSetRegistryLockPassword through UI");
throw new FormException(
"Cannot modify isAllowedToSetRegistryLockPassword through the UI");
}
}
}

View file

@ -180,7 +180,8 @@ registry.json.RegistrarAddress;
* faxNumber: (string?|undefined),
* types: (string?|undefined),
* allowedToSetRegistryLockPassword: boolean,
* registryLockAllowed: boolean
* registryLockAllowed: boolean,
* registryLockEmailAddress: (string?|undefined)
* }}
*/
registry.json.RegistrarContact;

View file

@ -176,6 +176,10 @@
{if isNonnull($item['gaeUserId'])}
<input type="hidden" name="{$namePrefix}gaeUserId" value="{$item['gaeUserId']}">
{/if}
{if isNonnull($item['registryLockEmailAddress'])}
<input type="hidden" name="{$namePrefix}registryLockEmailAddress"
value="{$item['registryLockEmailAddress']}">
{/if}
</div>
{/template}

View file

@ -67,26 +67,28 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
@Test
public void testPost_updateContacts_success() throws Exception {
// Remove all the contacts but one by updating with a list of just it
ImmutableMap<String, String> adminContact1 =
ImmutableMap.of(
"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");
Map<String, Object> adminContact =
loadRegistrar(CLIENT_ID).getContacts().stream()
.filter(rc -> rc.getEmailAddress().equals("Marla.Singer@crr.com"))
.findFirst()
.get()
.toJsonMap();
// Keep an admin to avoid superfluous issues
adminContact.put("types", "ADMIN,TECH");
Registrar registrar = loadRegistrar(CLIENT_ID);
Map<String, Object> regMap = registrar.toJsonMap();
regMap.put("contacts", ImmutableList.of(adminContact1));
regMap.put("contacts", ImmutableList.of(adminContact));
Map<String, Object> response =
action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", regMap));
assertThat(response).containsEntry("status", "SUCCESS");
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.getName()).isEqualTo(adminContact.get("name"));
assertThat(foundContact.getEmailAddress()).isEqualTo(adminContact.get("emailAddress"));
assertThat(foundContact.getPhoneNumber()).isEqualTo(adminContact.get("phoneNumber"));
assertThat(foundContact.getTypes()).containsExactly(Type.ADMIN, Type.TECH);
assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
verifyNotificationEmailsSent();
@ -272,7 +274,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
"results",
ImmutableList.of(),
"message",
"Not allowed to set registry lock password directly on new contact");
"Cannot set registry lock password directly on new contact");
assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormException");
}
@ -323,7 +325,39 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
"results",
ImmutableList.of(),
"message",
"Cannot set isAllowedToSetRegistryLockPassword through UI");
"Cannot modify isAllowedToSetRegistryLockPassword through the UI");
assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormException");
}
@Test
public void testPost_failure_setRegistryLockEmail() {
addPasswordToContactTwo();
Map<String, Object> reqJson = loadRegistrar(CLIENT_ID).toJsonMap();
String emailAddress = AppEngineRule.makeRegistrarContact2().getEmailAddress();
RegistrarContact newContactWithPassword =
loadRegistrar(CLIENT_ID).getContacts().stream()
.filter(rc -> rc.getEmailAddress().equals(emailAddress))
.findFirst()
.get();
Map<String, Object> contactJson = newContactWithPassword.toJsonMap();
contactJson.put("registryLockEmailAddress", "bogus.email@bogus.tld");
reqJson.put(
"contacts",
ImmutableList.of(
AppEngineRule.makeRegistrarContact1().toJsonMap(),
contactJson,
AppEngineRule.makeRegistrarContact3().toJsonMap()));
Map<String, Object> response =
action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson));
assertThat(response)
.containsExactly(
"status",
"ERROR",
"results",
ImmutableList.of(),
"message",
"Cannot modify registryLockEmailAddress through the UI");
assertMetric(CLIENT_ID, "update", "[OWNER]", "ERROR: FormException");
}

View file

@ -14,6 +14,7 @@
package google.registry.webdriver;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.server.Fixture.BASIC;
import static google.registry.server.Route.route;
import static google.registry.testing.AppEngineRule.makeRegistrar2;
@ -32,11 +33,13 @@ import com.googlecode.objectify.ObjectifyFilter;
import google.registry.model.domain.DomainBase;
import google.registry.model.ofy.OfyFilter;
import google.registry.model.registrar.Registrar.State;
import google.registry.model.registrar.RegistrarContact;
import google.registry.module.frontend.FrontendServlet;
import google.registry.schema.domain.RegistryLock;
import google.registry.server.RegistryTestServer;
import google.registry.testing.AppEngineRule;
import google.registry.testing.CertificateSamples;
import java.util.Optional;
import java.util.UUID;
import org.junit.Rule;
import org.junit.Test;
@ -150,13 +153,13 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
public void settingsContactEdit_setRegistryLockPassword() throws Throwable {
server.runInAppEngineEnvironment(
() -> {
persistResource(makeRegistrar2().asBuilder().setRegistryLockAllowed(true).build());
persistResource(
makeRegistrarContact2()
.asBuilder()
.setRegistryLockEmailAddress("johndoe.registrylock@example.com")
.setAllowedToSetRegistryLockPassword(true)
.build());
persistResource(makeRegistrar2().asBuilder().setRegistryLockAllowed(true).build());
return null;
});
driver.manage().window().setSize(new Dimension(1050, 2000));
@ -165,6 +168,25 @@ public class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
driver.waitForElement(By.tagName("h1"));
driver.waitForElement(By.id("reg-app-btn-edit")).click();
driver.diffPage("page");
// now actually set the password
driver.findElement(By.id("contacts[1].registryLockPassword")).sendKeys("password");
driver.waitForElement(By.id("reg-app-btn-save")).click();
Thread.sleep(500);
driver.diffPage("contactview");
server.runInAppEngineEnvironment(
() -> {
RegistrarContact contact =
loadRegistrar("TheRegistrar").getContacts().stream()
.filter(c -> c.getEmailAddress().equals("johndoe@theregistrar.com"))
.findFirst()
.get();
assertThat(contact.verifyRegistryLockPassword("password")).isTrue();
assertThat(contact.getRegistryLockEmailAddress())
.isEqualTo(Optional.of("johndoe.registrylock@example.com"));
return null;
});
}
@Test