From 0f1f418034bf473e7f98edf9b8a66236d6a72c93 Mon Sep 17 00:00:00 2001 From: guyben Date: Thu, 18 Oct 2018 10:42:50 -0700 Subject: [PATCH] Add registrar contact to OTE registrars When creating the various registrar objects in Sandbox for OTE, we also give access to all the registrars' data to a given google account (identified by the email) This email has to belong to the registry's G-Suite account, just like in the registrar_contact command. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217728407 --- .../tools/RegistrarContactCommand.java | 12 +-- .../registry/tools/SetupOteCommand.java | 70 +++++++++++++++-- .../registry/tools/SetupOteCommandTest.java | 78 ++++++++++++++++++- 3 files changed, 146 insertions(+), 14 deletions(-) diff --git a/java/google/registry/tools/RegistrarContactCommand.java b/java/google/registry/tools/RegistrarContactCommand.java index 89b5fc826..411d1bf10 100644 --- a/java/google/registry/tools/RegistrarContactCommand.java +++ b/java/google/registry/tools/RegistrarContactCommand.java @@ -57,19 +57,19 @@ final class RegistrarContactCommand extends MutatingCommand { @Parameter( description = "Client identifier of the registrar account.", required = true) - private List mainParameters; + List mainParameters; @Parameter( names = "--mode", description = "Type of operation you want to perform (LIST, CREATE, UPDATE, or DELETE).", required = true) - private Mode mode; + Mode mode; @Nullable @Parameter( names = "--name", description = "Contact name.") - private String name; + String name; @Nullable @Parameter( @@ -82,7 +82,7 @@ final class RegistrarContactCommand extends MutatingCommand { @Parameter( names = "--email", description = "Contact email address.") - private String email; + String email; @Nullable @Parameter( @@ -105,7 +105,7 @@ final class RegistrarContactCommand extends MutatingCommand { names = "--allow_console_access", description = "Enable or disable access to the registrar console for this contact.", arity = 1) - private Boolean allowConsoleAccess; + Boolean allowConsoleAccess; @Nullable @Parameter( @@ -138,7 +138,7 @@ final class RegistrarContactCommand extends MutatingCommand { validateWith = PathParameter.OutputFile.class) private Path output = Paths.get("/dev/stdout"); - private enum Mode { LIST, CREATE, UPDATE, DELETE } + enum Mode { LIST, CREATE, UPDATE, DELETE } private static final ImmutableSet MODES_REQUIRING_CONTACT_SYNC = ImmutableSet.of(Mode.CREATE, Mode.UPDATE, Mode.DELETE); diff --git a/java/google/registry/tools/SetupOteCommand.java b/java/google/registry/tools/SetupOteCommand.java index bc6b34d9e..af0d34045 100644 --- a/java/google/registry/tools/SetupOteCommand.java +++ b/java/google/registry/tools/SetupOteCommand.java @@ -15,15 +15,20 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; +import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.tools.CommandUtilities.promptForYes; import static google.registry.util.X509Utils.loadCertificate; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSortedMap; import com.google.re2j.Pattern; import google.registry.config.RegistryEnvironment; +import google.registry.model.common.GaeUserIdConverter; import google.registry.model.registrar.Registrar; import google.registry.model.registry.Registry.TldState; import google.registry.tools.params.PathParameter; @@ -53,6 +58,10 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo private static final Duration SHORT_REDEMPTION_GRACE_PERIOD = Duration.standardMinutes(10); private static final Duration SHORT_PENDING_DELETE_LENGTH = Duration.standardMinutes(5); + // Whether to prompt the user on command failures. Set to false for testing of these failures. + @VisibleForTesting + static boolean interactive = true; + private static final ImmutableSortedMap EAP_FEE_SCHEDULE = ImmutableSortedMap.of( new DateTime(0), @@ -87,6 +96,14 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo ) private List ipWhitelist = new ArrayList<>(); + @Parameter( + names = {"--email"}, + description = + "the registrar's account to use for console access. " + + "Must be on the registry's G-Suite domain.", + required = true) + private String email; + @Parameter( names = {"-c", "--certfile"}, description = "full path to cert file in PEM format (best if on local storage)", @@ -130,6 +147,21 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo */ private int roidSuffixCounter = 0; + /** Runs a command, clearing the cache before and prompting the user on failures. */ + private void runCommand(Command command) { + ofy().clearSessionCache(); + try { + command.run(); + } catch (Exception e) { + System.err.format("Command failed with error %s\n", e); + if (interactive && promptForYes("Continue to next command?")) { + return; + } + Throwables.throwIfUnchecked(e); + throw new RuntimeException(e); + } + } + /** Constructs and runs a CreateTldCommand. */ private void createTld( String tldName, @@ -137,8 +169,7 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo Duration addGracePeriod, Duration redemptionGracePeriod, Duration pendingDeleteLength, - boolean isEarlyAccess) - throws Exception { + boolean isEarlyAccess) { CreateTldCommand command = new CreateTldCommand(); command.addGracePeriod = addGracePeriod; command.dnsWriters = dnsWriters; @@ -158,11 +189,11 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo if (isEarlyAccess) { command.eapFeeSchedule = EAP_FEE_SCHEDULE; } - command.run(); + runCommand(command); } /** Constructs and runs a CreateRegistrarCommand */ - private void createRegistrar(String registrarName, String password, String tld) throws Exception { + private void createRegistrar(String registrarName, String password, String tld) { CreateRegistrarCommand command = new CreateRegistrarCommand(); command.mainParameters = ImmutableList.of(registrarName); command.createGoogleGroups = false; // Don't create Google Groups for OT&E registrars. @@ -183,7 +214,19 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo command.phone = Optional.of("+1.2125550100"); command.icannReferralEmail = "nightmare@registrar.test"; command.force = force; - command.run(); + runCommand(command); + } + + /** Constructs and runs a RegistrarContactCommand */ + private void createRegistrarContact(String registrarName) { + RegistrarContactCommand command = new RegistrarContactCommand(); + command.mainParameters = ImmutableList.of(registrarName); + command.mode = RegistrarContactCommand.Mode.CREATE; + command.name = email; + command.email = email; + command.allowConsoleAccess = true; + command.force = force; + runCommand(command); } /** Run any pre-execute command checks */ @@ -193,6 +236,14 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo REGISTRAR_PATTERN.matcher(registrar).matches(), "Registrar name is invalid (see usage text for requirements)."); + // Make sure the email is "correct" - as in it's a valid email we can convert to gaeId + // There's no need to look at the result - it'll be converted again inside + // RegistrarContactCommand. + checkNotNull( + GaeUserIdConverter.convertEmailAddressToGaeUserId(email), + "Email address %s is not associated with any GAE ID", + email); + boolean warned = false; if (RegistryEnvironment.get() != RegistryEnvironment.SANDBOX && RegistryEnvironment.get() != RegistryEnvironment.UNITTEST) { @@ -227,7 +278,9 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo return "Creating TLD:\n" + " " + registrar + "-eap\n" + "Creating registrar:\n" - + " " + registrar + "-5 (access to TLD " + registrar + "-eap)"; + + " " + registrar + "-5 (access to TLD " + registrar + "-eap)\n" + + "Giving contact access to this registrar:\n" + + " " + email; } else { return "Creating TLDs:\n" + " " + registrar + "-sunrise\n" @@ -239,7 +292,9 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo + " " + registrar + "-2 (access to TLD " + registrar + "-landrush)\n" + " " + registrar + "-3 (access to TLD " + registrar + "-ga)\n" + " " + registrar + "-4 (access to TLD " + registrar + "-ga)\n" - + " " + registrar + "-5 (access to TLD " + registrar + "-eap)"; + + " " + registrar + "-5 (access to TLD " + registrar + "-eap)\n" + + "Giving contact access to these registrars:\n" + + " " + email; } } @@ -297,6 +352,7 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo for (List r : registrars) { createRegistrar(r.get(0), r.get(1), r.get(2)); + createRegistrarContact(r.get(0)); } StringBuilder output = new StringBuilder(); diff --git a/javatests/google/registry/tools/SetupOteCommandTest.java b/javatests/google/registry/tools/SetupOteCommandTest.java index 8ecdf7eb8..dc403172b 100644 --- a/javatests/google/registry/tools/SetupOteCommandTest.java +++ b/javatests/google/registry/tools/SetupOteCommandTest.java @@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import google.registry.model.registrar.Registrar; +import google.registry.model.registrar.RegistrarContact; import google.registry.model.registry.Registry; import google.registry.model.registry.Registry.TldState; import google.registry.testing.DeterministicStringGenerator; @@ -57,6 +58,7 @@ public class SetupOteCommandTest extends CommandTestCase { @Before public void init() { + SetupOteCommand.interactive = false; command.validDnsWriterNames = ImmutableSet.of("FooDnsWriter", "BarDnsWriter", "VoidDnsWriter"); command.passwordGenerator = passwordGenerator; persistPremiumList("default_sandbox_list", "sandbox,USD 1000"); @@ -144,11 +146,22 @@ public class SetupOteCommandTest extends CommandTestCase { verifyRegistrarCreation(registrarName, allowedTld, password, ipWhitelist, false); } + private void verifyRegistrarContactCreation(String registrarName, String email) { + ImmutableSet registrarContacts = + loadRegistrar(registrarName).getContacts(); + assertThat(registrarContacts).hasSize(1); + RegistrarContact registrarContact = registrarContacts.stream().findAny().get(); + assertThat(registrarContact.getEmailAddress()).isEqualTo(email); + assertThat(registrarContact.getName()).isEqualTo(email); + assertThat(registrarContact.getGaeUserId()).isNotNull(); + } + @Test public void testSuccess() throws Exception { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename()); @@ -189,6 +202,12 @@ public class SetupOteCommandTest extends CommandTestCase { verifyRegistrarCreation("blobio-3", "blobio-ga", passwords.get(2), ipAddress); verifyRegistrarCreation("blobio-4", "blobio-ga", passwords.get(3), ipAddress); verifyRegistrarCreation("blobio-5", "blobio-eap", passwords.get(4), ipAddress); + + verifyRegistrarContactCreation("blobio-1", "contact@email.com"); + verifyRegistrarContactCreation("blobio-2", "contact@email.com"); + verifyRegistrarContactCreation("blobio-3", "contact@email.com"); + verifyRegistrarContactCreation("blobio-4", "contact@email.com"); + verifyRegistrarContactCreation("blobio-5", "contact@email.com"); } @Test @@ -196,6 +215,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=abc", + "--email=abc@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename()); @@ -236,6 +256,12 @@ public class SetupOteCommandTest extends CommandTestCase { verifyRegistrarCreation("abc-3", "abc-ga", passwords.get(2), ipAddress); verifyRegistrarCreation("abc-4", "abc-ga", passwords.get(3), ipAddress); verifyRegistrarCreation("abc-5", "abc-eap", passwords.get(4), ipAddress); + + verifyRegistrarContactCreation("abc-1", "abc@email.com"); + verifyRegistrarContactCreation("abc-2", "abc@email.com"); + verifyRegistrarContactCreation("abc-3", "abc@email.com"); + verifyRegistrarContactCreation("abc-4", "abc@email.com"); + verifyRegistrarContactCreation("abc-5", "abc@email.com"); } @Test @@ -244,6 +270,7 @@ public class SetupOteCommandTest extends CommandTestCase { "--eap_only", "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certhash=" + SAMPLE_CERT_HASH); @@ -262,6 +289,8 @@ public class SetupOteCommandTest extends CommandTestCase { ImmutableList.of(CidrAddressBlock.create("1.1.1.1")); verifyRegistrarCreation("blobio-5", "blobio-eap", passwords.get(0), ipAddress, true); + + verifyRegistrarContactCreation("blobio-5", "contact@email.com"); } @Test @@ -270,6 +299,7 @@ public class SetupOteCommandTest extends CommandTestCase { "--eap_only", "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename()); @@ -288,6 +318,8 @@ public class SetupOteCommandTest extends CommandTestCase { CidrAddressBlock.create("1.1.1.1")); verifyRegistrarCreation("blobio-5", "blobio-eap", passwords.get(0), ipAddress); + + verifyRegistrarContactCreation("blobio-5", "contact@email.com"); } @Test @@ -295,6 +327,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1,2.2.2.2", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=FooDnsWriter", "--certfile=" + getCertFilename()); @@ -336,6 +369,12 @@ public class SetupOteCommandTest extends CommandTestCase { verifyRegistrarCreation("blobio-3", "blobio-ga", passwords.get(2), ipAddresses); verifyRegistrarCreation("blobio-4", "blobio-ga", passwords.get(3), ipAddresses); verifyRegistrarCreation("blobio-5", "blobio-eap", passwords.get(4), ipAddresses); + + verifyRegistrarContactCreation("blobio-1", "contact@email.com"); + verifyRegistrarContactCreation("blobio-2", "contact@email.com"); + verifyRegistrarContactCreation("blobio-3", "contact@email.com"); + verifyRegistrarContactCreation("blobio-4", "contact@email.com"); + verifyRegistrarContactCreation("blobio-5", "contact@email.com"); } @Test @@ -343,6 +382,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--certfile=" + getCertFilename(), "--dns_writers=BarDnsWriter", "--premium_list=alternate_list"); @@ -384,6 +424,12 @@ public class SetupOteCommandTest extends CommandTestCase { verifyRegistrarCreation("blobio-3", "blobio-ga", passwords.get(2), ipAddress); verifyRegistrarCreation("blobio-4", "blobio-ga", passwords.get(3), ipAddress); verifyRegistrarCreation("blobio-5", "blobio-eap", passwords.get(4), ipAddress); + + verifyRegistrarContactCreation("blobio-1", "contact@email.com"); + verifyRegistrarContactCreation("blobio-2", "contact@email.com"); + verifyRegistrarContactCreation("blobio-3", "contact@email.com"); + verifyRegistrarContactCreation("blobio-4", "contact@email.com"); + verifyRegistrarContactCreation("blobio-5", "contact@email.com"); } @Test @@ -394,6 +440,7 @@ public class SetupOteCommandTest extends CommandTestCase { () -> runCommandForced( "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("option is required: -w, --ip_whitelist"); @@ -407,6 +454,7 @@ public class SetupOteCommandTest extends CommandTestCase { () -> runCommandForced( "--ip_whitelist=1.1.1.1", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("option is required: -r, --registrar"); @@ -419,7 +467,10 @@ public class SetupOteCommandTest extends CommandTestCase { IllegalArgumentException.class, () -> runCommandForced( - "--ip_whitelist=1.1.1.1", "--dns_writers=VoidDnsWriter", "--registrar=blobio")); + "--ip_whitelist=1.1.1.1", + "--email=contact@email.com", + "--dns_writers=VoidDnsWriter", + "--registrar=blobio")); assertThat(thrown) .hasMessageThat() .contains( @@ -434,6 +485,7 @@ public class SetupOteCommandTest extends CommandTestCase { () -> runCommandForced( "--ip_whitelist=1.1.1.1", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--registrar=blobio", "--certfile=" + getCertFilename(), @@ -452,11 +504,26 @@ public class SetupOteCommandTest extends CommandTestCase { () -> runCommandForced( "--ip_whitelist=1.1.1.1", + "--email=contact@email.com", "--certfile=" + getCertFilename(), "--registrar=blobio")); assertThat(thrown).hasMessageThat().contains("option is required: --dns_writers"); } + @Test + public void testFailure_missingEmail() { + ParameterException thrown = + assertThrows( + ParameterException.class, + () -> + runCommandForced( + "--ip_whitelist=1.1.1.1", + "--dns_writers=VoidDnsWriter", + "--certfile=" + getCertFilename(), + "--registrar=blobio")); + assertThat(thrown).hasMessageThat().contains("option is required: --email"); + } + @Test public void testFailure_invalidCert() { CertificateParsingException thrown = @@ -466,6 +533,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=/dev/null")); assertThat(thrown).hasMessageThat().contains("No X509Certificate found"); @@ -480,6 +548,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=3blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Registrar name is invalid"); @@ -494,6 +563,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=InvalidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown) @@ -510,6 +580,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=bl", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Registrar name is invalid"); @@ -524,6 +595,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobiotoooolong", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Registrar name is invalid"); @@ -538,6 +610,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blo#bio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Registrar name is invalid"); @@ -552,6 +625,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename(), "--premium_list=foo")); @@ -568,6 +642,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("TLD 'blobio-sunrise' already exists"); @@ -587,6 +662,7 @@ public class SetupOteCommandTest extends CommandTestCase { runCommandForced( "--ip_whitelist=1.1.1.1", "--registrar=blobio", + "--email=contact@email.com", "--dns_writers=VoidDnsWriter", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Registrar blobio-1 already exists");