Rationalize OT&E client ID validation rules

This makes the validation rules much simpler, thus placing less cognitive load on the users of the console and nomulus tool.  The changes are:

1. Don't allow hyphens. No real registrars use hyphens in their client IDs, and it's better to reserve these solely as the delimiter between the base client ID and the number representing the environment.
2. Allow the first character to be a number.  This has affected multiple real registrars, causing their OT&E and production client IDs to be different.  There's no reason for this restriction, as the only reason motivating it was that there are no TLDs that start with a number.  However, the OT&E TLDs are created only in sandbox and never have DNS syncing enabled, so this restriction is purposeless.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229187198
This commit is contained in:
mcilwain 2019-01-14 08:27:52 -08:00 committed by Ben McIlwain
parent fd8a18b72e
commit 8ac8ecf8f6
6 changed files with 37 additions and 50 deletions

View file

@ -72,8 +72,18 @@ import org.joda.time.Duration;
*/
public final class OteAccountBuilder {
// Regex: 3-14 lower-case alphanumeric characters or hyphens, the first of which must be a letter.
private static final Pattern REGISTRAR_PATTERN = Pattern.compile("^[a-z][-a-z0-9]{2,13}$");
/**
* Validation regex for registrar base client IDs (3-14 lowercase alphanumeric characters).
*
* <p>The base client ID is appended with numbers to create four different test registrar accounts
* (e.g. reg-1, reg-3, reg-4, reg-5). Registrar client IDs are of type clIDType in eppcom.xsd
* which is limited to 16 characters, hence the limit of 14 here to account for the dash and
* numbers.
*
* <p>The base client ID is also used to generate the OT&E TLDs, hence the restriction to
* lowercase alphanumeric characters.
*/
private static final Pattern REGISTRAR_PATTERN = Pattern.compile("^[a-z0-9]{3,14}$");
// Durations are short so that registrars can test with quick transfer (etc.) turnaround.
private static final Duration SHORT_ADD_GRACE_PERIOD = Duration.standardMinutes(60);

View file

@ -40,51 +40,41 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo
private static final int PASSWORD_LENGTH = 16;
@Parameter(
names = {"-r", "--registrar"},
description =
"must 1) consist of only lowercase letters, numbers, or hyphens, "
+ "2) start with a letter, and 3) be between 3 and 14 characters (inclusive). "
+ "We require 1 and 2 since the registrar name will be used to create TLDs,"
+ "and we require 3 since we append \"-[1234]\" to the name to create client"
+ "IDs which are required by the EPP XML schema to be between 3-16 chars.",
required = true
)
names = {"-r", "--registrar"},
description = "The registrar client ID, consisting of 3-14 lowercase letters and numbers.",
required = true)
private String registrar;
@Parameter(
names = {"-w", "--ip_whitelist"},
description = "comma separated list of IP addreses or CIDR ranges",
required = true
)
names = {"-w", "--ip_whitelist"},
description = "Comma-separated list of IP addreses or CIDR ranges.",
required = true)
private List<String> ipWhitelist = new ArrayList<>();
@Parameter(
names = {"--email"},
description =
"the registrar's account to use for console access. "
"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)",
validateWith = PathParameter.InputFile.class
)
names = {"-c", "--certfile"},
description = "Full path to cert file in PEM format (best if on local storage).",
validateWith = PathParameter.InputFile.class)
private Path certFile;
@Parameter(
names = {"-h", "--certhash"},
description =
"Hash of client certificate (SHA256 base64 no padding). Do not use this unless "
+ "you want to store ONLY the hash and not the full certificate"
)
names = {"-h", "--certhash"},
description =
"Hash of client certificate (SHA256 base64 no padding). Do not use this unless "
+ "you want to store ONLY the hash and not the full certificate.")
private String certHash;
@Parameter(
names = {"--overwrite"},
description = "whether to replace existing entities if we encounter any, instead of failing"
)
names = {"--overwrite"},
description = "Whether to replace existing entities if we encounter any, instead of failing.")
private boolean overwrite = false;
@Inject

View file

@ -116,15 +116,7 @@
{param required: true /}
{/call}
<span class="{css('description')}">
Must:
<ol type="1">
<li>consist of only lowercase letters, numbers, or hyphens,</li>
<li>start with a letter, and</li>
<li>be between 3 and 14 characters (inclusive).</li>
</ol>
We require 1 and 2 since the registrar name will be used to create TLDs, and we
require 3 since we append "-[12345]" to the name to create client IDs which are
required by the EPP XML schema to be between 3-16 chars.
Must consist of 3-14 lower-case letters and numbers.
</span>
</td>
</tr>

View file

@ -192,9 +192,9 @@ public final class OteAccountBuilderTest {
public void testCreateOteEntities_invalidClientId_fails() {
assertThat(
assertThrows(
IllegalArgumentException.class, () -> OteAccountBuilder.forClientId("3blobio")))
IllegalArgumentException.class, () -> OteAccountBuilder.forClientId("3blo-bio")))
.hasMessageThat()
.isEqualTo("Invalid registrar name: 3blobio");
.isEqualTo("Invalid registrar name: 3blo-bio");
}
@Test
@ -319,11 +319,6 @@ public final class OteAccountBuilderTest {
assertThat(OteAccountBuilder.getBaseClientId("myclientid-4")).isEqualTo("myclientid");
}
@Test
public void testGetBaseClientId_multipleDashesValid() {
assertThat(OteAccountBuilder.getBaseClientId("two-dashes-3")).isEqualTo("two-dashes");
}
@Test
public void testGetBaseClientId_invalidInput_malformed() {
assertThat(

View file

@ -324,10 +324,10 @@ public class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
() ->
runCommandForced(
"--ip_whitelist=1.1.1.1",
"--registrar=3blobio",
"--registrar=3blo-bio",
"--email=contact@email.com",
"--certfile=" + getCertFilename()));
assertThat(thrown).hasMessageThat().contains("Invalid registrar name: 3blobio");
assertThat(thrown).hasMessageThat().contains("Invalid registrar name: 3blo-bio");
}
@Test

View file

@ -105,9 +105,9 @@ public final class OteStatusActionTest {
@Test
public void testFailure_noRegistrar() {
assertThat(action.handleJsonRequest(ImmutableMap.of("clientId", "nonexistent-id-3")))
assertThat(action.handleJsonRequest(ImmutableMap.of("clientId", "nonexistent-3")))
.containsExactlyEntriesIn(
errorResultWithMessage("TestUserId doesn't have access to registrar nonexistent-id-3"));
errorResultWithMessage("TestUserId doesn't have access to registrar nonexistent-3"));
}
@Test
@ -121,10 +121,10 @@ public final class OteStatusActionTest {
@Test
public void testFailure_malformedRegistrarName() {
assertThat(action.handleJsonRequest(ImmutableMap.of("clientId", "bad-client-id")))
assertThat(action.handleJsonRequest(ImmutableMap.of("clientId", "badclient-id")))
.containsExactlyEntriesIn(
errorResultWithMessage(
"ID bad-client-id is not one of the OT&E client IDs for base bad-client"));
"ID badclient-id is not one of the OT&E client IDs for base badclient"));
}
@Test