diff --git a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java index f191ebe22..8ab7d43b7 100644 --- a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java +++ b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java @@ -169,16 +169,20 @@ public final class AsyncTaskEnqueuer { lock.getRelockDuration().isPresent(), "Lock with ID %s not configured for relock", lock.getRevisionId()); + enqueueDomainRelock(lock.getRelockDuration().get(), lock.getRevisionId(), 0); + } + + /** Enqueues a task to asynchronously re-lock a registry-locked domain after it was unlocked. */ + void enqueueDomainRelock(Duration countdown, long lockRevisionId, int previousAttempts) { String backendHostname = appEngineServiceUtils.getServiceHostname("backend"); addTaskToQueueWithRetry( asyncActionsPushQueue, TaskOptions.Builder.withUrl(RelockDomainAction.PATH) .method(Method.POST) .header("Host", backendHostname) - .param( - RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM, - String.valueOf(lock.getRevisionId())) - .countdownMillis(lock.getRelockDuration().get().getMillis())); + .param(RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM, String.valueOf(lockRevisionId)) + .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, String.valueOf(previousAttempts)) + .countdownMillis(countdown.getMillis())); } /** diff --git a/core/src/main/java/google/registry/batch/BatchModule.java b/core/src/main/java/google/registry/batch/BatchModule.java index d69db421e..d947abaa5 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.extractIntParameter; import static google.registry.request.RequestParameters.extractLongParameter; import static google.registry.request.RequestParameters.extractOptionalBooleanParameter; import static google.registry.request.RequestParameters.extractOptionalIntParameter; @@ -94,9 +95,15 @@ public class BatchModule { } @Provides - @Parameter("oldUnlockRevisionId") + @Parameter(RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM) static long provideOldUnlockRevisionId(HttpServletRequest req) { - return extractLongParameter(req, "oldUnlockRevisionId"); + return extractLongParameter(req, RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM); + } + + @Provides + @Parameter(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM) + static int providePreviousAttempts(HttpServletRequest req) { + return extractIntParameter(req, RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM); } @Provides diff --git a/core/src/main/java/google/registry/batch/RelockDomainAction.java b/core/src/main/java/google/registry/batch/RelockDomainAction.java index 3d0a24045..44ef261bd 100644 --- a/core/src/main/java/google/registry/batch/RelockDomainAction.java +++ b/core/src/main/java/google/registry/batch/RelockDomainAction.java @@ -15,19 +15,23 @@ package google.registry.batch; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; 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.config.RegistryConfig.Config; import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.StatusValue; +import google.registry.model.registrar.Registrar; +import google.registry.model.registrar.RegistrarContact; import google.registry.model.registry.RegistryLockDao; import google.registry.request.Action; import google.registry.request.Parameter; @@ -36,11 +40,15 @@ import google.registry.request.auth.Auth; import google.registry.schema.domain.RegistryLock; import google.registry.tools.DomainLockUtils; import google.registry.util.DateTimeUtils; +import google.registry.util.EmailMessage; +import google.registry.util.SendEmailService; +import java.util.Optional; import javax.inject.Inject; +import javax.mail.internet.AddressException; +import javax.mail.internet.InternetAddress; +import org.joda.time.Duration; -/** - * Task that relocks a previously-Registry-Locked domain after some predetermined period of time. - */ +/** Task that re-locks a previously-Registry-Locked domain after a predetermined period of time. */ @Action( service = Action.Service.BACKEND, path = RelockDomainAction.PATH, @@ -51,30 +59,78 @@ public class RelockDomainAction implements Runnable { public static final String PATH = "/_dr/task/relockDomain"; public static final String OLD_UNLOCK_REVISION_ID_PARAM = "oldUnlockRevisionId"; + public static final String PREVIOUS_ATTEMPTS_PARAM = "previousAttempts"; + + static final int ATTEMPTS_BEFORE_SLOWDOWN = 36; // every ten minutes for six hours then every hour + static final int FAILURES_BEFORE_EMAIL = 2; // email after three failures, one half hour + private static final Duration TEN_MINUTES = Duration.standardMinutes(10); + private static final Duration ONE_HOUR = Duration.standardHours(1); + + private static final String RELOCK_SUCCESS_EMAIL_TEMPLATE = + "The domain %s was successfully re-locked.\n\nPlease contact support at %s if you have any " + + "questions."; + private static final String RELOCK_NON_RETRYABLE_FAILURE_EMAIL_TEMPLATE = + "There was an error when automatically re-locking %s. Error message: %s\n\nPlease contact " + + "support at %s if you have any questions."; + private static final String RELOCK_TRANSIENT_FAILURE_EMAIL_TEMPLATE = + "There was an unexpected error when automatically re-locking %s. We will continue retrying " + + "the lock for five hours. Please contact support at %s if you have any questions"; + private static final String RELOCK_UNKNOWN_ID_FAILURE_EMAIL_TEMPLATE = + "The old lock with revision ID %d is not present or is not accessible"; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final long oldUnlockRevisionId; + private final int previousAttempts; + private final InternetAddress alertRecipientAddress; + private final InternetAddress gSuiteOutgoingEmailAddress; + private final String supportEmail; + private final SendEmailService sendEmailService; private final DomainLockUtils domainLockUtils; private final Response response; + private final AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject public RelockDomainAction( @Parameter(OLD_UNLOCK_REVISION_ID_PARAM) long oldUnlockRevisionId, + @Parameter(PREVIOUS_ATTEMPTS_PARAM) int previousAttempts, + @Config("alertRecipientEmailAddress") InternetAddress alertRecipientAddress, + @Config("gSuiteOutgoingEmailAddress") InternetAddress gSuiteOutgoingEmailAddress, + @Config("supportEmail") String supportEmail, + SendEmailService sendEmailService, DomainLockUtils domainLockUtils, - Response response) { + Response response, + AsyncTaskEnqueuer asyncTaskEnqueuer) { this.oldUnlockRevisionId = oldUnlockRevisionId; + this.previousAttempts = previousAttempts; + this.alertRecipientAddress = alertRecipientAddress; + this.gSuiteOutgoingEmailAddress = gSuiteOutgoingEmailAddress; + this.supportEmail = supportEmail; + this.sendEmailService = sendEmailService; this.domainLockUtils = domainLockUtils; this.response = response; + this.asyncTaskEnqueuer = asyncTaskEnqueuer; } @Override public void run() { - jpaTm().transact(this::relockDomain); + /* We wish to manually control our retry behavior, in order to limit the number of retries + * and/or notify registrars / support only after a certain number of retries, or only + * with a certain type of failure. AppEngine will automatically retry on any non-2xx status + * code, so return SC_NO_CONTENT (204) by default to avoid this auto-retry. + * + * See https://cloud.google.com/appengine/docs/standard/java/taskqueue/push/retrying-tasks + * for more details on retry behavior. */ + response.setStatus(SC_NO_CONTENT); + response.setContentType(MediaType.PLAIN_TEXT_UTF_8); + + // nb: DomainLockUtils relies on the JPA transaction being the outermost transaction + jpaTm().transact(() -> tm().transact(this::relockDomain)); } private void relockDomain() { - RegistryLock oldLock; + RegistryLock oldLock = null; + DomainBase domain; try { oldLock = RegistryLockDao.getByRevisionId(oldUnlockRevisionId) @@ -82,87 +138,187 @@ public class RelockDomainAction implements Runnable { () -> new IllegalArgumentException( String.format("Unknown revision ID %d", oldUnlockRevisionId))); - DomainBase domain = + 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.getDomainName()); - 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())); + handleTransientFailure(Optional.ofNullable(oldLock), t); return; } - applyRelock(oldLock); + + if (domain.getStatusValues().containsAll(REGISTRY_LOCK_STATUSES) + || oldLock.getRelock() != null) { + // The domain was manually locked, so we shouldn't worry about re-locking + String message = + String.format( + "Domain %s is already manually re-locked, skipping automated re-lock.", + domain.getDomainName()); + logger.atInfo().log(message); + response.setPayload(message); + return; + } + try { + verifyDomainAndLockState(oldLock, domain); + } catch (Throwable t) { + // If the domain was, for example, transferred, then notify the old registrar and don't retry. + handleNonRetryableFailure(oldLock, t); + return; + } + try { + applyRelock(oldLock); + } catch (Throwable t) { + handleTransientFailure(Optional.of(oldLock), t); + } } 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())); + domainLockUtils.administrativelyApplyLock( + oldLock.getDomainName(), + oldLock.getRegistrarId(), + oldLock.getRegistrarPocId(), + oldLock.isSuperuser()); + logger.atInfo().log("Re-locked domain %s.", oldLock.getDomainName()); + response.setStatus(SC_OK); + // Only send a success email if we previously sent a failure email + if (previousAttempts > FAILURES_BEFORE_EMAIL) { + sendSuccessEmail(oldLock); } } private void verifyDomainAndLockState(RegistryLock oldLock, DomainBase domain) { // Domain shouldn't be deleted or have a pending transfer/delete String domainName = domain.getDomainName(); - 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", + "Domain %s has a pending delete.", + domainName); + checkArgument( + !DateTimeUtils.isAtOrAfter(jpaTm().getTransactionTime(), domain.getDeletionTime()), + "Domain %s has been deleted.", domainName); checkArgument( !statusValues.contains(StatusValue.PENDING_TRANSFER), - "Domain %s has a 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", + "Domain %s has been transferred from registrar %s to registrar %s since the unlock.", domainName, oldLock.getRegistrarId(), domain.getCurrentSponsorClientId()); } + + private void handleNonRetryableFailure(RegistryLock oldLock, Throwable t) { + logger.atWarning().withCause(t).log( + "Exception thrown when attempting to re-lock domain with old revision ID %d.", + oldUnlockRevisionId); + response.setPayload(String.format("Re-lock failed: %s", t.getMessage())); + + String body = + String.format( + RELOCK_NON_RETRYABLE_FAILURE_EMAIL_TEMPLATE, + oldLock.getDomainName(), + t.getMessage(), + supportEmail); + sendEmailService.sendEmail( + EmailMessage.newBuilder() + .setFrom(gSuiteOutgoingEmailAddress) + .setBody(body) + .setSubject(String.format("Error re-locking domain %s", oldLock.getDomainName())) + .setRecipients(getEmailRecipients(oldLock.getRegistrarId())) + .build()); + } + + private void handleTransientFailure(Optional oldLock, Throwable t) { + String message = String.format("Re-lock failed: %s", t.getMessage()); + logger.atSevere().withCause(t).log(message); + response.setPayload(message); + + if (previousAttempts == FAILURES_BEFORE_EMAIL) { + if (oldLock.isPresent()) { + sendGenericTransientFailureEmail(oldLock.get()); + } else { + // if the old lock isn't present, something has gone horribly wrong + sendUnknownRevisionIdAlertEmail(); + } + } + Duration timeBeforeRetry = previousAttempts < ATTEMPTS_BEFORE_SLOWDOWN ? TEN_MINUTES : ONE_HOUR; + asyncTaskEnqueuer.enqueueDomainRelock( + timeBeforeRetry, oldUnlockRevisionId, previousAttempts + 1); + } + + private void sendSuccessEmail(RegistryLock oldLock) { + String body = + String.format(RELOCK_SUCCESS_EMAIL_TEMPLATE, oldLock.getDomainName(), supportEmail); + + sendEmailService.sendEmail( + EmailMessage.newBuilder() + .setFrom(gSuiteOutgoingEmailAddress) + .setBody(body) + .setSubject(String.format("Successful re-lock of domain %s", oldLock.getDomainName())) + .setRecipients(getEmailRecipients(oldLock.getRegistrarId())) + .build()); + } + + private void sendGenericTransientFailureEmail(RegistryLock oldLock) { + String body = + String.format( + RELOCK_TRANSIENT_FAILURE_EMAIL_TEMPLATE, oldLock.getDomainName(), supportEmail); + // For an unexpected failure, notify both the lock-enabled contacts and our alerting email + ImmutableSet allRecipients = + new ImmutableSet.Builder() + .addAll(getEmailRecipients(oldLock.getRegistrarId())) + .add(alertRecipientAddress) + .build(); + sendEmailService.sendEmail( + EmailMessage.newBuilder() + .setFrom(gSuiteOutgoingEmailAddress) + .setBody(body) + .setSubject(String.format("Error re-locking domain %s", oldLock.getDomainName())) + .setRecipients(allRecipients) + .build()); + } + + private void sendUnknownRevisionIdAlertEmail() { + sendEmailService.sendEmail( + EmailMessage.newBuilder() + .setFrom(gSuiteOutgoingEmailAddress) + .setBody(String.format(RELOCK_UNKNOWN_ID_FAILURE_EMAIL_TEMPLATE, oldUnlockRevisionId)) + .setSubject("Error re-locking domain") + .setRecipients(ImmutableSet.of(alertRecipientAddress)) + .build()); + } + + private ImmutableSet getEmailRecipients(String registrarId) { + Registrar registrar = + Registrar.loadByClientIdCached(registrarId) + .orElseThrow( + () -> + new IllegalStateException(String.format("Unknown registrar %s", registrarId))); + + ImmutableSet registryLockEmailAddresses = + registrar.getContacts().stream() + .filter(RegistrarContact::isRegistryLockAllowed) + .map(RegistrarContact::getRegistryLockEmailAddress) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(toImmutableSet()); + + ImmutableSet.Builder builder = new ImmutableSet.Builder<>(); + // can't use streams due to the 'throws' in the InternetAddress constructor + for (String registryLockEmailAddress : registryLockEmailAddresses) { + try { + builder.add(new InternetAddress(registryLockEmailAddress)); + } catch (AddressException e) { + // This shouldn't stop any other emails going out, so swallow it + logger.atWarning().log("Invalid email address %s", registryLockEmailAddress); + } + } + return builder.build(); + } } diff --git a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java index 6dad3e8be..b585e3297 100644 --- a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java +++ b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java @@ -31,7 +31,7 @@ import static google.registry.testing.TestLogHandlerUtils.assertLogMessage; import static org.joda.time.Duration.standardDays; import static org.joda.time.Duration.standardHours; import static org.joda.time.Duration.standardSeconds; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableSortedSet; @@ -159,7 +159,7 @@ public class AsyncTaskEnqueuerTest { .setRegistrarPocId("someone@example.com") .setVerificationCode("hi") .build()); - asyncTaskEnqueuer.enqueueDomainRelock(lock); + asyncTaskEnqueuer.enqueueDomainRelock(lock.getRelockDuration().get(), lock.getRevisionId(), 0); assertTasksEnqueued( QUEUE_ASYNC_ACTIONS, new TaskMatcher() @@ -169,6 +169,7 @@ public class AsyncTaskEnqueuerTest { .param( RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM, String.valueOf(lock.getRevisionId())) + .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, "0") .etaDelta( standardHours(6).minus(standardSeconds(30)), standardHours(6).plus(standardSeconds(30)))); @@ -188,9 +189,9 @@ public class AsyncTaskEnqueuerTest { .setVerificationCode("hi") .build()); assertThat( - assertThrows( - IllegalArgumentException.class, - () -> asyncTaskEnqueuer.enqueueDomainRelock(lockWithoutDuration))) + assertThrows( + IllegalArgumentException.class, + () -> asyncTaskEnqueuer.enqueueDomainRelock(lockWithoutDuration))) .hasMessageThat() .isEqualTo( String.format( diff --git a/core/src/test/java/google/registry/batch/RelockDomainActionTest.java b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java index 8f058d87f..a84aea326 100644 --- a/core/src/test/java/google/registry/batch/RelockDomainActionTest.java +++ b/core/src/test/java/google/registry/batch/RelockDomainActionTest.java @@ -15,10 +15,12 @@ package google.registry.batch; import static com.google.common.truth.Truth.assertThat; +import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS; 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.deleteResource; import static google.registry.testing.DatastoreHelper.newDomainBase; import static google.registry.testing.DatastoreHelper.persistActiveHost; import static google.registry.testing.DatastoreHelper.persistDomainAsDeleted; @@ -26,9 +28,16 @@ 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.testing.TaskQueueHelper.assertNoTasksEnqueued; +import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; +import static javax.servlet.http.HttpServletResponse.SC_OK; +import static org.joda.time.Duration.standardSeconds; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import com.google.common.collect.ImmutableSet; import google.registry.model.domain.DomainBase; @@ -38,17 +47,27 @@ import google.registry.testing.AppEngineExtension; import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; +import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.testing.UserInfo; import google.registry.tools.DomainLockUtils; import google.registry.util.AppEngineServiceUtils; +import google.registry.util.EmailMessage; +import google.registry.util.SendEmailService; import google.registry.util.StringGenerator.Alphabets; import java.util.Optional; +import javax.mail.internet.InternetAddress; +import org.joda.time.DateTime; import org.joda.time.Duration; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; /** Unit tests for {@link RelockDomainAction}. */ +@ExtendWith(MockitoExtension.class) public class RelockDomainActionTest { private static final String DOMAIN_NAME = "example.tld"; @@ -56,7 +75,7 @@ public class RelockDomainActionTest { private static final String POC_ID = "marla.singer@example.com"; private final FakeResponse response = new FakeResponse(); - private final FakeClock clock = new FakeClock(); + private final FakeClock clock = new FakeClock(DateTime.parse("2015-05-18T12:34:56Z")); private final DomainLockUtils domainLockUtils = new DomainLockUtils( new DeterministicStringGenerator(Alphabets.BASE_58), @@ -68,15 +87,18 @@ public class RelockDomainActionTest { public final AppEngineExtension appEngineRule = AppEngineExtension.builder() .withDatastoreAndCloudSql() + .withTaskQueue() .withUserService(UserInfo.create(POC_ID, "12345")) .build(); private DomainBase domain; private RegistryLock oldLock; + @Mock private SendEmailService sendEmailService; + private AsyncTaskEnqueuer asyncTaskEnqueuer; private RelockDomainAction action; @BeforeEach - void beforeEach() { + void beforeEach() throws Exception { createTlds("tld", "net"); HostResource host = persistActiveHost("ns1.example.net"); domain = persistResource(newDomainBase(DOMAIN_NAME, host)); @@ -88,9 +110,22 @@ public class RelockDomainActionTest { domainLockUtils.administrativelyApplyUnlock( DOMAIN_NAME, CLIENT_ID, false, Optional.empty()); assertThat(reloadDomain(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES); + + AppEngineServiceUtils appEngineServiceUtils = mock(AppEngineServiceUtils.class); + lenient() + .when(appEngineServiceUtils.getServiceHostname("backend")) + .thenReturn("backend.hostname.fake"); + + asyncTaskEnqueuer = + AsyncTaskEnqueuerTest.createForTesting(appEngineServiceUtils, clock, Duration.ZERO); action = createAction(oldLock.getRevisionId()); } + @AfterEach + void afterEach() { + verifyNoMoreInteractions(sendEmailService); + } + @Test void testLock() { action.run(); @@ -104,29 +139,36 @@ public class RelockDomainActionTest { } @Test - void testFailure_unknownCode() { + void testFailure_unknownCode() throws Exception { action = createAction(12128675309L); action.run(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); - assertThat(response.getPayload()).isEqualTo("Relock failed: Unknown revision ID 12128675309"); + assertThat(response.getPayload()).isEqualTo("Re-lock failed: Unknown revision ID 12128675309"); + assertTaskEnqueued(1, 12128675309L, Duration.standardMinutes(10)); // should retry, transient } @Test - void testFailure_pendingDelete() { + void testFailure_pendingDelete() throws Exception { persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_DELETE)).build()); action.run(); + String expectedFailureMessage = "Domain example.tld has a pending delete."; assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()) - .isEqualTo(String.format("Relock failed: Domain %s has a pending delete", DOMAIN_NAME)); + .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage)); + assertNonTransientFailureEmail(expectedFailureMessage); + assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } @Test - void testFailure_pendingTransfer() { + void testFailure_pendingTransfer() throws Exception { persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_TRANSFER)).build()); action.run(); + String expectedFailureMessage = "Domain example.tld has a pending transfer."; assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()) - .isEqualTo(String.format("Relock failed: Domain %s has a pending transfer", DOMAIN_NAME)); + .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage)); + assertNonTransientFailureEmail(expectedFailureMessage); + assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } @Test @@ -135,29 +177,64 @@ public class RelockDomainActionTest { action.run(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()) - .isEqualTo("Domain example.tld is already manually relocked, skipping automated relock."); + .isEqualTo("Domain example.tld is already manually re-locked, skipping automated re-lock."); + assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } @Test - void testFailure_domainDeleted() { + void testFailure_domainDeleted() throws Exception { persistDomainAsDeleted(domain, clock.nowUtc()); action.run(); + String expectedFailureMessage = "Domain example.tld has been deleted."; assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()) - .isEqualTo(String.format("Relock failed: Domain %s has been deleted", DOMAIN_NAME)); + .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage)); + assertNonTransientFailureEmail(expectedFailureMessage); + assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); } @Test - void testFailure_domainTransferred() { + void testFailure_domainTransferred() throws Exception { persistResource(domain.asBuilder().setPersistedCurrentSponsorClientId("NewRegistrar").build()); action.run(); + String expectedFailureMessage = + "Domain example.tld has been transferred from registrar TheRegistrar to registrar " + + "NewRegistrar since the unlock."; 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")); + .isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage)); + assertNonTransientFailureEmail(expectedFailureMessage); + assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); + } + + @Test + public void testFailure_transientFailure_enqueuesTask() { + // Hard-delete the domain to simulate a DB failure + deleteResource(domain); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()).isEqualTo("Re-lock failed: null"); + assertTaskEnqueued(1); + } + + @Test + void testFailure_sufficientTransientFailures_sendsEmail() throws Exception { + // Hard-delete the domain to simulate a DB failure + deleteResource(domain); + action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL); + action.run(); + assertTaskEnqueued(RelockDomainAction.FAILURES_BEFORE_EMAIL + 1); + assertTransientFailureEmail(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()).isEqualTo("Re-lock failed: null"); + } + + @Test + void testSuccess_afterSufficientFailures_sendsEmail() throws Exception { + action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL + 1); + action.run(); + assertThat(response.getStatus()).isEqualTo(SC_OK); + assertSuccessEmailSent(); } @Test @@ -170,14 +247,108 @@ public class RelockDomainActionTest { action.run(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()) - .isEqualTo("Domain example.tld is already manually relocked, skipping automated relock."); + .isEqualTo("Domain example.tld is already manually re-locked, skipping automated re-lock."); + assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS); + } + + @Test + void testFailure_slowsDown() throws Exception { + deleteResource(domain); + action = createAction(oldLock.getRevisionId(), RelockDomainAction.ATTEMPTS_BEFORE_SLOWDOWN); + action.run(); + assertTaskEnqueued( + RelockDomainAction.ATTEMPTS_BEFORE_SLOWDOWN + 1, + oldLock.getRevisionId(), + Duration.standardHours(1)); + } + + private void assertSuccessEmailSent() throws Exception { + EmailMessage expectedEmail = + EmailMessage.newBuilder() + .setSubject("Successful re-lock of domain example.tld") + .setBody( + "The domain example.tld was successfully re-locked.\n\nPlease " + + "contact support at support@example.com if you have any questions.") + .setRecipients( + ImmutableSet.of(new InternetAddress("Marla.Singer.RegistryLock@crr.com"))) + .setFrom(new InternetAddress("outgoing@example.com")) + .build(); + verify(sendEmailService).sendEmail(expectedEmail); + } + + private void assertNonTransientFailureEmail(String exceptionMessage) throws Exception { + String expectedBody = + String.format( + "There was an error when automatically re-locking example.tld. Error message: %s\n\n" + + "Please contact support at support@example.com if you have any questions.", + exceptionMessage); + assertFailureEmailWithBody( + expectedBody, ImmutableSet.of(new InternetAddress("Marla.Singer.RegistryLock@crr.com"))); + } + + private void assertTransientFailureEmail() throws Exception { + String expectedBody = + "There was an unexpected error when automatically re-locking example.tld. We will continue " + + "retrying the lock for five hours. Please contact support at support@example.com if " + + "you have any questions"; + assertFailureEmailWithBody( + expectedBody, + ImmutableSet.of( + new InternetAddress("Marla.Singer.RegistryLock@crr.com"), + new InternetAddress("alerts@example.com"))); + } + + private void assertFailureEmailWithBody(String body, ImmutableSet recipients) + throws Exception { + EmailMessage expectedEmail = + EmailMessage.newBuilder() + .setSubject("Error re-locking domain example.tld") + .setBody(body) + .setRecipients(recipients) + .setFrom(new InternetAddress("outgoing@example.com")) + .build(); + verify(sendEmailService).sendEmail(expectedEmail); + } + + private void assertTaskEnqueued(int numAttempts) { + assertTaskEnqueued(numAttempts, oldLock.getRevisionId(), Duration.standardMinutes(10)); + } + + private void assertTaskEnqueued(int numAttempts, long oldUnlockRevisionId, Duration duration) { + assertTasksEnqueued( + QUEUE_ASYNC_ACTIONS, + new TaskMatcher() + .url(RelockDomainAction.PATH) + .method("POST") + .header("Host", "backend.hostname.fake") + .param( + RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM, + String.valueOf(oldUnlockRevisionId)) + .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, String.valueOf(numAttempts)) + .etaDelta(duration.minus(standardSeconds(30)), duration.plus(standardSeconds(30)))); } private DomainBase reloadDomain(DomainBase domain) { return ofy().load().entity(domain).now(); } - private RelockDomainAction createAction(Long oldUnlockRevisionId) { - return new RelockDomainAction(oldUnlockRevisionId, domainLockUtils, response); + private RelockDomainAction createAction(Long oldUnlockRevisionId) throws Exception { + return createAction(oldUnlockRevisionId, 0); + } + + private RelockDomainAction createAction(Long oldUnlockRevisionId, int previousAttempts) + throws Exception { + InternetAddress alertRecipientAddress = new InternetAddress("alerts@example.com"); + InternetAddress gSuiteOutgoingAddress = new InternetAddress("outgoing@example.com"); + return new RelockDomainAction( + oldUnlockRevisionId, + previousAttempts, + alertRecipientAddress, + gSuiteOutgoingAddress, + "support@example.com", + sendEmailService, + domainLockUtils, + response, + asyncTaskEnqueuer); } } diff --git a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java index 0a09336c9..bc4f89658 100644 --- a/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java +++ b/core/src/test/java/google/registry/tools/DomainLockUtilsTest.java @@ -271,6 +271,7 @@ public final class DomainLockUtilsTest { .param( RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM, String.valueOf(lock.getRevisionId())) + .param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, "0") .etaDelta( standardHours(6).minus(standardSeconds(30)), standardDays(6).plus(standardSeconds(30)))); diff --git a/util/src/main/java/google/registry/util/EmailMessage.java b/util/src/main/java/google/registry/util/EmailMessage.java index 2984b53c1..646c06990 100644 --- a/util/src/main/java/google/registry/util/EmailMessage.java +++ b/util/src/main/java/google/registry/util/EmailMessage.java @@ -16,6 +16,7 @@ package google.registry.util; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.net.MediaType; import java.util.Collection; import java.util.Optional; @@ -27,10 +28,10 @@ public abstract class EmailMessage { public abstract String subject(); public abstract String body(); - public abstract ImmutableList recipients(); + public abstract ImmutableSet recipients(); public abstract InternetAddress from(); - public abstract ImmutableList bccs(); + public abstract ImmutableSet bccs(); public abstract Optional contentType(); public abstract Optional attachment(); @@ -63,9 +64,9 @@ public abstract class EmailMessage { public abstract Builder setContentType(MediaType contentType); public abstract Builder setAttachment(Attachment attachment); - abstract ImmutableList.Builder recipientsBuilder(); + abstract ImmutableSet.Builder recipientsBuilder(); - abstract ImmutableList.Builder bccsBuilder(); + abstract ImmutableSet.Builder bccsBuilder(); public Builder addRecipient(InternetAddress value) { recipientsBuilder().add(value);