Use randomly-generated base-58 strings for RegistryLock verification codes (#464)

* Use randomly-generated strings for RegistryLock verification codes

We were using UUIDs before which are also fine, but unnecessarily long.
The RegistryLock class itself does not enforce any particular format for
the lock verification codes.
This commit is contained in:
gbrodman 2020-02-03 13:50:54 -05:00 committed by GitHub
parent 76d8afe856
commit c0afb9aeee
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 118 additions and 76 deletions

View file

@ -25,7 +25,6 @@ import google.registry.schema.domain.RegistryLock;
import google.registry.testing.AppEngineRule;
import google.registry.testing.FakeClock;
import java.util.Optional;
import java.util.UUID;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -201,7 +200,7 @@ public final class RegistryLockDaoTest {
.setRepoId("repoId")
.setDomainName("example.test")
.setRegistrarId("TheRegistrar")
.setVerificationCode(UUID.randomUUID().toString())
.setVerificationCode("123456789ABCDEFGHJKLMNPQRSTUVWXY")
.isSuperuser(true)
.build();
}

View file

@ -38,8 +38,10 @@ import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCo
import google.registry.schema.domain.RegistryLock;
import google.registry.testing.AppEngineRule;
import google.registry.testing.DatastoreHelper;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.testing.UserInfo;
import google.registry.util.StringGenerator.Alphabets;
import java.util.Set;
import java.util.stream.Collectors;
import org.joda.time.Duration;
@ -57,6 +59,8 @@ public final class DomainLockUtilsTest {
private static final String POC_ID = "marla.singer@example.com";
private final FakeClock clock = new FakeClock();
private final DomainLockUtils domainLockUtils =
new DomainLockUtils(new DeterministicStringGenerator(Alphabets.BASE_58));
@Rule
public final AppEngineRule appEngineRule =
@ -80,39 +84,39 @@ public final class DomainLockUtilsTest {
@Test
public void testSuccess_createLock() {
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
}
@Test
public void testSuccess_createUnlock() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
DomainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
}
@Test
public void testSuccess_createUnlock_adminUnlockingAdmin() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock);
DomainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", true, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock);
domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", true, clock);
}
@Test
public void testSuccess_createLock_previousLockExpired() {
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
clock.advanceBy(Duration.standardDays(1));
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
}
@Test
public void testSuccess_applyLockDomain() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
assertThat(reloadDomain().getStatusValues()).containsExactlyElementsIn(REGISTRY_LOCK_STATUSES);
HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE);
assertThat(historyEntry.getRequestedByRegistrar()).isTrue();
@ -125,12 +129,12 @@ public final class DomainLockUtilsTest {
@Test
public void testSuccess_applyUnlockDomain() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
RegistryLock unlock =
DomainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
DomainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false, clock);
domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false, clock);
assertThat(reloadDomain().getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES);
ImmutableList<HistoryEntry> historyEntries =
@ -149,8 +153,8 @@ public final class DomainLockUtilsTest {
@Test
public void testSuccess_applyAdminLock_onlyHistoryEntry() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock);
HistoryEntry historyEntry = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_UPDATE);
assertThat(historyEntry.getRequestedByRegistrar()).isFalse();
@ -161,16 +165,16 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_createUnlock_alreadyPendingUnlock() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
DomainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
assertThat(
assertThrows(
IllegalArgumentException.class,
() ->
DomainLockUtils.createRegistryUnlockRequest(
domainLockUtils.createRegistryUnlockRequest(
DOMAIN_NAME, "TheRegistrar", false, clock)))
.hasMessageThat()
.isEqualTo("A pending unlock action already exists for example.tld");
@ -179,13 +183,13 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_createUnlock_nonAdminUnlockingAdmin() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock);
assertThat(
assertThrows(
IllegalArgumentException.class,
() ->
DomainLockUtils.createRegistryUnlockRequest(
domainLockUtils.createRegistryUnlockRequest(
DOMAIN_NAME, "TheRegistrar", false, clock)))
.hasMessageThat()
.isEqualTo("Non-admin user cannot unlock admin-locked domain example.tld");
@ -197,7 +201,7 @@ public final class DomainLockUtilsTest {
assertThrows(
IllegalArgumentException.class,
() ->
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
"asdf.tld", "TheRegistrar", POC_ID, false, clock)))
.hasMessageThat()
.isEqualTo("Unknown domain asdf.tld");
@ -205,12 +209,12 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_createLock_alreadyPendingLock() {
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
assertThat(
assertThrows(
IllegalArgumentException.class,
() ->
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock)))
.hasMessageThat()
.isEqualTo("A pending or completed lock action already exists for example.tld");
@ -223,7 +227,7 @@ public final class DomainLockUtilsTest {
assertThrows(
IllegalArgumentException.class,
() ->
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock)))
.hasMessageThat()
.isEqualTo("Domain example.tld is already locked");
@ -235,7 +239,7 @@ public final class DomainLockUtilsTest {
assertThrows(
IllegalArgumentException.class,
() ->
DomainLockUtils.createRegistryUnlockRequest(
domainLockUtils.createRegistryUnlockRequest(
DOMAIN_NAME, "TheRegistrar", false, clock)))
.hasMessageThat()
.isEqualTo("Domain example.tld is already unlocked");
@ -244,14 +248,14 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_applyLock_alreadyApplied() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
domain = reloadDomain();
assertThat(
assertThrows(
IllegalArgumentException.class,
() -> DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock)))
() -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock)))
.hasMessageThat()
.isEqualTo("Domain example.tld is already locked");
assertNoDomainChanges();
@ -260,13 +264,13 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_applyLock_expired() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
clock.advanceBy(Duration.standardDays(1));
assertThat(
assertThrows(
IllegalArgumentException.class,
() -> DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock)))
() -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, clock)))
.hasMessageThat()
.isEqualTo("The pending lock has expired; please try again");
assertNoDomainChanges();
@ -275,11 +279,11 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_applyLock_nonAdmin_applyAdminLock() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
domainLockUtils.createRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true, clock);
assertThat(
assertThrows(
IllegalArgumentException.class,
() -> DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock)))
() -> domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock)))
.hasMessageThat()
.isEqualTo("Non-admin user cannot complete admin lock");
assertNoDomainChanges();
@ -288,18 +292,18 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_applyUnlock_alreadyUnlocked() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false, clock);
RegistryLock unlock =
DomainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
DomainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false, clock);
domainLockUtils.createRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false, clock);
domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false, clock);
assertThat(
assertThrows(
IllegalArgumentException.class,
() ->
DomainLockUtils.verifyAndApplyUnlock(
domainLockUtils.verifyAndApplyUnlock(
unlock.getVerificationCode(), false, clock)))
.hasMessageThat()
.isEqualTo("Domain example.tld is already unlocked");
@ -309,7 +313,7 @@ public final class DomainLockUtilsTest {
@Test
public void testFailure_applyLock_alreadyLocked() {
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
domainLockUtils.createRegistryLockRequest(
DOMAIN_NAME, "TheRegistrar", POC_ID, false, clock);
String verificationCode = lock.getVerificationCode();
// reload to pick up modification times, etc
@ -318,7 +322,7 @@ public final class DomainLockUtilsTest {
assertThat(
assertThrows(
IllegalArgumentException.class,
() -> DomainLockUtils.verifyAndApplyLock(verificationCode, false, clock)))
() -> domainLockUtils.verifyAndApplyLock(verificationCode, false, clock)))
.hasMessageThat()
.isEqualTo("Domain example.tld is already locked");

View file

@ -30,7 +30,9 @@ import google.registry.model.registrar.Registrar.Type;
import google.registry.model.registry.RegistryLockDao;
import google.registry.persistence.transaction.JpaTestRules;
import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.util.StringGenerator.Alphabets;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
@ -51,6 +53,8 @@ public class LockDomainCommandTest extends CommandTestCase<LockDomainCommand> {
createTld("tld");
command.registryAdminClientId = "adminreg";
command.clock = new FakeClock();
command.domainLockUtils =
new DomainLockUtils(new DeterministicStringGenerator(Alphabets.BASE_58));
}
@Test

View file

@ -33,7 +33,9 @@ import google.registry.model.registry.RegistryLockDao;
import google.registry.persistence.transaction.JpaTestRules;
import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule;
import google.registry.schema.domain.RegistryLock;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.util.StringGenerator.Alphabets;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
@ -54,14 +56,16 @@ public class UnlockDomainCommandTest extends CommandTestCase<UnlockDomainCommand
createTld("tld");
command.registryAdminClientId = "adminreg";
command.clock = new FakeClock();
command.domainLockUtils =
new DomainLockUtils(new DeterministicStringGenerator(Alphabets.BASE_58));
}
private DomainBase persistLockedDomain(String domainName, String registrarId) {
DomainBase domain = persistResource(newDomainBase(domainName));
RegistryLock lock =
DomainLockUtils.createRegistryLockRequest(
command.domainLockUtils.createRegistryLockRequest(
domainName, registrarId, null, true, command.clock);
DomainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, command.clock);
command.domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true, command.clock);
return reloadResource(domain);
}

View file

@ -43,7 +43,6 @@ import google.registry.testing.FakeClock;
import google.registry.testing.FakeResponse;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Rule;
@ -97,7 +96,7 @@ public final class RegistryLockGetActionTest {
.setRepoId("repoId")
.setDomainName("example.test")
.setRegistrarId("TheRegistrar")
.setVerificationCode(UUID.randomUUID().toString())
.setVerificationCode("123456789ABCDEFGHJKLMNPQRSTUVWXY")
.setRegistrarPocId("johndoe@theregistrar.com")
.setLockCompletionTimestamp(fakeClock.nowUtc())
.build();
@ -107,7 +106,7 @@ public final class RegistryLockGetActionTest {
.setRepoId("repoId")
.setDomainName("adminexample.test")
.setRegistrarId("TheRegistrar")
.setVerificationCode(UUID.randomUUID().toString())
.setVerificationCode("122222222ABCDEFGHJKLMNPQRSTUVWXY")
.isSuperuser(true)
.setLockCompletionTimestamp(fakeClock.nowUtc())
.build();
@ -116,7 +115,7 @@ public final class RegistryLockGetActionTest {
.setRepoId("repoId")
.setDomainName("incomplete.test")
.setRegistrarId("TheRegistrar")
.setVerificationCode(UUID.randomUUID().toString())
.setVerificationCode("111111111ABCDEFGHJKLMNPQRSTUVWXY")
.setRegistrarPocId("johndoe@theregistrar.com")
.build();
@ -126,7 +125,7 @@ public final class RegistryLockGetActionTest {
.setDomainName("unlocked.test")
.setRegistrarId("TheRegistrar")
.setRegistrarPocId("johndoe@theregistrar.com")
.setVerificationCode(UUID.randomUUID().toString())
.setVerificationCode("123456789ABCDEFGHJKLMNPQRSTUUUUU")
.setLockCompletionTimestamp(fakeClock.nowUtc())
.setUnlockRequestTimestamp(fakeClock.nowUtc())
.setUnlockCompletionTimestamp(fakeClock.nowUtc())

View file

@ -42,9 +42,12 @@ import google.registry.request.auth.AuthenticatedRegistrarAccessor.Role;
import google.registry.request.auth.UserAuthInfo;
import google.registry.schema.domain.RegistryLock;
import google.registry.testing.AppEngineRule;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.tools.DomainLockUtils;
import google.registry.util.EmailMessage;
import google.registry.util.SendEmailService;
import google.registry.util.StringGenerator.Alphabets;
import java.util.Map;
import java.util.UUID;
import javax.mail.internet.InternetAddress;
@ -402,7 +405,15 @@ public final class RegistryLockPostActionTest {
ImmutableSetMultimap.of("TheRegistrar", Role.OWNER, "NewRegistrar", Role.OWNER));
JsonActionRunner jsonActionRunner =
new JsonActionRunner(ImmutableMap.of(), new JsonResponse(new ResponseImpl(mockResponse)));
DomainLockUtils domainLockUtils =
new DomainLockUtils(new DeterministicStringGenerator(Alphabets.BASE_58));
return new RegistryLockPostAction(
jsonActionRunner, authResult, registrarAccessor, emailService, clock, outgoingAddress);
jsonActionRunner,
authResult,
registrarAccessor,
emailService,
clock,
domainLockUtils,
outgoingAddress);
}
}

View file

@ -47,11 +47,14 @@ import google.registry.schema.domain.RegistryLock;
import google.registry.security.XsrfTokenManager;
import google.registry.testing.AppEngineRule;
import google.registry.testing.DatastoreHelper;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.FakeClock;
import google.registry.testing.FakeResponse;
import google.registry.testing.InjectRule;
import google.registry.testing.UserInfo;
import java.util.UUID;
import google.registry.tools.DomainLockUtils;
import google.registry.util.StringGenerator;
import google.registry.util.StringGenerator.Alphabets;
import javax.servlet.http.HttpServletRequest;
import org.joda.time.Duration;
import org.junit.Before;
@ -81,7 +84,9 @@ public final class RegistryLockVerifyActionTest {
private final HttpServletRequest request = mock(HttpServletRequest.class);
private final UserService userService = UserServiceFactory.getUserService();
private final User user = new User("marla.singer@example.com", "gmail.com", "12345");
private final String lockId = "f1be78a2-2d61-458c-80f0-9dd8f2f8625f";
private final String lockId = "123456789ABCDEFGHJKLMNPQRSTUVWXY";
private final StringGenerator stringGenerator =
new DeterministicStringGenerator(Alphabets.BASE_58);
private FakeResponse response;
private DomainBase domain;
@ -145,7 +150,7 @@ public final class RegistryLockVerifyActionTest {
@Test
public void testFailure_badVerificationCode() {
RegistryLockDao.save(
createLock().asBuilder().setVerificationCode(UUID.randomUUID().toString()).build());
createLock().asBuilder().setVerificationCode("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA").build());
action.run();
assertThat(response.getPayload()).contains("Failed: Invalid verification code");
assertNoDomainChanges();
@ -330,7 +335,8 @@ public final class RegistryLockVerifyActionTest {
private RegistryLockVerifyAction createAction(String lockVerificationCode, Boolean isLock) {
response = new FakeResponse();
RegistryLockVerifyAction action =
new RegistryLockVerifyAction(fakeClock, lockVerificationCode, isLock);
new RegistryLockVerifyAction(
fakeClock, new DomainLockUtils(stringGenerator), lockVerificationCode, isLock);
authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, false));
action.req = request;
action.response = response;