mirror of
https://github.com/google/nomulus.git
synced 2025-08-12 20:49:37 +02:00
Throttle outgoing emails (#2168)
Adds a delay between emails sent in a tight loop. This helps avoid triggering Gmail abuse detections. Also updated the recipient address for billing alerts.
This commit is contained in:
parent
4fb8a1b50b
commit
1580555d30
6 changed files with 52 additions and 1 deletions
|
@ -879,6 +879,17 @@ public final class RegistryConfig {
|
||||||
return Optional.ofNullable(config.misc.sheetExportId);
|
return Optional.ofNullable(config.misc.sheetExportId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns the desired delay between outgoing emails when sending in bulk.
|
||||||
|
*
|
||||||
|
* <p>Gmail apparently has unpublished limits on peak throughput over short period.
|
||||||
|
*/
|
||||||
|
@Provides
|
||||||
|
@Config("emailThrottleDuration")
|
||||||
|
public static Duration provideEmailThrottleSeconds(RegistryConfigSettings config) {
|
||||||
|
return Duration.standardSeconds(config.misc.emailThrottleSeconds);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the email address we send various alert e-mails to.
|
* Returns the email address we send various alert e-mails to.
|
||||||
*
|
*
|
||||||
|
|
|
@ -208,6 +208,7 @@ public class RegistryConfigSettings {
|
||||||
public static class Misc {
|
public static class Misc {
|
||||||
public String sheetExportId;
|
public String sheetExportId;
|
||||||
public boolean isEmailSendingEnabled;
|
public boolean isEmailSendingEnabled;
|
||||||
|
public int emailThrottleSeconds;
|
||||||
public String alertRecipientEmailAddress;
|
public String alertRecipientEmailAddress;
|
||||||
// TODO(b/279671974): remove below field after migration
|
// TODO(b/279671974): remove below field after migration
|
||||||
public String newAlertRecipientEmailAddress;
|
public String newAlertRecipientEmailAddress;
|
||||||
|
|
|
@ -443,6 +443,9 @@ misc:
|
||||||
# Whether emails may be sent. For Prod and Sandbox this should be true.
|
# Whether emails may be sent. For Prod and Sandbox this should be true.
|
||||||
isEmailSendingEnabled: false
|
isEmailSendingEnabled: false
|
||||||
|
|
||||||
|
# Delay between bulk messages to avoid triggering Gmail fraud checks
|
||||||
|
emailThrottleSeconds: 30
|
||||||
|
|
||||||
# Address we send alert summary emails to.
|
# Address we send alert summary emails to.
|
||||||
alertRecipientEmailAddress: email@example.com
|
alertRecipientEmailAddress: email@example.com
|
||||||
|
|
||||||
|
|
|
@ -53,7 +53,7 @@ public class BillingEmailUtils {
|
||||||
GmailClient gmailClient,
|
GmailClient gmailClient,
|
||||||
YearMonth yearMonth,
|
YearMonth yearMonth,
|
||||||
@Config("gSuiteOutgoingEmailAddress") InternetAddress outgoingEmailAddress,
|
@Config("gSuiteOutgoingEmailAddress") InternetAddress outgoingEmailAddress,
|
||||||
@Config("alertRecipientEmailAddress") InternetAddress alertRecipientAddress,
|
@Config("newAlertRecipientEmailAddress") InternetAddress alertRecipientAddress,
|
||||||
@Config("invoiceEmailRecipients") ImmutableList<InternetAddress> invoiceEmailRecipients,
|
@Config("invoiceEmailRecipients") ImmutableList<InternetAddress> invoiceEmailRecipients,
|
||||||
@Config("invoiceReplyToEmailAddress") Optional<InternetAddress> replyToEmailAddress,
|
@Config("invoiceReplyToEmailAddress") Optional<InternetAddress> replyToEmailAddress,
|
||||||
@Config("billingBucket") String billingBucket,
|
@Config("billingBucket") String billingBucket,
|
||||||
|
|
|
@ -37,11 +37,13 @@ import google.registry.model.registrar.Registrar;
|
||||||
import google.registry.model.registrar.RegistrarPoc;
|
import google.registry.model.registrar.RegistrarPoc;
|
||||||
import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo;
|
import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo;
|
||||||
import google.registry.util.EmailMessage;
|
import google.registry.util.EmailMessage;
|
||||||
|
import google.registry.util.Sleeper;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import javax.inject.Inject;
|
import javax.inject.Inject;
|
||||||
import javax.mail.MessagingException;
|
import javax.mail.MessagingException;
|
||||||
import javax.mail.internet.InternetAddress;
|
import javax.mail.internet.InternetAddress;
|
||||||
|
import org.joda.time.Duration;
|
||||||
import org.joda.time.LocalDate;
|
import org.joda.time.LocalDate;
|
||||||
|
|
||||||
/** Provides e-mail functionality for Spec11 tasks, such as sending Spec11 reports to registrars. */
|
/** Provides e-mail functionality for Spec11 tasks, such as sending Spec11 reports to registrars. */
|
||||||
|
@ -57,6 +59,8 @@ public class Spec11EmailUtils {
|
||||||
.build()
|
.build()
|
||||||
.compileToTofu();
|
.compileToTofu();
|
||||||
private final GmailClient gmailClient;
|
private final GmailClient gmailClient;
|
||||||
|
private final Sleeper sleeper;
|
||||||
|
private final Duration emailThrottleDuration;
|
||||||
private final InternetAddress outgoingEmailAddress;
|
private final InternetAddress outgoingEmailAddress;
|
||||||
private final ImmutableList<InternetAddress> spec11BccEmailAddresses;
|
private final ImmutableList<InternetAddress> spec11BccEmailAddresses;
|
||||||
private final InternetAddress alertRecipientAddress;
|
private final InternetAddress alertRecipientAddress;
|
||||||
|
@ -66,12 +70,16 @@ public class Spec11EmailUtils {
|
||||||
@Inject
|
@Inject
|
||||||
Spec11EmailUtils(
|
Spec11EmailUtils(
|
||||||
GmailClient gmailClient,
|
GmailClient gmailClient,
|
||||||
|
Sleeper sleeper,
|
||||||
|
@Config("emailThrottleDuration") Duration emailThrottleDuration,
|
||||||
@Config("newAlertRecipientEmailAddress") InternetAddress alertRecipientAddress,
|
@Config("newAlertRecipientEmailAddress") InternetAddress alertRecipientAddress,
|
||||||
@Config("spec11OutgoingEmailAddress") InternetAddress spec11OutgoingEmailAddress,
|
@Config("spec11OutgoingEmailAddress") InternetAddress spec11OutgoingEmailAddress,
|
||||||
@Config("spec11BccEmailAddresses") ImmutableList<InternetAddress> spec11BccEmailAddresses,
|
@Config("spec11BccEmailAddresses") ImmutableList<InternetAddress> spec11BccEmailAddresses,
|
||||||
@Config("spec11WebResources") ImmutableList<String> spec11WebResources,
|
@Config("spec11WebResources") ImmutableList<String> spec11WebResources,
|
||||||
@Config("registryName") String registryName) {
|
@Config("registryName") String registryName) {
|
||||||
this.gmailClient = gmailClient;
|
this.gmailClient = gmailClient;
|
||||||
|
this.sleeper = sleeper;
|
||||||
|
this.emailThrottleDuration = emailThrottleDuration;
|
||||||
this.outgoingEmailAddress = spec11OutgoingEmailAddress;
|
this.outgoingEmailAddress = spec11OutgoingEmailAddress;
|
||||||
this.spec11BccEmailAddresses = spec11BccEmailAddresses;
|
this.spec11BccEmailAddresses = spec11BccEmailAddresses;
|
||||||
this.alertRecipientAddress = alertRecipientAddress;
|
this.alertRecipientAddress = alertRecipientAddress;
|
||||||
|
@ -94,6 +102,13 @@ public class Spec11EmailUtils {
|
||||||
for (RegistrarThreatMatches registrarThreatMatches : registrarThreatMatchesSet) {
|
for (RegistrarThreatMatches registrarThreatMatches : registrarThreatMatchesSet) {
|
||||||
RegistrarThreatMatches filteredMatches = filterOutNonPublishedMatches(registrarThreatMatches);
|
RegistrarThreatMatches filteredMatches = filterOutNonPublishedMatches(registrarThreatMatches);
|
||||||
if (!filteredMatches.threatMatches().isEmpty()) {
|
if (!filteredMatches.threatMatches().isEmpty()) {
|
||||||
|
if (numRegistrarsEmailed > 0) {
|
||||||
|
try {
|
||||||
|
sleeper.sleep(emailThrottleDuration);
|
||||||
|
} catch (InterruptedException ie) {
|
||||||
|
throw new RuntimeException(ie);
|
||||||
|
}
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
// Handle exceptions individually per registrar so that one failed email doesn't prevent
|
// Handle exceptions individually per registrar so that one failed email doesn't prevent
|
||||||
// the rest from being sent.
|
// the rest from being sent.
|
||||||
|
|
|
@ -26,6 +26,8 @@ import static google.registry.testing.DatabaseHelper.loadByEntity;
|
||||||
import static google.registry.testing.DatabaseHelper.persistActiveHost;
|
import static google.registry.testing.DatabaseHelper.persistActiveHost;
|
||||||
import static google.registry.testing.DatabaseHelper.persistResource;
|
import static google.registry.testing.DatabaseHelper.persistResource;
|
||||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||||
|
import static org.mockito.ArgumentMatchers.any;
|
||||||
|
import static org.mockito.ArgumentMatchers.same;
|
||||||
import static org.mockito.Mockito.doThrow;
|
import static org.mockito.Mockito.doThrow;
|
||||||
import static org.mockito.Mockito.times;
|
import static org.mockito.Mockito.times;
|
||||||
import static org.mockito.Mockito.verify;
|
import static org.mockito.Mockito.verify;
|
||||||
|
@ -41,11 +43,13 @@ import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationT
|
||||||
import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo;
|
import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo;
|
||||||
import google.registry.testing.DatabaseHelper;
|
import google.registry.testing.DatabaseHelper;
|
||||||
import google.registry.util.EmailMessage;
|
import google.registry.util.EmailMessage;
|
||||||
|
import google.registry.util.Sleeper;
|
||||||
import java.util.LinkedHashSet;
|
import java.util.LinkedHashSet;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
import javax.mail.MessagingException;
|
import javax.mail.MessagingException;
|
||||||
import javax.mail.internet.InternetAddress;
|
import javax.mail.internet.InternetAddress;
|
||||||
|
import org.joda.time.Duration;
|
||||||
import org.joda.time.LocalDate;
|
import org.joda.time.LocalDate;
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
@ -101,6 +105,8 @@ class Spec11EmailUtilsTest {
|
||||||
new JpaTestExtensions.Builder().buildIntegrationTestExtension();
|
new JpaTestExtensions.Builder().buildIntegrationTestExtension();
|
||||||
|
|
||||||
@Mock private GmailClient gmailClient;
|
@Mock private GmailClient gmailClient;
|
||||||
|
@Mock private Sleeper sleeper;
|
||||||
|
private Duration emailThrottleDuration = Duration.millis(1);
|
||||||
private Spec11EmailUtils emailUtils;
|
private Spec11EmailUtils emailUtils;
|
||||||
private ArgumentCaptor<EmailMessage> contentCaptor;
|
private ArgumentCaptor<EmailMessage> contentCaptor;
|
||||||
private final LocalDate date = new LocalDate(2018, 7, 15);
|
private final LocalDate date = new LocalDate(2018, 7, 15);
|
||||||
|
@ -114,6 +120,8 @@ class Spec11EmailUtilsTest {
|
||||||
emailUtils =
|
emailUtils =
|
||||||
new Spec11EmailUtils(
|
new Spec11EmailUtils(
|
||||||
gmailClient,
|
gmailClient,
|
||||||
|
sleeper,
|
||||||
|
emailThrottleDuration,
|
||||||
new InternetAddress("my-receiver@test.com"),
|
new InternetAddress("my-receiver@test.com"),
|
||||||
new InternetAddress("abuse@test.com"),
|
new InternetAddress("abuse@test.com"),
|
||||||
ImmutableList.of(
|
ImmutableList.of(
|
||||||
|
@ -128,6 +136,19 @@ class Spec11EmailUtilsTest {
|
||||||
persistDomainWithHost("c.com", host);
|
persistDomainWithHost("c.com", host);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void testSuccess_sleepsBetweenSending() throws Exception {
|
||||||
|
emailUtils.emailSpec11Reports(
|
||||||
|
date,
|
||||||
|
Spec11EmailSoyInfo.MONTHLY_SPEC_11_EMAIL,
|
||||||
|
"Super Cool Registry Monthly Threat Detector [2018-07-15]",
|
||||||
|
sampleThreatMatches());
|
||||||
|
// We inspect individual parameters because Message doesn't implement equals().
|
||||||
|
verify(gmailClient, times(3)).sendEmail(any(EmailMessage.class));
|
||||||
|
// Sleep once between two reports sent in a tight loop. No sleep before the final alert message.
|
||||||
|
verify(sleeper, times(1)).sleep(same(emailThrottleDuration));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void testSuccess_emailMonthlySpec11Reports() throws Exception {
|
void testSuccess_emailMonthlySpec11Reports() throws Exception {
|
||||||
emailUtils.emailSpec11Reports(
|
emailUtils.emailSpec11Reports(
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue