diff --git a/core/src/main/java/google/registry/tools/DomainLockUtils.java b/core/src/main/java/google/registry/tools/DomainLockUtils.java index ab5f8054f..39facc25c 100644 --- a/core/src/main/java/google/registry/tools/DomainLockUtils.java +++ b/core/src/main/java/google/registry/tools/DomainLockUtils.java @@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.googlecode.objectify.Key; import google.registry.batch.AsyncTaskEnqueuer; +import google.registry.config.RegistryConfig.Config; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Reason; import google.registry.model.domain.DomainBase; @@ -52,13 +53,16 @@ public final class DomainLockUtils { private static final int VERIFICATION_CODE_LENGTH = 32; private final StringGenerator stringGenerator; + private final String registryAdminRegistrarId; private final AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject public DomainLockUtils( @Named("base58StringGenerator") StringGenerator stringGenerator, + @Config("registryAdminClientId") String registryAdminRegistrarId, AsyncTaskEnqueuer asyncTaskEnqueuer) { this.stringGenerator = stringGenerator; + this.registryAdminRegistrarId = registryAdminRegistrarId; this.asyncTaskEnqueuer = asyncTaskEnqueuer; } @@ -217,7 +221,7 @@ public final class DomainLockUtils { private RegistryLock.Builder createLockBuilder( String domainName, String registrarId, @Nullable String registrarPocId, boolean isAdmin) { DateTime now = jpaTm().getTransactionTime(); - DomainBase domainBase = getDomain(domainName, now); + DomainBase domainBase = getDomain(domainName, registrarId, now); verifyDomainNotLocked(domainBase); // Multiple pending actions are not allowed @@ -242,7 +246,7 @@ public final class DomainLockUtils { private RegistryLock.Builder createUnlockBuilder( String domainName, String registrarId, boolean isAdmin, Optional relockDuration) { DateTime now = jpaTm().getTransactionTime(); - DomainBase domainBase = getDomain(domainName, now); + DomainBase domainBase = getDomain(domainName, registrarId, now); Optional lockOptional = RegistryLockDao.getMostRecentVerifiedLockByRepoId(domainBase.getRepoId()); @@ -303,10 +307,19 @@ public final class DomainLockUtils { domainBase.getDomainName()); } - private static DomainBase getDomain(String domainName, DateTime now) { - return loadByForeignKeyCached(DomainBase.class, domainName, now) - .orElseThrow( - () -> new IllegalArgumentException(String.format("Unknown domain %s", domainName))); + private DomainBase getDomain(String domainName, String registrarId, DateTime now) { + DomainBase domain = + loadByForeignKeyCached(DomainBase.class, domainName, now) + .orElseThrow( + () -> new IllegalArgumentException(String.format("Unknown domain %s", domainName))); + // The user must have specified either the correct registrar ID or the admin registrar ID + checkArgument( + registryAdminRegistrarId.equals(registrarId) + || domain.getCurrentSponsorClientId().equals(registrarId), + "Domain %s is not owned by registrar %s", + domainName, + registrarId); + return domain; } private static RegistryLock getByVerificationCode(String verificationCode) { @@ -317,8 +330,8 @@ public final class DomainLockUtils { String.format("Invalid verification code %s", verificationCode))); } - private static void applyLockStatuses(RegistryLock lock, DateTime lockTime) { - DomainBase domain = getDomain(lock.getDomainName(), lockTime); + private void applyLockStatuses(RegistryLock lock, DateTime lockTime) { + DomainBase domain = getDomain(lock.getDomainName(), lock.getRegistrarId(), lockTime); verifyDomainNotLocked(domain); DomainBase newDomain = @@ -330,8 +343,8 @@ public final class DomainLockUtils { saveEntities(newDomain, lock, lockTime, true); } - private static void removeLockStatuses(RegistryLock lock, boolean isAdmin, DateTime unlockTime) { - DomainBase domain = getDomain(lock.getDomainName(), unlockTime); + private void removeLockStatuses(RegistryLock lock, boolean isAdmin, DateTime unlockTime) { + DomainBase domain = getDomain(lock.getDomainName(), lock.getRegistrarId(), unlockTime); if (!isAdmin) { verifyDomainLocked(domain); } diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java index b3f59964a..a21e0e5d8 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/RegistryLockPostAction.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.security.JsonResponseHelper.Status.ERROR; import static google.registry.security.JsonResponseHelper.Status.SUCCESS; -import static google.registry.ui.server.registrar.RegistrarConsoleModule.PARAM_CLIENT_ID; import static google.registry.ui.server.registrar.RegistryLockGetAction.getContactMatchingLogin; import static google.registry.ui.server.registrar.RegistryLockGetAction.getRegistrarAndVerifyLockAccess; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; @@ -116,10 +115,8 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc checkArgumentNotNull(input, "Null JSON"); RegistryLockPostInput postInput = GSON.fromJson(GSON.toJsonTree(input), RegistryLockPostInput.class); - checkArgument( - !Strings.isNullOrEmpty(postInput.clientId), - "Missing key for client: %s", - PARAM_CLIENT_ID); + String registrarId = postInput.registrarId; + checkArgument(!Strings.isNullOrEmpty(registrarId), "Missing key for registrarId"); checkArgument(!Strings.isNullOrEmpty(postInput.domainName), "Missing key for domainName"); checkNotNull(postInput.isLock, "Missing key for isLock"); UserAuthInfo userAuthInfo = @@ -135,12 +132,12 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc postInput.isLock ? domainLockUtils.saveNewRegistryLockRequest( postInput.domainName, - postInput.clientId, + registrarId, userEmail, registrarAccessor.isAdmin()) : domainLockUtils.saveNewRegistryUnlockRequest( postInput.domainName, - postInput.clientId, + registrarId, registrarAccessor.isAdmin(), Optional.ofNullable(postInput.relockDurationMillis).map(Duration::new)); sendVerificationEmail(registryLock, userEmail, postInput.isLock); @@ -190,9 +187,9 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc return user.getEmail(); } // Verify that the user can access the registrar, that the user has - // registry lock enabled, and that the user providjed a correct password + // registry lock enabled, and that the user provided a correct password Registrar registrar = - getRegistrarAndVerifyLockAccess(registrarAccessor, postInput.clientId, false); + getRegistrarAndVerifyLockAccess(registrarAccessor, postInput.registrarId, false); RegistrarContact registrarContact = getContactMatchingLogin(user, registrar) .orElseThrow( @@ -215,7 +212,7 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc /** Value class that represents the expected input body from the UI request. */ private static class RegistryLockPostInput { - private String clientId; + private String registrarId; private String domainName; private Boolean isLock; private String password; diff --git a/core/src/main/javascript/google/registry/ui/js/registrar/registry_lock.js b/core/src/main/javascript/google/registry/ui/js/registrar/registry_lock.js index 779bef2d4..5c1304685 100644 --- a/core/src/main/javascript/google/registry/ui/js/registrar/registry_lock.js +++ b/core/src/main/javascript/google/registry/ui/js/registrar/registry_lock.js @@ -172,7 +172,7 @@ registry.registrar.RegistryLock.prototype.lockOrUnlockDomain_ = function(isLock, e => this.fillLocksPage_(e), 'POST', goog.json.serialize({ - 'clientId': this.clientId, + 'registrarId': this.clientId, 'domainName': domain, 'isLock': isLock, 'password': password, diff --git a/core/src/test/java/google/registry/batch/RelockDomainActionTest.java b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java index 1e7feea04..18a88891b 100644 --- a/core/src/test/java/google/registry/batch/RelockDomainActionTest.java +++ b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java @@ -63,6 +63,7 @@ public class RelockDomainActionTest { private final DomainLockUtils domainLockUtils = new DomainLockUtils( new DeterministicStringGenerator(Alphabets.BASE_58), + "adminreg", AsyncTaskEnqueuerTest.createForTesting( mock(AppEngineServiceUtils.class), clock, Duration.ZERO)); diff --git a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java index 3ee5706ae..aa0f10ac6 100644 --- a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java +++ b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java @@ -74,6 +74,7 @@ public final class DomainLockUtilsTest { private final DomainLockUtils domainLockUtils = new DomainLockUtils( new DeterministicStringGenerator(Alphabets.BASE_58), + "adminreg", AsyncTaskEnqueuerTest.createForTesting( mock(AppEngineServiceUtils.class), clock, standardSeconds(90))); diff --git a/core/src/test/java/google/registry/tools/LockDomainCommandTest.java b/core/src/test/java/google/registry/tools/LockDomainCommandTest.java index 853c02c6c..d9987ad8d 100644 --- a/core/src/test/java/google/registry/tools/LockDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/LockDomainCommandTest.java @@ -51,6 +51,7 @@ public class LockDomainCommandTest extends CommandTestCase { command.domainLockUtils = new DomainLockUtils( new DeterministicStringGenerator(Alphabets.BASE_58), + "adminreg", AsyncTaskEnqueuerTest.createForTesting( mock(AppEngineServiceUtils.class), fakeClock, Duration.ZERO)); } @@ -58,7 +59,7 @@ public class LockDomainCommandTest extends CommandTestCase { @Test public void testSuccess_locksDomain() throws Exception { DomainBase domain = persistActiveDomain("example.tld"); - runCommandForced("--client=NewRegistrar", "example.tld"); + runCommandForced("--client=TheRegistrar", "example.tld"); assertThat(reloadResource(domain).getStatusValues()) .containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); } @@ -71,7 +72,7 @@ public class LockDomainCommandTest extends CommandTestCase { .asBuilder() .addStatusValue(SERVER_TRANSFER_PROHIBITED) .build()); - runCommandForced("--client=NewRegistrar", "example.tld"); + runCommandForced("--client=TheRegistrar", "example.tld"); assertThat(reloadResource(domain).getStatusValues()) .containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); } @@ -87,7 +88,7 @@ public class LockDomainCommandTest extends CommandTestCase { } runCommandForced( ImmutableList.builder() - .add("--client=NewRegistrar") + .add("--client=TheRegistrar") .addAll(domains.stream().map(DomainBase::getDomainName).collect(Collectors.toList())) .build()); for (DomainBase domain : domains) { diff --git a/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java b/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java index 9ce5aad4a..cfd8a43ad 100644 --- a/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java @@ -54,6 +54,7 @@ public class UnlockDomainCommandTest extends CommandTestCase domains = new ArrayList<>(); for (int n = 0; n < 26; n++) { String domain = String.format("domain%d.tld", n); - domains.add(persistLockedDomain(domain, "NewRegistrar")); + domains.add(persistLockedDomain(domain, "TheRegistrar")); } runCommandForced( ImmutableList.builder() - .add("--client=NewRegistrar") + .add("--client=TheRegistrar") .addAll(domains.stream().map(DomainBase::getDomainName).collect(Collectors.toList())) .build()); for (DomainBase domain : domains) { @@ -111,20 +112,20 @@ public class UnlockDomainCommandTest extends CommandTestCase runCommandForced("--client=NewRegistrar", "missing.tld")); + () -> runCommandForced("--client=TheRegistrar", "missing.tld")); assertThat(e).hasMessageThat().isEqualTo("Domain 'missing.tld' does not exist or is deleted"); } @Test public void testSuccess_alreadyUnlockedDomain_performsNoAction() throws Exception { DomainBase domain = persistActiveDomain("example.tld"); - runCommandForced("--client=NewRegistrar", "example.tld"); + runCommandForced("--client=TheRegistrar", "example.tld"); assertThat(reloadResource(domain)).isEqualTo(domain); } @Test public void testSuccess_defaultsToAdminRegistrar_ifUnspecified() throws Exception { - DomainBase domain = persistLockedDomain("example.tld", "NewRegistrar"); + DomainBase domain = persistLockedDomain("example.tld", "TheRegistrar"); runCommandForced("example.tld"); assertThat(getMostRecentRegistryLockByRepoId(domain.getRepoId()).get().getRegistrarId()) .isEqualTo("adminreg"); @@ -135,7 +136,7 @@ public class UnlockDomainCommandTest extends CommandTestCase runCommandForced("--client=NewRegistrar", "dupe.tld", "dupe.tld")); + () -> runCommandForced("--client=TheRegistrar", "dupe.tld", "dupe.tld")); assertThat(e).hasMessageThat().isEqualTo("Duplicate domain arguments found: 'dupe.tld'"); } } diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java index ce1cecbdb..7851d23f4 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockPostActionTest.java @@ -14,6 +14,7 @@ package google.registry.ui.server.registrar; +import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.testing.DatastoreHelper.createTld; @@ -33,7 +34,7 @@ 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.ImmutableSetMultimap; +import com.google.common.collect.ImmutableSet; import google.registry.batch.AsyncTaskEnqueuerTest; import google.registry.model.domain.DomainBase; import google.registry.request.JsonActionRunner; @@ -104,6 +105,7 @@ public final class RegistryLockPostActionTest { userWithoutPermission = userFromRegistrarContact(AppEngineRule.makeRegistrarContact2()); createTld("tld"); domain = persistResource(newDomainBase("example.tld")); + outgoingAddress = new InternetAddress("domain-registry@example.com"); when(mockRequest.getServerName()).thenReturn("registrarconsole.tld"); @@ -224,7 +226,7 @@ public final class RegistryLockPostActionTest { Map response = action.handleJsonRequest( ImmutableMap.of( - "clientId", "TheRegistrar", + "registrarId", "TheRegistrar", "domainName", "example.tld", "isLock", true)); assertSuccess(response, "lock", "johndoe@theregistrar.com"); @@ -237,22 +239,45 @@ public final class RegistryLockPostActionTest { } @Test - public void testFailure_noClientId() { + public void testFailure_noRegistrarId() { Map response = action.handleJsonRequest(ImmutableMap.of()); - assertFailureWithMessage(response, "Missing key for client: clientId"); + assertFailureWithMessage(response, "Missing key for registrarId"); } @Test - public void testFailure_emptyClientId() { - Map response = action.handleJsonRequest(ImmutableMap.of("clientId", "")); - assertFailureWithMessage(response, "Missing key for client: clientId"); + public void testFailure_emptyRegistrarId() { + Map response = action.handleJsonRequest(ImmutableMap.of("registrarId", "")); + assertFailureWithMessage(response, "Missing key for registrarId"); + } + + @Test + public void testFailure_unauthorizedRegistrarId() { + AuthResult authResult = + AuthResult.create(AuthLevel.USER, UserAuthInfo.create(userWithLockPermission, false)); + action = createAction(authResult, ImmutableSet.of("TheRegistrar")); + Map response = + action.handleJsonRequest( + ImmutableMap.of( + "isLock", true, + "registrarId", "NewRegistrar", + "domainName", "example.tld", + "password", "hi")); + assertFailureWithMessage(response, "TestUserId doesn't have access to registrar NewRegistrar"); + } + + @Test + public void testFailure_incorrectRegistrarIdForDomain() { + persistResource(domain.asBuilder().setPersistedCurrentSponsorClientId("NewRegistrar").build()); + Map response = action.handleJsonRequest(lockRequest()); + assertFailureWithMessage( + response, "Domain example.tld is not owned by registrar TheRegistrar"); } @Test public void testFailure_noDomainName() { Map response = action.handleJsonRequest( - ImmutableMap.of("clientId", "TheRegistrar", "password", "hi", "isLock", true)); + ImmutableMap.of("registrarId", "TheRegistrar", "password", "hi", "isLock", true)); assertFailureWithMessage(response, "Missing key for domainName"); } @@ -261,7 +286,7 @@ public final class RegistryLockPostActionTest { Map response = action.handleJsonRequest( ImmutableMap.of( - "clientId", "TheRegistrar", + "registrarId", "TheRegistrar", "domainName", "example.tld", "password", "hi")); assertFailureWithMessage(response, "Missing key for isLock"); @@ -280,7 +305,7 @@ public final class RegistryLockPostActionTest { Map response = action.handleJsonRequest( ImmutableMap.of( - "clientId", "TheRegistrar", + "registrarId", "TheRegistrar", "domainName", "example.tld", "isLock", true)); assertFailureWithMessage(response, "Incorrect registry lock password for contact"); @@ -294,7 +319,7 @@ public final class RegistryLockPostActionTest { Map response = action.handleJsonRequest( ImmutableMap.of( - "clientId", "TheRegistrar", + "registrarId", "TheRegistrar", "domainName", "example.tld", "isLock", true, "password", "hi")); @@ -306,7 +331,7 @@ public final class RegistryLockPostActionTest { Map response = action.handleJsonRequest( ImmutableMap.of( - "clientId", "TheRegistrar", + "registrarId", "TheRegistrar", "domainName", "example.tld", "isLock", true, "password", "badPassword")); @@ -318,7 +343,7 @@ public final class RegistryLockPostActionTest { Map response = action.handleJsonRequest( ImmutableMap.of( - "clientId", "TheRegistrar", + "registrarId", "TheRegistrar", "domainName", "bad.tld", "isLock", true, "password", "hi")); @@ -381,7 +406,7 @@ public final class RegistryLockPostActionTest { private ImmutableMap fullRequest(boolean lock) { return ImmutableMap.of( "isLock", lock, - "clientId", "TheRegistrar", + "registrarId", "TheRegistrar", "domainName", "example.tld", "password", "hi"); } @@ -425,15 +450,21 @@ public final class RegistryLockPostActionTest { } private RegistryLockPostAction createAction(AuthResult authResult) { + return createAction(authResult, ImmutableSet.of("TheRegistrar", "NewRegistrar")); + } + + private RegistryLockPostAction createAction( + AuthResult authResult, ImmutableSet accessibleRegistrars) { Role role = authResult.userAuthInfo().get().isUserAdmin() ? Role.ADMIN : Role.OWNER; AuthenticatedRegistrarAccessor registrarAccessor = AuthenticatedRegistrarAccessor.createForTesting( - ImmutableSetMultimap.of("TheRegistrar", role, "NewRegistrar", role)); + accessibleRegistrars.stream().collect(toImmutableSetMultimap(r -> r, r -> role))); JsonActionRunner jsonActionRunner = new JsonActionRunner(ImmutableMap.of(), new JsonResponse(new ResponseImpl(mockResponse))); DomainLockUtils domainLockUtils = new DomainLockUtils( new DeterministicStringGenerator(Alphabets.BASE_58), + "adminreg", AsyncTaskEnqueuerTest.createForTesting( mock(AppEngineServiceUtils.class), clock, Duration.ZERO)); return new RegistryLockPostAction( diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java index b300d96fa..aab6a320b 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistryLockVerifyActionTest.java @@ -332,6 +332,7 @@ public final class RegistryLockVerifyActionTest { new RegistryLockVerifyAction( new DomainLockUtils( stringGenerator, + "adminreg", AsyncTaskEnqueuerTest.createForTesting( mock(AppEngineServiceUtils.class), fakeClock, Duration.ZERO)), lockVerificationCode,