Verify that the RegistryLock input has the correct registrar ID (#661)

* Verify that the RegistryLock input has the correct registrar ID

We already verify (correctly) that the user has access to the registrar
they specify, but nowhere did we verify that the registrar ID they used
is actually the current sponsor ID for the domain in question. This is
an oversight caused by the fact that our testing framework only uses
admin accounts, which by the nature of things have access to all
registrars and domains.

In addition, rename "clientId" to "registrarId" in the RLPA object

* Change the wording on the incorrect-registrar message
This commit is contained in:
gbrodman
2020-07-05 22:31:14 -04:00
committed by GitHub
parent 333170a724
commit 806f3b2456
9 changed files with 95 additions and 49 deletions
@@ -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<Duration> relockDuration) {
DateTime now = jpaTm().getTransactionTime();
DomainBase domainBase = getDomain(domainName, now);
DomainBase domainBase = getDomain(domainName, registrarId, now);
Optional<RegistryLock> 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);
}
@@ -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;
@@ -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,