From 560bec1e835e15e0adec3d456d12a79a68d90e2d Mon Sep 17 00:00:00 2001 From: gbrodman Date: Thu, 12 Mar 2020 16:02:27 -0400 Subject: [PATCH] Add a RelockDomainAction for future auto-relocks (#485) * Add a RelockAction and reference to relocks in RegistryLocks * Respond to CR - refactor the request param exception logging a bit - don't log an error if the domain was already locked, just skip * Save a relock for all locks (if possible) * derp * Long -> long + remove unnecessary transact * semantic merge conflict woo * fix another semantic merge conflict --- .../google/registry/batch/BatchModule.java | 11 +- .../registry/batch/RelockDomainAction.java | 167 +++++++++++++++++ .../model/registry/RegistryLockDao.java | 30 ++- .../backend/BackendRequestComponent.java | 2 + .../spec11/PublishSpec11ReportAction.java | 3 +- .../registry/request/RequestParameters.java | 54 ++++-- .../registry/schema/domain/RegistryLock.java | 23 +++ .../registry/tools/DomainLockUtils.java | 18 +- .../batch/RelockDomainActionTest.java | 176 ++++++++++++++++++ .../model/registry/RegistryLockDaoTest.java | 40 ++++ .../google/registry/testing/SqlHelper.java | 8 + .../registry/tools/DomainLockUtilsTest.java | 47 ++++- .../module/backend/backend_routing.txt | 1 + db/README.md | 2 +- .../V19__add_registry_relock_reference.sql | 20 ++ .../sql/schema/db-schema.sql.generated | 6 + .../resources/sql/schema/nomulus.golden.sql | 11 +- 17 files changed, 578 insertions(+), 41 deletions(-) create mode 100644 core/src/main/java/google/registry/batch/RelockDomainAction.java create mode 100644 core/src/test/java/google/registry/batch/RelockDomainActionTest.java create mode 100644 db/src/main/resources/sql/flyway/V19__add_registry_relock_reference.sql diff --git a/core/src/main/java/google/registry/batch/BatchModule.java b/core/src/main/java/google/registry/batch/BatchModule.java index befe21ad7..d69db421e 100644 --- a/core/src/main/java/google/registry/batch/BatchModule.java +++ b/core/src/main/java/google/registry/batch/BatchModule.java @@ -21,6 +21,7 @@ import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE; import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME; +import static google.registry.request.RequestParameters.extractLongParameter; import static google.registry.request.RequestParameters.extractOptionalBooleanParameter; import static google.registry.request.RequestParameters.extractOptionalIntParameter; import static google.registry.request.RequestParameters.extractOptionalParameter; @@ -40,9 +41,7 @@ import javax.inject.Named; import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; -/** - * Dagger module for injecting common settings for batch actions. - */ +/** Dagger module for injecting common settings for batch actions. */ @Module public class BatchModule { @@ -94,6 +93,12 @@ public class BatchModule { return extractSetOfDatetimeParameters(req, PARAM_RESAVE_TIMES); } + @Provides + @Parameter("oldUnlockRevisionId") + static long provideOldUnlockRevisionId(HttpServletRequest req) { + return extractLongParameter(req, "oldUnlockRevisionId"); + } + @Provides @Named(QUEUE_ASYNC_ACTIONS) static Queue provideAsyncActionsPushQueue() { diff --git a/core/src/main/java/google/registry/batch/RelockDomainAction.java b/core/src/main/java/google/registry/batch/RelockDomainAction.java new file mode 100644 index 000000000..ef27f68f2 --- /dev/null +++ b/core/src/main/java/google/registry/batch/RelockDomainAction.java @@ -0,0 +1,167 @@ +// Copyright 2020 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.batch; + +import static com.google.common.base.Preconditions.checkArgument; +import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.request.Action.Method.POST; +import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; +import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; +import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; +import static javax.servlet.http.HttpServletResponse.SC_OK; + +import com.google.common.collect.ImmutableSet; +import com.google.common.flogger.FluentLogger; +import com.google.common.net.MediaType; +import google.registry.model.domain.DomainBase; +import google.registry.model.eppcommon.StatusValue; +import google.registry.model.registry.RegistryLockDao; +import google.registry.request.Action; +import google.registry.request.Parameter; +import google.registry.request.Response; +import google.registry.request.auth.Auth; +import google.registry.schema.domain.RegistryLock; +import google.registry.tools.DomainLockUtils; +import google.registry.util.DateTimeUtils; +import javax.inject.Inject; + +/** + * Task that relocks a previously-Registry-Locked domain after some predetermined period of time. + */ +@Action( + service = Action.Service.BACKEND, + path = RelockDomainAction.PATH, + method = POST, + automaticallyPrintOk = true, + auth = Auth.AUTH_INTERNAL_OR_ADMIN) +public class RelockDomainAction implements Runnable { + + public static final String PATH = "/_dr/task/relockDomain"; + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + private final long oldUnlockRevisionId; + private final DomainLockUtils domainLockUtils; + private final Response response; + + @Inject + public RelockDomainAction( + @Parameter("oldUnlockRevisionId") long oldUnlockRevisionId, + DomainLockUtils domainLockUtils, + Response response) { + this.oldUnlockRevisionId = oldUnlockRevisionId; + this.domainLockUtils = domainLockUtils; + this.response = response; + } + + @Override + public void run() { + jpaTm().transact(this::relockDomain); + } + + private void relockDomain() { + RegistryLock oldLock; + try { + oldLock = + RegistryLockDao.getByRevisionId(oldUnlockRevisionId) + .orElseThrow( + () -> + new IllegalArgumentException( + String.format("Unknown revision ID %d", oldUnlockRevisionId))); + DomainBase domain = + ofy() + .load() + .type(DomainBase.class) + .id(oldLock.getRepoId()) + .now() + .cloneProjectedAtTime(jpaTm().getTransactionTime()); + + if (domain.getStatusValues().containsAll(REGISTRY_LOCK_STATUSES) + || oldLock.getRelock() != null) { + // The domain was manually locked, so we shouldn't worry about relocking + String message = + String.format( + "Domain %s is already manually relocked, skipping automated relock.", + domain.getFullyQualifiedDomainName()); + logger.atInfo().log(message); + // SC_NO_CONTENT (204) skips retry -- see the comment below + response.setStatus(SC_NO_CONTENT); + response.setContentType(MediaType.PLAIN_TEXT_UTF_8); + response.setPayload(message); + return; + } + verifyDomainAndLockState(oldLock, domain); + } catch (Throwable t) { + /* If there's a bad verification code or the domain is in a bad state, we won't want to retry. + * AppEngine will retry on non-2xx error codes, so we return SC_NO_CONTENT (204) to avoid it. + * + * See https://cloud.google.com/appengine/docs/standard/java/taskqueue/push/retrying-tasks + * for more details on retry behavior. */ + logger.atWarning().withCause(t).log( + "Exception when attempting to relock domain with old revision ID %d.", + oldUnlockRevisionId); + response.setStatus(SC_NO_CONTENT); + response.setContentType(MediaType.PLAIN_TEXT_UTF_8); + response.setPayload(String.format("Relock failed: %s", t.getMessage())); + return; + } + applyRelock(oldLock); + } + + private void applyRelock(RegistryLock oldLock) { + try { + domainLockUtils.administrativelyApplyLock( + oldLock.getDomainName(), + oldLock.getRegistrarId(), + oldLock.getRegistrarPocId(), + oldLock.isSuperuser()); + logger.atInfo().log("Relocked domain %s.", oldLock.getDomainName()); + response.setStatus(SC_OK); + } catch (Throwable t) { + // Any errors that occur here are unexpected, so we should retry. Return a non-2xx + // error code to get AppEngine to retry + logger.atSevere().withCause(t).log( + "Exception when attempting to relock domain %s.", oldLock.getDomainName()); + response.setStatus(SC_INTERNAL_SERVER_ERROR); + response.setContentType(MediaType.PLAIN_TEXT_UTF_8); + response.setPayload(String.format("Relock failed: %s", t.getMessage())); + } + } + + private void verifyDomainAndLockState(RegistryLock oldLock, DomainBase domain) { + // Domain shouldn't be deleted or have a pending transfer/delete + String domainName = domain.getFullyQualifiedDomainName(); + checkArgument( + !DateTimeUtils.isAtOrAfter(jpaTm().getTransactionTime(), domain.getDeletionTime()), + "Domain %s has been deleted", + domainName); + ImmutableSet statusValues = domain.getStatusValues(); + checkArgument( + !statusValues.contains(StatusValue.PENDING_DELETE), + "Domain %s has a pending delete", + domainName); + checkArgument( + !statusValues.contains(StatusValue.PENDING_TRANSFER), + "Domain %s has a pending transfer", + domainName); + checkArgument( + domain.getCurrentSponsorClientId().equals(oldLock.getRegistrarId()), + "Domain %s has been transferred from registrar %s to registrar %s since the unlock", + domainName, + oldLock.getRegistrarId(), + domain.getCurrentSponsorClientId()); + } +} diff --git a/core/src/main/java/google/registry/model/registry/RegistryLockDao.java b/core/src/main/java/google/registry/model/registry/RegistryLockDao.java index 8568cce79..7cf61f3e8 100644 --- a/core/src/main/java/google/registry/model/registry/RegistryLockDao.java +++ b/core/src/main/java/google/registry/model/registry/RegistryLockDao.java @@ -25,6 +25,12 @@ import javax.persistence.EntityManager; /** Data access object for {@link google.registry.schema.domain.RegistryLock}. */ public final class RegistryLockDao { + /** Returns the {@link RegistryLock} referred to by this revision ID, or empty if none exists. */ + public static Optional getByRevisionId(long revisionId) { + jpaTm().assertInTransaction(); + return Optional.ofNullable(jpaTm().getEntityManager().find(RegistryLock.class, revisionId)); + } + /** Returns the most recent version of the {@link RegistryLock} referred to by the code. */ public static Optional getByVerificationCode(String verificationCode) { jpaTm().assertInTransaction(); @@ -84,7 +90,29 @@ public final class RegistryLockDao { .getEntityManager() .createQuery( "SELECT lock FROM RegistryLock lock WHERE lock.repoId = :repoId AND" - + " lock.lockCompletionTimestamp IS NOT NULL ORDER BY lock.revisionId" + + " lock.lockCompletionTimestamp IS NOT NULL AND" + + " lock.unlockCompletionTimestamp IS NULL ORDER BY lock.revisionId" + + " DESC", + RegistryLock.class) + .setParameter("repoId", repoId) + .setMaxResults(1) + .getResultStream() + .findFirst(); + } + + /** + * Returns the most recent verified unlock for a given domain specified by repo ID. + * + *

Returns empty if no unlock has ever been finalized for this domain. This is different from + * {@link #getMostRecentByRepoId(String)} in that it only returns verified unlocks. + */ + public static Optional getMostRecentVerifiedUnlockByRepoId(String repoId) { + jpaTm().assertInTransaction(); + return jpaTm() + .getEntityManager() + .createQuery( + "SELECT lock FROM RegistryLock lock WHERE lock.repoId = :repoId AND" + + " lock.unlockCompletionTimestamp IS NOT NULL ORDER BY lock.revisionId" + " DESC", RegistryLock.class) .setParameter("repoId", repoId) diff --git a/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java b/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java index ba70d1e84..d8c5e5580 100644 --- a/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java +++ b/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java @@ -26,6 +26,7 @@ import google.registry.batch.DeleteLoadTestDataAction; import google.registry.batch.DeleteProberDataAction; import google.registry.batch.ExpandRecurringBillingEventsAction; import google.registry.batch.RefreshDnsOnHostRenameAction; +import google.registry.batch.RelockDomainAction; import google.registry.batch.ResaveAllEppResourcesAction; import google.registry.batch.ResaveEntityAction; import google.registry.cron.CommitLogFanoutAction; @@ -140,6 +141,7 @@ interface BackendRequestComponent { RdeReporter rdeReporter(); RefreshDnsAction refreshDnsAction(); RefreshDnsOnHostRenameAction refreshDnsOnHostRenameAction(); + RelockDomainAction relockDomainAction(); ResaveAllEppResourcesAction resaveAllEppResourcesAction(); ResaveEntityAction resaveEntityAction(); SyncGroupMembersAction syncGroupMembersAction(); diff --git a/core/src/main/java/google/registry/reporting/spec11/PublishSpec11ReportAction.java b/core/src/main/java/google/registry/reporting/spec11/PublishSpec11ReportAction.java index 89ee7ebf1..384f6fc21 100644 --- a/core/src/main/java/google/registry/reporting/spec11/PublishSpec11ReportAction.java +++ b/core/src/main/java/google/registry/reporting/spec11/PublishSpec11ReportAction.java @@ -193,8 +193,7 @@ public class PublishSpec11ReportAction implements Runnable { // Group by email address then flat-map all of the ThreatMatch objects together return ImmutableMap.copyOf( Maps.transformValues( - Multimaps.index(registrarThreatMatches, RegistrarThreatMatches::clientId) - .asMap(), + Multimaps.index(registrarThreatMatches, RegistrarThreatMatches::clientId).asMap(), registrarThreatMatchesCollection -> registrarThreatMatchesCollection.stream() .flatMap(matches -> matches.threatMatches().stream()) diff --git a/core/src/main/java/google/registry/request/RequestParameters.java b/core/src/main/java/google/registry/request/RequestParameters.java index 90af2d61b..91372b42a 100644 --- a/core/src/main/java/google/registry/request/RequestParameters.java +++ b/core/src/main/java/google/registry/request/RequestParameters.java @@ -44,11 +44,11 @@ public final class RequestParameters { * method to yield the following results: * *

    - *
  • /foo?bar=hello → hello - *
  • /foo?bar=hello&bar=there → hello - *
  • /foo?bar= → 400 error (empty) - *
  • /foo?bar=&bar=there → 400 error (empty) - *
  • /foo → 400 error (absent) + *
  • /foo?bar=hello → hello + *
  • /foo?bar=hello&bar=there → hello + *
  • /foo?bar= → 400 error (empty) + *
  • /foo?bar=&bar=there → 400 error (empty) + *
  • /foo → 400 error (absent) *
* * @throws BadRequestException if request parameter is absent or empty @@ -88,10 +88,27 @@ public final class RequestParameters { * @throws BadRequestException if request parameter is absent, empty, or not a valid integer */ public static int extractIntParameter(HttpServletRequest req, String name) { + String stringParam = req.getParameter(name); try { - return Integer.parseInt(nullToEmpty(req.getParameter(name))); + return Integer.parseInt(nullToEmpty(stringParam)); } catch (NumberFormatException e) { - throw new BadRequestException("Expected integer: " + name); + throw new BadRequestException( + String.format("Expected int for parameter %s but received %s", name, stringParam)); + } + } + + /** + * Returns first GET or POST parameter associated with {@code name} as a long. + * + * @throws BadRequestException if request parameter is absent, empty, or not a valid long + */ + public static long extractLongParameter(HttpServletRequest req, String name) { + String stringParam = req.getParameter(name); + try { + return Long.parseLong(nullToEmpty(stringParam)); + } catch (NumberFormatException e) { + throw new BadRequestException( + String.format("Expected long for parameter %s but received %s", name, stringParam)); } } @@ -126,9 +143,7 @@ public final class RequestParameters { if (parameter == null || parameter.isEmpty()) { return ImmutableSet.of(); } - return Splitter.on(',') - .splitToList(parameter) - .stream() + return Splitter.on(',').splitToList(parameter).stream() .filter(s -> !s.isEmpty()) .collect(toImmutableSet()); } @@ -160,8 +175,8 @@ public final class RequestParameters { * @throws BadRequestException if request parameter named {@code name} is absent, empty, or not * equal to any of the values in {@code enumClass} */ - public static > - C extractEnumParameter(HttpServletRequest req, Class enumClass, String name) { + public static > C extractEnumParameter( + HttpServletRequest req, Class enumClass, String name) { return getEnumValue(enumClass, extractRequiredParameter(req, name), name); } @@ -216,9 +231,9 @@ public final class RequestParameters { } /** - * Returns first request parameter associated with {@code name} parsed as an - * ISO 8601 timestamp, e.g. {@code 1984-12-18TZ}, - * {@code 2000-01-01T16:20:00Z}. + * Returns first request parameter associated with {@code name} parsed as an ISO 8601 timestamp, e.g. {@code 1984-12-18TZ}, {@code + * 2000-01-01T16:20:00Z}. * * @throws BadRequestException if request parameter named {@code name} is absent, empty, or could * not be parsed as an ISO 8601 timestamp @@ -233,9 +248,9 @@ public final class RequestParameters { } /** - * Returns first request parameter associated with {@code name} parsed as an - * ISO 8601 timestamp, e.g. {@code 1984-12-18TZ}, - * {@code 2000-01-01T16:20:00Z}. + * Returns first request parameter associated with {@code name} parsed as an ISO 8601 timestamp, e.g. {@code 1984-12-18TZ}, {@code + * 2000-01-01T16:20:00Z}. * * @throws BadRequestException if request parameter is present but not a valid {@link DateTime}. */ @@ -262,8 +277,7 @@ public final class RequestParameters { public static ImmutableSet extractSetOfDatetimeParameters( HttpServletRequest req, String name) { try { - return extractSetOfParameters(req, name) - .stream() + return extractSetOfParameters(req, name).stream() .filter(not(String::isEmpty)) .map(DateTime::parse) .collect(toImmutableSet()); diff --git a/core/src/main/java/google/registry/schema/domain/RegistryLock.java b/core/src/main/java/google/registry/schema/domain/RegistryLock.java index 3acdf5dcf..63e213e8e 100644 --- a/core/src/main/java/google/registry/schema/domain/RegistryLock.java +++ b/core/src/main/java/google/registry/schema/domain/RegistryLock.java @@ -28,10 +28,13 @@ import java.time.ZonedDateTime; import java.util.Optional; import javax.persistence.Column; import javax.persistence.Entity; +import javax.persistence.FetchType; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; import javax.persistence.Id; import javax.persistence.Index; +import javax.persistence.JoinColumn; +import javax.persistence.OneToOne; import javax.persistence.Table; import org.joda.time.DateTime; @@ -123,6 +126,11 @@ public final class RegistryLock extends ImmutableObject implements Buildable { @Column(nullable = false) private boolean isSuperuser; + /** The lock that undoes this lock, if this lock has been unlocked and the domain locked again. */ + @OneToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "relockRevisionId", referencedColumnName = "revisionId") + private RegistryLock relock; + /** Time that this entity was last updated. */ private UpdateAutoTimestamp lastUpdateTimestamp; @@ -179,6 +187,16 @@ public final class RegistryLock extends ImmutableObject implements Buildable { return revisionId; } + /** + * The lock that undoes this lock, if this lock has been unlocked and the domain locked again. + * + *

Note: this is lazily loaded, so it may not be initialized if referenced outside of the + * transaction in which this lock is loaded. + */ + public RegistryLock getRelock() { + return relock; + } + public boolean isLocked() { return lockCompletionTimestamp != null && unlockCompletionTimestamp == null; } @@ -266,5 +284,10 @@ public final class RegistryLock extends ImmutableObject implements Buildable { getInstance().isSuperuser = isSuperuser; return this; } + + public Builder setRelock(RegistryLock relock) { + getInstance().relock = relock; + return this; + } } } diff --git a/core/src/main/java/google/registry/tools/DomainLockUtils.java b/core/src/main/java/google/registry/tools/DomainLockUtils.java index 558a1c8da..c2374118b 100644 --- a/core/src/main/java/google/registry/tools/DomainLockUtils.java +++ b/core/src/main/java/google/registry/tools/DomainLockUtils.java @@ -106,6 +106,7 @@ public final class DomainLockUtils { RegistryLock newLock = RegistryLockDao.save(lock.asBuilder().setLockCompletionTimestamp(now).build()); + setAsRelock(newLock); tm().transact(() -> applyLockStatuses(newLock, now)); return newLock; }); @@ -149,13 +150,14 @@ public final class DomainLockUtils { .transact( () -> { DateTime now = jpaTm().getTransactionTime(); - RegistryLock result = + RegistryLock newLock = RegistryLockDao.save( createLockBuilder(domainName, registrarId, registrarPocId, isAdmin) .setLockCompletionTimestamp(now) .build()); - tm().transact(() -> applyLockStatuses(result, now)); - return result; + tm().transact(() -> applyLockStatuses(newLock, now)); + setAsRelock(newLock); + return newLock; }); } @@ -179,6 +181,16 @@ public final class DomainLockUtils { }); } + private void setAsRelock(RegistryLock newLock) { + jpaTm() + .transact( + () -> + RegistryLockDao.getMostRecentVerifiedUnlockByRepoId(newLock.getRepoId()) + .ifPresent( + oldLock -> + RegistryLockDao.save(oldLock.asBuilder().setRelock(newLock).build()))); + } + private RegistryLock.Builder createLockBuilder( String domainName, String registrarId, @Nullable String registrarPocId, boolean isAdmin) { DateTime now = jpaTm().getTransactionTime(); diff --git a/core/src/test/java/google/registry/batch/RelockDomainActionTest.java b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java new file mode 100644 index 000000000..a5bcbcb35 --- /dev/null +++ b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java @@ -0,0 +1,176 @@ +// Copyright 2020 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.batch; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.eppcommon.StatusValue.PENDING_DELETE; +import static google.registry.model.eppcommon.StatusValue.PENDING_TRANSFER; +import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.testing.DatastoreHelper.createTlds; +import static google.registry.testing.DatastoreHelper.newDomainBase; +import static google.registry.testing.DatastoreHelper.persistActiveHost; +import static google.registry.testing.DatastoreHelper.persistDomainAsDeleted; +import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.getMostRecentVerifiedRegistryLockByRepoId; +import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode; +import static google.registry.testing.SqlHelper.saveRegistryLock; +import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; +import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; + +import com.google.common.collect.ImmutableSet; +import google.registry.model.domain.DomainBase; +import google.registry.model.host.HostResource; +import google.registry.schema.domain.RegistryLock; +import google.registry.testing.AppEngineRule; +import google.registry.testing.DeterministicStringGenerator; +import google.registry.testing.FakeClock; +import google.registry.testing.FakeResponse; +import google.registry.testing.UserInfo; +import google.registry.tools.DomainLockUtils; +import google.registry.util.StringGenerator.Alphabets; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link RelockDomainAction}. */ +@RunWith(JUnit4.class) +public class RelockDomainActionTest { + + private static final String DOMAIN_NAME = "example.tld"; + private static final String CLIENT_ID = "TheRegistrar"; + private static final String POC_ID = "marla.singer@example.com"; + + private final FakeResponse response = new FakeResponse(); + private final FakeClock clock = new FakeClock(); + private final DomainLockUtils domainLockUtils = + new DomainLockUtils(new DeterministicStringGenerator(Alphabets.BASE_58)); + + @Rule + public final AppEngineRule appEngineRule = + AppEngineRule.builder() + .withDatastoreAndCloudSql() + .withUserService(UserInfo.create(POC_ID, "12345")) + .build(); + + private DomainBase domain; + private RegistryLock oldLock; + private RelockDomainAction action; + + @Before + public void setup() { + createTlds("tld", "net"); + HostResource host = persistActiveHost("ns1.example.net"); + domain = persistResource(newDomainBase(DOMAIN_NAME, host)); + + oldLock = domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, CLIENT_ID, POC_ID, false); + assertThat(reloadDomain(domain).getStatusValues()) + .containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); + oldLock = domainLockUtils.administrativelyApplyUnlock(DOMAIN_NAME, CLIENT_ID, false); + assertThat(reloadDomain(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); + action = createAction(oldLock.getRevisionId()); + } + + @Test + public void testLock() { + action.run(); + assertThat(reloadDomain(domain).getStatusValues()) + .containsAtLeastElementsIn(REGISTRY_LOCK_STATUSES); + + // the old lock should have a reference to the relock + RegistryLock newLock = getMostRecentVerifiedRegistryLockByRepoId(domain.getRepoId()).get(); + assertThat(getRegistryLockByVerificationCode(oldLock.getVerificationCode()).get().getRelock()) + .isEqualTo(newLock); + } + + @Test + public void testFailure_unknownCode() { + action = createAction(12128675309L); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()).isEqualTo("Relock failed: Unknown revision ID 12128675309"); + } + + @Test + public void testFailure_pendingDelete() { + persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_DELETE)).build()); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()) + .isEqualTo(String.format("Relock failed: Domain %s has a pending delete", DOMAIN_NAME)); + } + + @Test + public void testFailure_pendingTransfer() { + persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_TRANSFER)).build()); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()) + .isEqualTo(String.format("Relock failed: Domain %s has a pending transfer", DOMAIN_NAME)); + } + + @Test + public void testFailure_domainAlreadyLocked() { + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, CLIENT_ID, null, true); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()) + .isEqualTo("Domain example.tld is already manually relocked, skipping automated relock."); + } + + @Test + public void testFailure_domainDeleted() { + persistDomainAsDeleted(domain, clock.nowUtc()); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()) + .isEqualTo(String.format("Relock failed: Domain %s has been deleted", DOMAIN_NAME)); + } + + @Test + public void testFailure_domainTransferred() { + persistResource(domain.asBuilder().setPersistedCurrentSponsorClientId("NewRegistrar").build()); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()) + .isEqualTo( + String.format( + "Relock failed: Domain %s has been transferred from registrar %s to registrar " + + "%s since the unlock", + DOMAIN_NAME, CLIENT_ID, "NewRegistrar")); + } + + @Test + public void testFailure_relockAlreadySet() { + RegistryLock newLock = + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, CLIENT_ID, null, true); + saveRegistryLock(oldLock.asBuilder().setRelock(newLock).build()); + // Save the domain without the lock statuses so that we pass that check in the action + persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of()).build()); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()) + .isEqualTo("Domain example.tld is already manually relocked, skipping automated relock."); + } + + private DomainBase reloadDomain(DomainBase domain) { + return ofy().load().entity(domain).now(); + } + + private RelockDomainAction createAction(Long oldUnlockRevisionId) { + return new RelockDomainAction(oldUnlockRevisionId, domainLockUtils, response); + } +} diff --git a/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java b/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java index bc022de2b..ff5c5d766 100644 --- a/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java +++ b/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java @@ -18,7 +18,9 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.testing.SqlHelper.getMostRecentRegistryLockByRepoId; +import static google.registry.testing.SqlHelper.getMostRecentUnlockedRegistryLockByRepoId; import static google.registry.testing.SqlHelper.getMostRecentVerifiedRegistryLockByRepoId; +import static google.registry.testing.SqlHelper.getRegistryLockByRevisionId; import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode; import static google.registry.testing.SqlHelper.getRegistryLocksByRegistrarId; import static google.registry.testing.SqlHelper.saveRegistryLock; @@ -121,6 +123,20 @@ public final class RegistryLockDaoTest { assertThat(getRegistryLockByVerificationCode("hi").isPresent()).isFalse(); } + @Test + public void testByRevisionId_valid() { + RegistryLock lock = saveRegistryLock(createLock()); + RegistryLock otherLock = getRegistryLockByRevisionId(lock.getRevisionId()).get(); + // can't do direct comparison due to update time + assertThat(lock.getDomainName()).isEqualTo(otherLock.getDomainName()); + assertThat(lock.getVerificationCode()).isEqualTo(otherLock.getVerificationCode()); + } + + @Test + public void testByRevisionId_invalid() { + assertThat(getRegistryLockByRevisionId(8675309L).isPresent()).isFalse(); + } + @Test public void testLoad_lockedDomains_byRegistrarId() { RegistryLock lock = createLock(); @@ -192,6 +208,30 @@ public final class RegistryLockDaoTest { assertThat(mostRecent.isPresent()).isFalse(); } + @Test + public void testLoad_verifiedUnlock_byRepoId() { + RegistryLock lock = + saveRegistryLock( + createLock() + .asBuilder() + .setLockCompletionTimestamp(fakeClock.nowUtc()) + .setUnlockRequestTimestamp(fakeClock.nowUtc()) + .setUnlockCompletionTimestamp(fakeClock.nowUtc()) + .build()); + + Optional mostRecent = getMostRecentUnlockedRegistryLockByRepoId(lock.getRepoId()); + assertThat(mostRecent.get().getRevisionId()).isEqualTo(lock.getRevisionId()); + } + + @Test + public void testLoad_verifiedUnlock_empty() { + RegistryLock completedLock = + createLock().asBuilder().setLockCompletionTimestamp(fakeClock.nowUtc()).build(); + saveRegistryLock(completedLock); + assertThat(getMostRecentUnlockedRegistryLockByRepoId(completedLock.getRepoId()).isPresent()) + .isFalse(); + } + private RegistryLock createLock() { return new RegistryLock.Builder() .setRepoId("repoId") diff --git a/core/src/test/java/google/registry/testing/SqlHelper.java b/core/src/test/java/google/registry/testing/SqlHelper.java index 6fbef9be8..d24af781f 100644 --- a/core/src/test/java/google/registry/testing/SqlHelper.java +++ b/core/src/test/java/google/registry/testing/SqlHelper.java @@ -40,9 +40,17 @@ public class SqlHelper { return jpaTm().transact(() -> RegistryLockDao.getMostRecentVerifiedLockByRepoId(repoId)); } + public static Optional getMostRecentUnlockedRegistryLockByRepoId(String repoId) { + return jpaTm().transact(() -> RegistryLockDao.getMostRecentVerifiedUnlockByRepoId(repoId)); + } + public static ImmutableList getRegistryLocksByRegistrarId(String registrarId) { return jpaTm().transact(() -> RegistryLockDao.getLocksByRegistrarId(registrarId)); } + public static Optional getRegistryLockByRevisionId(long revisionId) { + return jpaTm().transact(() -> RegistryLockDao.getByRevisionId(revisionId)); + } + private SqlHelper() {} } diff --git a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java index e911ecfd8..4892c1cd9 100644 --- a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java +++ b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java @@ -23,6 +23,7 @@ import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType; import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistActiveHost; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.SqlHelper.getRegistryLockByRevisionId; import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static org.junit.Assert.assertThrows; @@ -80,23 +81,26 @@ public final class DomainLockUtilsTest { @Test public void testSuccess_createLock() { - domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + RegistryLock lock = + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + assertNoDomainChanges(); + assertThat(lock.getLockCompletionTimestamp().isPresent()).isFalse(); } @Test public void testSuccess_createUnlock() { + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock lock = - domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); - domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false); + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false); + assertThat(lock.getUnlockCompletionTimestamp().isPresent()).isFalse(); } @Test public void testSuccess_createUnlock_adminUnlockingAdmin() { + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", null, true); RegistryLock lock = - domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", null, true); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), true); - domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", true); + domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", true); + assertThat(lock.getUnlockCompletionTimestamp().isPresent()).isFalse(); } @Test @@ -130,9 +134,7 @@ public final class DomainLockUtilsTest { @Test public void testSuccess_applyUnlockDomain() { - RegistryLock lock = - domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); - domainLockUtils.verifyAndApplyLock(lock.getVerificationCode(), false); + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); RegistryLock unlock = domainLockUtils.saveNewRegistryUnlockRequest(DOMAIN_NAME, "TheRegistrar", false); domainLockUtils.verifyAndApplyUnlock(unlock.getVerificationCode(), false); @@ -189,6 +191,31 @@ public final class DomainLockUtilsTest { verifyProperlyUnlockedDomain(true); } + @Test + public void testSuccess_regularLock_relockSet() { + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + RegistryLock oldLock = + domainLockUtils.administrativelyApplyUnlock(DOMAIN_NAME, "TheRegistrar", false); + RegistryLock newLock = + domainLockUtils.saveNewRegistryLockRequest(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + newLock = domainLockUtils.verifyAndApplyLock(newLock.getVerificationCode(), false); + assertThat( + getRegistryLockByRevisionId(oldLock.getRevisionId()).get().getRelock().getRevisionId()) + .isEqualTo(newLock.getRevisionId()); + } + + @Test + public void testSuccess_administrativelyLock_relockSet() { + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + RegistryLock oldLock = + domainLockUtils.administrativelyApplyUnlock(DOMAIN_NAME, "TheRegistrar", false); + RegistryLock newLock = + domainLockUtils.administrativelyApplyLock(DOMAIN_NAME, "TheRegistrar", POC_ID, false); + assertThat( + getRegistryLockByRevisionId(oldLock.getRevisionId()).get().getRelock().getRevisionId()) + .isEqualTo(newLock.getRevisionId()); + } + @Test public void testFailure_createUnlock_alreadyPendingUnlock() { RegistryLock lock = diff --git a/core/src/test/resources/google/registry/module/backend/backend_routing.txt b/core/src/test/resources/google/registry/module/backend/backend_routing.txt index 4601b456e..e65577530 100644 --- a/core/src/test/resources/google/registry/module/backend/backend_routing.txt +++ b/core/src/test/resources/google/registry/module/backend/backend_routing.txt @@ -31,6 +31,7 @@ PATH CLASS METHOD /_dr/task/rdeStaging RdeStagingAction GET,POST n INTERNAL,API APP ADMIN /_dr/task/rdeUpload RdeUploadAction POST n INTERNAL,API APP ADMIN /_dr/task/refreshDnsOnHostRename RefreshDnsOnHostRenameAction GET n INTERNAL,API APP ADMIN +/_dr/task/relockDomain RelockDomainAction POST y INTERNAL,API APP ADMIN /_dr/task/resaveAllEppResources ResaveAllEppResourcesAction GET n INTERNAL,API APP ADMIN /_dr/task/resaveEntity ResaveEntityAction POST n INTERNAL,API APP ADMIN /_dr/task/syncGroupMembers SyncGroupMembersAction POST n INTERNAL,API APP ADMIN diff --git a/db/README.md b/db/README.md index 1a49a2fb2..944979fb2 100644 --- a/db/README.md +++ b/db/README.md @@ -31,7 +31,7 @@ Below are the steps to submit a schema change: 1. Make your changes to entity classes, remembering to add new ones to `core/src/main/resources/META-INF/persistence.xml` so they'll be picked up. -2. Run the `nomulus generate_sql_schema` command to generate a new version of +2. Run the `devTool generate_sql_schema` command to generate a new version of `db-schema.sql.generated`. The full command line to do this is: `./gradlew devTool --args="-e localhost generate_sql_schema diff --git a/db/src/main/resources/sql/flyway/V19__add_registry_relock_reference.sql b/db/src/main/resources/sql/flyway/V19__add_registry_relock_reference.sql new file mode 100644 index 000000000..778f115a2 --- /dev/null +++ b/db/src/main/resources/sql/flyway/V19__add_registry_relock_reference.sql @@ -0,0 +1,20 @@ +-- Copyright 2020 The Nomulus Authors. All Rights Reserved. +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. + +ALTER TABLE "RegistryLock" ADD COLUMN relock_revision_id bigint; + +ALTER TABLE IF EXISTS "RegistryLock" + ADD CONSTRAINT FK2lhcwpxlnqijr96irylrh1707 + FOREIGN KEY (relock_revision_id) + REFERENCES "RegistryLock"; diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index 903bb42d0..fb832fa7e 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -180,6 +180,7 @@ unlock_completion_timestamp timestamptz, unlock_request_timestamp timestamptz, verification_code text not null, + relock_revision_id int8, primary key (revision_id) ); @@ -224,6 +225,11 @@ create index reservedlist_name_idx on "ReservedList" (name); foreign key (revision_id) references "PremiumList"; + alter table if exists "RegistryLock" + add constraint FK2lhcwpxlnqijr96irylrh1707 + foreign key (relock_revision_id) + references "RegistryLock"; + alter table if exists "ReservedEntry" add constraint FKgq03rk0bt1hb915dnyvd3vnfc foreign key (revision_id) diff --git a/db/src/main/resources/sql/schema/nomulus.golden.sql b/db/src/main/resources/sql/schema/nomulus.golden.sql index 9ac62b4b1..f8b9554b9 100644 --- a/db/src/main/resources/sql/schema/nomulus.golden.sql +++ b/db/src/main/resources/sql/schema/nomulus.golden.sql @@ -261,7 +261,8 @@ CREATE TABLE public."RegistryLock" ( verification_code text NOT NULL, unlock_request_timestamp timestamp with time zone, unlock_completion_timestamp timestamp with time zone, - last_update_timestamp timestamp with time zone + last_update_timestamp timestamp with time zone, + relock_revision_id bigint ); @@ -543,6 +544,14 @@ CREATE INDEX registrarpoc_gae_user_id_idx ON public."RegistrarPoc" USING btree ( CREATE INDEX reservedlist_name_idx ON public."ReservedList" USING btree (name); +-- +-- Name: RegistryLock fk2lhcwpxlnqijr96irylrh1707; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."RegistryLock" + ADD CONSTRAINT fk2lhcwpxlnqijr96irylrh1707 FOREIGN KEY (relock_revision_id) REFERENCES public."RegistryLock"(revision_id); + + -- -- Name: ClaimsEntry fk6sc6at5hedffc0nhdcab6ivuq; Type: FK CONSTRAINT; Schema: public; Owner: - --